[lttng-dev] [PATCH lttng-tools V2] Test: Replace test relying pselect6(2) man page ambiguity

Jérémie Galarneau jeremie.galarneau at efficios.com
Thu Jun 1 18:14:53 UTC 2017


Merged with minor comment/documentation modifications in master,
stable-2.10, and stable-2.9.

Also, the "usage" text was not adapted to reflect the new test.

Thanks!
Jérémie

On 31 May 2017 at 17:08, Francis Deslauriers
<francis.deslauriers at efficios.com> wrote:
> The `pselect_fd_too_big` test is checking for the case where the `nfds`
> is larger than the number of open files allowed for this process
> (RLIMIT_NOFILE).
>
> According to the ERRORS section of the pselect6(2) kernel man page[1], if
> `nfds` > RLIMIT_NOFILE is evaluate to true the pselect6 syscall should
> return EINVAL but the BUGS section mentions that the current
> implementation ignores any FD larger than the highest numbered FD of the
> current process.
>
> This is in fact what happens, the Linux implementation of the pselect6
> syscall[2] does not compare the `nfds` and RLIMIT_NOFILE but rather caps
> `nfds` to the highest numbered FD of the current process as the BUGS
> kernel man page mentionned.
>
> It was observed elsewhere that there is a discrepancy between the manual
> page and the implementation[3].
>
> As a solution, replace the current testcase with one that checks the
> behaviour of the syscall when passed an invalid FD.
>
> [1]:http://man7.org/linux/man-pages/man2/pselect6.2.html
> [2]:https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/select.c#n619
> [3]:https://patchwork.kernel.org/patch/9345805/
>
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> Signed-off-by: Julien Desfossez <jdesfossez at efficios.com>
> ---
>  tests/regression/kernel/select_poll_epoll.c        | 39 +++++++++++++---------
>  tests/regression/kernel/test_select_poll_epoll     |  6 ++--
>  .../kernel/validate_select_poll_epoll.py           | 13 ++++----
>  3 files changed, 33 insertions(+), 25 deletions(-)
>
> diff --git a/tests/regression/kernel/select_poll_epoll.c b/tests/regression/kernel/select_poll_epoll.c
> index 4b703b3..1d767b0 100644
> --- a/tests/regression/kernel/select_poll_epoll.c
> +++ b/tests/regression/kernel/select_poll_epoll.c
> @@ -437,30 +437,38 @@ void ppoll_fds_ulong_max(void)
>  }
>
>  /*
> - * Select is limited to 1024 FDs, should output a pselect event
> - * with 0 FDs.
> + * Pass a invalid file descriptor to pselect6(). The syscall should return
> + * -EBADF. The recorded event should contain a ret=-EBADF (-9).
>   */
> -void pselect_fd_too_big(void)
> +void pselect_invalid_fd(void)
>  {
> -       long rfds[2048 / (sizeof(long) * CHAR_BIT)] = { 0 };
> +       fd_set rfds;
>         int ret;
> -       int fd2;
> +       int fd;
>         char buf[BUF_SIZE];
>
>         /*
> -        * Test if nfds > 1024.
> -        * Make sure ulimit is set correctly (ulimit -n 2048).
> +        * Open a file, close it and use the closed FD in the pselect6 call
>          */
> -       fd2 = dup2(wait_fd, 2047);
> -       if (fd2 != 2047) {
> -               perror("dup2");
> -               return;
> +
> +       fd = open("/dev/null", O_RDONLY);
> +       if (fd == -1) {
> +               perror("open");
> +               goto error;
>         }
>
> -       FD_SET(fd2, (fd_set *) &rfds);
> -       ret = syscall(SYS_pselect6, fd2 + 1, &rfds, NULL, NULL, NULL, NULL);
> +       ret = close(fd);
>
>         if (ret == -1) {
> +               perror("close");
> +               goto error;
> +       }
> +
> +       FD_ZERO(&rfds);
> +       FD_SET(fd, &rfds);
> +
> +       ret = syscall(SYS_pselect6, fd + 1, &rfds, NULL, NULL, NULL, NULL);
> +       if (ret == -1) {
>                 perror("# pselect()");
>         } else if (ret) {
>                 printf("# [pselect] data available\n");
> @@ -471,7 +479,8 @@ void pselect_fd_too_big(void)
>         } else {
>                 printf("# [pselect] timeout\n");
>         }
> -
> +error:
> +       return;
>  }
>
>  /*
> @@ -892,7 +901,7 @@ int main(int argc, const char **argv)
>                 run_working_cases();
>                 break;
>         case 3:
> -               pselect_fd_too_big();
> +               pselect_invalid_fd();
>                 break;
>         case 4:
>                 test_ppoll_big();
> diff --git a/tests/regression/kernel/test_select_poll_epoll b/tests/regression/kernel/test_select_poll_epoll
> index e01866f..ec034e6 100755
> --- a/tests/regression/kernel/test_select_poll_epoll
> +++ b/tests/regression/kernel/test_select_poll_epoll
> @@ -126,13 +126,13 @@ function test_timeout_cases()
>         rm -rf $TRACE_PATH
>  }
>
> -function test_big_pselect()
> +function test_pselect_invalid_fd()
>  {
>         TRACE_PATH=$(mktemp -d)
>         SESSION_NAME="syscall_payload"
>         SYSCALL_LIST="pselect6"
>
> -       diag "pselect with a FD > 1023"
> +       diag "pselect with invalid FD"
>
>         create_lttng_session_ok $SESSION_NAME $TRACE_PATH
>
> @@ -384,7 +384,7 @@ skip $isroot "Root access is needed. Skipping all tests." $NUM_TESTS ||
>
>         test_working_cases
>         test_timeout_cases
> -       test_big_pselect
> +       test_pselect_invalid_fd
>         test_big_ppoll
>         test_ppoll_overflow
>         test_pselect_invalid_ptr
> diff --git a/tests/regression/kernel/validate_select_poll_epoll.py b/tests/regression/kernel/validate_select_poll_epoll.py
> index f4946e7..613cec3 100755
> --- a/tests/regression/kernel/validate_select_poll_epoll.py
> +++ b/tests/regression/kernel/validate_select_poll_epoll.py
> @@ -450,8 +450,8 @@ class Test2(TraceParser):
>  class Test3(TraceParser):
>      def __init__(self, trace, pid):
>          super().__init__(trace, pid)
> -        self.expect["select_too_big_in"] = 0
> -        self.expect["select_too_big_out"] = 0
> +        self.expect["select_invalid_fd_in"] = 0
> +        self.expect["select_invalid_fd_out"] = 0
>
>      def select_entry(self, event):
>          timestamp = event.timestamp
> @@ -466,9 +466,8 @@ class Test3(TraceParser):
>          _exceptfds_length = event["_exceptfds_length"]
>          exceptfds = event["exceptfds"]
>
> -        # make sure an invalid value still produces a valid event
> -        if n == 2048 and overflow == 0 and _readfds_length == 0:
> -            self.expect["select_too_big_in"] = 1
> +        if n > 0 and overflow == 0:
> +            self.expect["select_invalid_fd_in"] = 1
>
>      def select_exit(self, event):
>          timestamp = event.timestamp
> @@ -483,9 +482,9 @@ class Test3(TraceParser):
>          _exceptfds_length = event["_exceptfds_length"]
>          exceptfds = event["exceptfds"]
>
> -        # make sure an invalid value still produces a valid event
> +        # make sure the event has a ret field equal to -EBADF
>          if ret == -9 and overflow == 0 and _readfds_length == 0:
> -            self.expect["select_too_big_out"] = 1
> +            self.expect["select_invalid_fd_out"] = 1
>
>
>  class Test4(TraceParser):
> --
> 2.7.4
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com


More information about the lttng-dev mailing list