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

[Francis Deslauriers]

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