From 85716797531014241b9bc2b2ffa01215fd9ba6d1 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Wed, 31 May 2017 17:08:23 -0400 Subject: [PATCH] Test: Replace test relying on pselect6(2) man page ambiguity MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Julien Desfossez Signed-off-by: Jérémie Galarneau --- tests/regression/kernel/select_poll_epoll.c | 44 +++++++++++-------- .../regression/kernel/test_select_poll_epoll | 6 +-- .../kernel/validate_select_poll_epoll.py | 13 +++--- 3 files changed, 35 insertions(+), 28 deletions(-) diff --git a/tests/regression/kernel/select_poll_epoll.c b/tests/regression/kernel/select_poll_epoll.c index 4b703b380..08e7fce0d 100644 --- 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 e01866f75..ec034e632 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 f4946e73a..613cec3ab 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.34.1