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

[PATCH v2 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 a test 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.

Changes in v2:
- Dropped Cc: stable from patches 1 and 2, which are otherwise
  unchanged.
- Patch 3: dropped the select() test, and moved the FD_* test to the
  stdlib tests, with its helper in the same order as the test cases
  (Thomas). It is also shorter: FD_CLR() is now checked on a set
  filled with memset().
- Rebased on nolibc/for-next 6f114ae7b600.

v1: https://lore.kernel.org/all/20260924015222.31693-1-danishkhateeb03@gmail.com/

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 new fd_set test, which passes, also with clang
  on x86_64 and against glibc (make libc-test).

- Without patch 2, fd_set fails on the 64-bit architectures: with the
  default UBSan flags the run stops at fd_set on the sanitizer trap,
  and without them the test reports [FAIL] (checked on x86_64 and
  arm64). It passes on i386 and arm, which are not affected.

- The readdir_r() program, on ext4, on x86_64, arm64 and sparc64:
  before, it fails on /etc after 4 of 215 entries, /usr/include after
  1 of 1467 and /usr/lib after 2 of 6229. After, it lists as many
  entries as glibc. A native i386 build was already correct.

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

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

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


base-commit: 6f114ae7b6006ca7cf72e3e3348d263bbfec4525
-- 
2.55.0

[PATCH v2 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")
Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---

Notes:
    v2: No changes except dropping Cc: stable.
    
    v1: https://lore.kernel.org/all/20260924015222.31693-2-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 v2 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")
Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---

Notes:
    v2: No changes except dropping Cc: stable.
    
    v1: https://lore.kernel.org/all/20260924015222.31693-3-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 v2 3/3] selftests/nolibc: test the FD_* macros

[Danish Khateeb]

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

Check that FD_SET() and FD_CLR() change exactly one fd, for every fd
below FD_SETSIZE.

Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
---

Notes:
    v2:
    - Dropped the select() test.
    - Moved the FD_* test to the stdlib tests, with its helper in the same
      order as the test cases.
    - Shortened it: FD_CLR() is now checked on a set filled with memset().
    
    v1: https://lore.kernel.org/all/20260924015222.31693-4-danishkhateeb03@gmail.com/
    
    Tested with nolibc-test on x86_64 (GCC and clang), i386, arm, arm64 and
    sparc64 (qemu-user), and against glibc. With the fix, fd_set passes
    everywhere. Without it, fd_set fails on the 64-bit architectures (or
    UBSan traps on the shift) and passes on the 32-bit ones, which were not
    affected.

 tools/testing/selftests/nolibc/nolibc-test.c | 21 ++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 37c11a7fce23..1995d67dcca6 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1841,6 +1841,26 @@ int test_time_types(void)
 	return 0;
 }
 
+int test_fd_set(void)
+{
+	fd_set set, clr;
+	int fd, i;
+
+	/* FD_SET() and FD_CLR() must change exactly one fd */
+	for (fd = 0; fd < FD_SETSIZE; fd++) {
+		FD_ZERO(&set);
+		FD_SET(fd, &set);
+		memset(&clr, 0xff, sizeof(clr));
+		FD_CLR(fd, &clr);
+		for (i = 0; i < FD_SETSIZE; i++)
+			if (!!FD_ISSET(i, &set) != (i == fd) ||
+			    !!FD_ISSET(i, &clr) != (i != fd))
+				return 1;
+	}
+
+	return 0;
+}
+
 int test_malloc(void)
 {
 	size_t sz_array1, sz_array2, sz_array3;
@@ -2022,6 +2042,7 @@ int run_stdlib(int min, int max)
 		CASE_TEST(memchr_foobar6_o);        EXPECT_STREQ(1, memchr("foobar", 'o', 6), "oobar"); break;
 		CASE_TEST(memchr_foobar3_b);        EXPECT_STRZR(1, memchr("foobar", 'b', 3)); break;
 		CASE_TEST(time_types);              EXPECT_ZR(is_nolibc, test_time_types()); break;
+		CASE_TEST(fd_set);                  EXPECT_ZR(1, test_fd_set()); break;
 		CASE_TEST(makedev);                 EXPECT_EQ(1, makedev(0x12, 0x34), 0x1234); break;
 		CASE_TEST(major);                   EXPECT_EQ(1, major(0x1234), 0x12); break;
 		CASE_TEST(minor);                   EXPECT_EQ(1, minor(0x1234), 0x34); break;
-- 
2.55.0