[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