diff mbox series

[pseudo,1/3] pseudo_client: step fully past pseudo's own fds when computing startfd

Message ID 20260718043750.7-2-bbnpreetsingh@gmail.com
State New
Headers show
Series closefrom/close_range: protect every pseudo fd, then drop one | expand

Commit Message

Babanpreet Singh July 18, 2026, 4:37 a.m. UTC
OP_CLOSEFROM and OP_CLOSE_RANGE compute startfd, the first descriptor
the caller may hand to the kernel wholesale, by stepping past each
descriptor pseudo needs to keep:

	if (connect_fd > startfd)
		startfd = connect_fd + 1;

A protected descriptor exactly equal to the running startfd fails that
test and ends up protected by nothing: the close-by-hand loop stops
below startfd, the kernel sweep starts at startfd, and the descriptor
is closed even though it is on the protect list.

The default descriptor layout hits this. Client init opens the local
state directory fd and then connects to the server, so the two sit on
consecutive numbers, say 21 and 22: the local state fd raises startfd
to 22, connect_fd == 22 fails the > test, and closefrom(3) or
close_range(3, ~0U, 0) hands 22 straight to the kernel. The server
connection is closed out from under the client.

The client transparently reconnects on the next operation, which is
why this has gone unnoticed, but until that reconnect the stale
descriptor number is free for the process to reuse, and anything
pseudo believes about the connection is attached to the wrong
descriptor.

Use >= so startfd always ends up above every protected descriptor.

Present since the startfd computation was introduced in 21ff2fb
("ports/linux/guts: Add closefrom support for glibc 2.34").

AI-Generated: Uses Claude (claude-sonnet-5)
Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com>
---
 pseudo_client.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/pseudo_client.c b/pseudo_client.c
index 1acd948..085a8b3 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -1966,15 +1966,15 @@  pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	case OP_CLOSEFROM:
 		/* no request needed */
 		startfd = fd;
-		if (pseudo_util_debug_fd > startfd)
+		if (pseudo_util_debug_fd >= startfd)
 			startfd = pseudo_util_debug_fd + 1;
-		if (pseudo_localstate_dir_fd > startfd)
+		if (pseudo_localstate_dir_fd >= startfd)
 			startfd = pseudo_localstate_dir_fd + 1;
-		if (pseudo_pwd_fd > startfd)
+		if (pseudo_pwd_fd >= startfd)
 			startfd = pseudo_pwd_fd + 1;
-		if (pseudo_grp_fd > startfd)
+		if (pseudo_grp_fd >= startfd)
 			startfd = pseudo_grp_fd + 1;
-		if (connect_fd > startfd)
+		if (connect_fd >= startfd)
 			startfd = connect_fd + 1;
 		for (i = fd; i < startfd; ++i) {
 			if (i == pseudo_util_debug_fd || i == pseudo_localstate_dir_fd || i == pseudo_pwd_fd ||
@@ -1992,15 +1992,15 @@  pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	case OP_CLOSE_RANGE:
 		/* no request needed */
 		startfd = fd;
-		if (pseudo_util_debug_fd > startfd)
+		if (pseudo_util_debug_fd >= startfd)
 			startfd = pseudo_util_debug_fd + 1;
-		if (pseudo_localstate_dir_fd > startfd)
+		if (pseudo_localstate_dir_fd >= startfd)
 			startfd = pseudo_localstate_dir_fd + 1;
-		if (pseudo_pwd_fd > startfd)
+		if (pseudo_pwd_fd >= startfd)
 			startfd = pseudo_pwd_fd + 1;
-		if (pseudo_grp_fd > startfd)
+		if (pseudo_grp_fd >= startfd)
 			startfd = pseudo_grp_fd + 1;
-		if (connect_fd > startfd)
+		if (connect_fd >= startfd)
 			startfd = connect_fd + 1;
 		/* the fds below startfd are the ones our own are mixed in
 		 * with, so close those by hand and skip the ones we need