Test: Replace test relying on pselect6(2) man page ambiguity

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 an invalid FD is passed.

[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@efficios.com>
Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/tests/regression/kernel/select_poll_epoll.c b/tests/regression/kernel/select_poll_epoll.c
index 4b703b38039f60ac9ad146ca750d60af743e1a3f..08e7fce0d898df547da42e371dc694f90bea4141 100644 (file)
--- a/tests/regression/kernel/select_poll_epoll.c
+++ b/tests/regression/kernel/select_poll_epoll.c
@@ -437,29 +437,36 @@ void ppoll_fds_ulong_max(void)
 }
 
 /*
- * Select is limited to 1024 FDs, should output a pselect event
- * with 0 FDs.
+ * Pass an 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;
+       }
+
+       ret = close(fd);
+       if (ret == -1) {
+               perror("close");
+               goto error;
        }
 
-       FD_SET(fd2, (fd_set *) &rfds);
-       ret = syscall(SYS_pselect6, fd2 + 1, &rfds, NULL, NULL, NULL, NULL);
+       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) {
@@ -471,7 +478,8 @@ void pselect_fd_too_big(void)
        } else {
                printf("# [pselect] timeout\n");
        }
-
+error:
+       return;
 }
 
 /*
@@ -812,14 +820,14 @@ void print_list(void)
                        "and epoll, waiting for input\n");
        fprintf(stderr, "\t2: Timeout cases (1ms) for select, pselect6, poll, "
                        "ppoll and epoll\n");
-       fprintf(stderr, "\t3: pselect with a FD > 1023\n");
+       fprintf(stderr, "\t3: pselect with an invalid fd\n");
        fprintf(stderr, "\t4: ppoll with %d FDs\n", MAX_FDS);
        fprintf(stderr, "\t5: ppoll buffer overflow, should segfault, waits "
                        "for input\n");
-       fprintf(stderr, "\t6: pselect with invalid pointer, waits for "
+       fprintf(stderr, "\t6: pselect with an invalid pointer, waits for "
                        "input\n");
        fprintf(stderr, "\t7: ppoll with ulong_max fds, waits for input\n");
-       fprintf(stderr, "\t8: epoll_pwait with invalid pointer, waits for "
+       fprintf(stderr, "\t8: epoll_pwait with an invalid pointer, waits for "
                        "input\n");
        fprintf(stderr, "\t9: epoll_pwait with maxevents set to INT_MAX, "
                        "waits for input\n");
@@ -892,7 +900,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 e01866f75dcf22c0f0b6a603b9fffa08ee517ccc..ec034e6320c9fff90b7689c56aaad07895e0dabe 100755 (executable)
--- 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 f4946e73a94fb9b11432930cac49903a705c8aee..613cec3ab3480b53ed6d6a6f03a939ad8fdacf08 100755 (executable)
--- 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):