[PATCH 0/3] tools/nolibc: fix readdir_r() and the FD_* macros on 64-bit

[PATCH 0/3] tools/nolibc: fix readdir_r() and the FD_* macros on 64-bit

[Danish Khateeb]

Two bugs that only show up on 64-bit architectures:

- readdir_r() truncates the directory offset to an int. On ext4 it
  fails within the first few entries of almost every directory (since
  v6.19), or returns the next-to-last entry twice and never the last
  one (v6.15 to v6.18). Patch 1.

- FD_SET(), FD_CLR() and FD_ISSET() build their masks from an int, so
  they use the wrong bits for fds 31-63 of each 64-bit word, and
  select() on fd 40 fails with EBADF. Patch 2, with tests in patch 3.

There is no selftest for patch 1: it needs a directory with large
offsets, and nolibc-test only lists /proc/self, whose offsets are
small. I tested it with a small program that lists a directory with
readdir_r() instead.

The series applies to nolibc/for-next f2212892b6a0 and is independent
of my WIFSIGNALED() series [1]; the two merge cleanly.

Testing, with GCC 16.2, on x86_64 and i386 natively and on arm, arm64
and sparc64 under qemu-user:

- nolibc-test, all tests: no failures before or after the series. The
  only difference is the two new tests, which pass. They also pass
  against glibc (make libc-test).

- On x86_64 without patch 2, fd_set and select_high_fd fail (select()
  returns EBADF). With the default UBSan flags the run stops with
  SIGILL at fd_set instead.

- The readdir_r() program, on ext4, on x86_64, arm64 and sparc64:
  before, it fails on /etc after 4 of 213 entries, /usr/include after
  1 of 1454 and /usr/lib after 2 of 6186. After, it lists as many
  entries as glibc, and on x86_64 the same names. A native i386 build
  was already correct. The arm build failed too, but only because
  under qemu-user the kernel sees a 64-bit process and hands it
  64-bit offsets.

- Built for x86_64 against nolibc from before commit 4ada5679f18d, the
  same program lists a directory holding "..", "code" and "." as "..",
  "code", "code".

[1] https://lore.kernel.org/all/20260923205622.1123270-1-danishkhateeb03@gmail.com/

Danish Khateeb (3):
  tools/nolibc: fix readdir_r() with 64-bit directory offsets
  tools/nolibc: fix FD_SET(), FD_CLR() and FD_ISSET() on 64-bit
  selftests/nolibc: test the FD_* macros and select() on a high fd

 tools/include/nolibc/dirent.h                |  7 +--
 tools/include/nolibc/sys/select.h            |  6 +--
 tools/testing/selftests/nolibc/nolibc-test.c | 54 ++++++++++++++++++++
 3 files changed, 61 insertions(+), 6 deletions(-)


base-commit: f2212892b6a0c2adab438783e9fa8093c90786d7
-- 
2.55.0

[PATCH 1/3] tools/nolibc: fix readdir_r() with 64-bit directory offsets

[Danish Khateeb]

readdir_r() stores the result of _sys_lseek() in an int. Directory
offsets are opaque cookies which can use all 64 bits: ext4, for
instance, gives 64-bit processes 63-bit hashes, and
0x7fffffffffffffff as the offset after the last entry. Truncated to an
int, such an offset is negative about half the time, and readdir_r()
takes it for an error.

Since commit 4ada5679f18d ("tools/nolibc/dirent: avoid errno in
readdir_r"), readdir_r() fails at the first entry whose offset has
bit 31 set, which on ext4 is usually one of the first few, and returns
the truncated offset as the error number. Before that, only -1 counted
as an error, which the last entry always hits: readdir_r() then
returned errno, usually 0, without filling in the entry, so the caller
got the previous entry a second time and never saw the last one.

32-bit processes get 31-bit hashes from ext4 and are not affected.

Keep the offset in an off_t.

Fixes: 665fa8dea90d ("tools/nolibc: add support for directory access")
Cc: stable@vger.kernel.org # v6.15+
Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/include/nolibc/dirent.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/include/nolibc/dirent.h b/tools/include/nolibc/dirent.h
index 2dbf4052b85a..19857369de2a 100644
--- a/tools/include/nolibc/dirent.h
+++ b/tools/include/nolibc/dirent.h
@@ -81,6 +81,7 @@ int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result)
 	struct linux_dirent64 *ldir = (void *)buf;
 	intptr_t i = (intptr_t)dirp;
 	int fd, ret;
+	off_t off;
 
 	if (i >= 0)
 		return EBADF;
@@ -100,9 +101,9 @@ int readdir_r(DIR *dirp, struct dirent *entry, struct dirent **result)
 	 * readdir() can only return one entry at a time.
 	 * Make sure the non-returned ones are not skipped.
 	 */
-	ret = _sys_lseek(fd, ldir->d_off, SEEK_SET);
-	if (ret < 0)
-		return -ret;
+	off = _sys_lseek(fd, ldir->d_off, SEEK_SET);
+	if (off < 0)
+		return -off;
 
 	entry->d_ino = ldir->d_ino;
 	/* the destination should always be big enough */
-- 
2.55.0

[PATCH 2/3] tools/nolibc: fix FD_SET(), FD_CLR() and FD_ISSET() on 64-bit

[Danish Khateeb]

Since commit feaf75658783 ("nolibc: fix fd_set type"), an fd_set is an
array of unsigned long, but the FD_* macros still build their masks as
an int or an unsigned int: 1 << n in FD_SET(), 1U << n in FD_CLR() and
FD_ISSET(), where n is the fd modulo 64 on 64-bit architectures. So:

- FD_SET() of an fd with n == 31 sets the bits for n = 31-63, as
  1 << 31 is negative and gets sign-extended.
- For n >= 32 the shifts are undefined. x86-64, for instance, masks
  the shift count, so FD_SET(40) and FD_ISSET(40) use the bit of fd 8.
- FD_CLR() zero-extends ~(1U << n), so it also clears the bits for
  n = 32-63, whichever fd it is given.

select() on fd 40, for instance, fails with EBADF, or watches fd 8
instead if that one is open.

Build the masks from 1UL.

Fixes: feaf75658783 ("nolibc: fix fd_set type")
Cc: stable@vger.kernel.org # v6.2+
Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/include/nolibc/sys/select.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/include/nolibc/sys/select.h b/tools/include/nolibc/sys/select.h
index 6d65d9ef3d6a..f299c79939f5 100644
--- a/tools/include/nolibc/sys/select.h
+++ b/tools/include/nolibc/sys/select.h
@@ -26,7 +26,7 @@ typedef struct {
 		int __fd = (fd);					\
 		if (__fd >= 0)						\
 			__set->fds[__fd / FD_SETIDXMASK] &=		\
-				~(1U << (__fd & FD_SETBITMASK));	\
+				~(1UL << (__fd & FD_SETBITMASK));	\
 	} while (0)
 
 #define FD_SET(fd, set) do {						\
@@ -34,7 +34,7 @@ typedef struct {
 		int __fd = (fd);					\
 		if (__fd >= 0)						\
 			__set->fds[__fd / FD_SETIDXMASK] |=		\
-				1 << (__fd & FD_SETBITMASK);		\
+				1UL << (__fd & FD_SETBITMASK);		\
 	} while (0)
 
 #define FD_ISSET(fd, set) ({						\
@@ -43,7 +43,7 @@ typedef struct {
 		int __r = 0;						\
 		if (__fd >= 0)						\
 			__r = !!(__set->fds[__fd / FD_SETIDXMASK] &	\
-1U << (__fd & FD_SETBITMASK));						\
+1UL << (__fd & FD_SETBITMASK));						\
 		__r;							\
 	})
 
-- 
2.55.0

[PATCH 3/3] selftests/nolibc: test the FD_* macros and select() on a high fd

[Danish Khateeb]

The only select() test that passes an fd_set uses fd 1, so nothing
noticed that the FD_* macros used the wrong bits for fds 31-63 of each
word on 64-bit architectures.

Add a test that FD_SET() and FD_CLR() change exactly one fd, for every
fd below FD_SETSIZE, and one that select() reports a readable pipe
duplicated to fd 40, which also checks that the macros agree with the
kernel on the layout of the set.

Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 54 ++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index f62183a46205..d9a8bfd4b21b 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1369,6 +1369,58 @@ int test_pipe(void)
 	return !!memcmp(buf, msg, len);
 }
 
+int test_fd_set(void)
+{
+	fd_set fds;
+	int fd, i;
+
+	for (fd = 0; fd < FD_SETSIZE; fd++) {
+		/* FD_SET() must set this fd and no other */
+		FD_ZERO(&fds);
+		FD_SET(fd, &fds);
+		for (i = 0; i < FD_SETSIZE; i++)
+			if (!!FD_ISSET(i, &fds) != (i == fd))
+				return 1;
+
+		/* FD_CLR() must clear this fd and no other */
+		for (i = 0; i < FD_SETSIZE; i++)
+			FD_SET(i, &fds);
+		FD_CLR(fd, &fds);
+		for (i = 0; i < FD_SETSIZE; i++)
+			if (!!FD_ISSET(i, &fds) != (i != fd))
+				return 1;
+	}
+
+	return 0;
+}
+
+int test_select_high_fd(void)
+{
+	struct timeval tv = { 0 };
+	int pipefd[2], fd, ret = 1;
+	fd_set fds;
+
+	if (pipe(pipefd) == -1)
+		return 1;
+
+	/* fd 40 is in the upper half of a 64-bit fd_set word */
+	fd = dup2(pipefd[0], 40);
+	if (fd == -1)
+		goto out;
+
+	write(pipefd[1], "x", 1);
+	FD_ZERO(&fds);
+	FD_SET(fd, &fds);
+	if (select(fd + 1, &fds, NULL, NULL, &tv) == 1 && FD_ISSET(fd, &fds))
+		ret = 0;
+
+	close(fd);
+out:
+	close(pipefd[0]);
+	close(pipefd[1]);
+	return ret;
+}
+
 int test_rlimit(void)
 {
 	struct rlimit rlim = {
@@ -1711,6 +1763,7 @@ int run_syscall(int min, int max)
 		CASE_TEST(execve_root);       EXPECT_SYSER(1, execve("/", (char*[]){ [0] = (char []){"/"}, [1] = NULL }, NULL), -1, EACCES); break;
 		CASE_TEST(fchdir_stdin);      EXPECT_SYSER(1, fchdir(STDIN_FILENO), -1, ENOTDIR); break;
 		CASE_TEST(fchdir_badfd);      EXPECT_SYSER(1, fchdir(-1), -1, EBADF); break;
+		CASE_TEST(fd_set);            EXPECT_ZR(1, test_fd_set()); break;
 		CASE_TEST(fdopendir_notdir);  EXPECT_SYSER(1, (uintptr_t)fdopendir(STDIN_FILENO), (uintptr_t)NULL, ENOTDIR); break;
 		CASE_TEST(file_stream);       EXPECT_SYSZR(1, test_file_stream()); break;
 		CASE_TEST(file_stream_wsr);   EXPECT_SYSZR(1, test_file_stream_wsr()); break;
@@ -1753,6 +1806,7 @@ int run_syscall(int min, int max)
 		CASE_TEST(select_null);       EXPECT_SYSZR(1, ({ struct timeval tv = { 0 }; select(0, NULL, NULL, NULL, &tv); })); break;
 		CASE_TEST(select_stdout);     EXPECT_SYSNE(1, ({ fd_set fds; FD_ZERO(&fds); FD_SET(1, &fds); select(2, NULL, &fds, NULL, NULL); }), -1); break;
 		CASE_TEST(select_fault);      EXPECT_SYSER(1, select(1, (void *)1, NULL, NULL, 0), -1, EFAULT); break;
+		CASE_TEST(select_high_fd);    EXPECT_SYSZR(1, test_select_high_fd()); break;
 		CASE_TEST(sendfile);          EXPECT_SYSZR(1, test_sendfile()); break;
 		CASE_TEST(stat_blah);         EXPECT_SYSER(1, stat("/proc/self/blah", &stat_buf), -1, ENOENT); break;
 		CASE_TEST(stat_fault);        EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break;
-- 
2.55.0