| Message ID | 20260716055633.7-2-bbnpreetsingh@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | close_range: implement it rather than return ENOSYS | expand |
On Thu, 2026-07-16 at 05:56 +0000, Babanpreet Singh wrote: > The close_range() wrapper added in 35433e6 ("ports/linux/guts: Add > close_range wrapper for glibc 2.34") returns ENOSYS on the assumption > that callers handle that. systemd v260 no longer does: it removed its > /proc/self/fd fallback and treats a close_range() failure as fatal, so > every fork+exec under pseudo aborts: > > Failed to close all file descriptors: Function not implemented > '(mkfs)' failed with exit status 1. > > A client side op closes the low descriptors one at a time, skipping the > ones pseudo needs for itself, and returns the first fd above pseudo's > own so the caller can pass the rest of the range to the kernel directly. > > CLOSE_RANGE_UNSHARE is handled by calling unshare(CLONE_FILES) before > closing anything, so the closes only affect the caller and not other > processes sharing the descriptor table. Unknown flags and an inverted > range are rejected with EINVAL before anything is closed. > > A range starting entirely above INT_MAX cannot contain any of pseudo's > fds and is also passed straight through. > > [YOCTO #16339] > > AI-Generated: Uses Claude (claude-opus-4-8) > Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com> > --- > enums/op.in | 1 + > ports/linux/guts/close_range.c | 54 ++++++++++++++++++++++++++---- > ports/linux/portdefs.h | 16 +++++++++ > pseudo_client.c | 60 ++++++++++++++++++++++++++++++++++ > 4 files changed, 124 insertions(+), 7 deletions(-) Unfortunately there is something wrong in this patch. I added this series and your other series fixing the closed fds and with this patch present, we see failures: https://autobuilder.yoctoproject.org/valkyrie/#/builders/50/builds/4259 i.e. linux-libc-headers do_install fails, which is one of the first pieces of the OE build to use pseudo. This should be reproducible in a OE build with "bitbake linux-libc-headers -c install", with a patch like the one below applied to update pseudo. As soon as I revert this patch, the builds work ok. I also tested with the second series applied without this patch (patches tweaked to apply) and that also seems to build ok, so it would appear to be something in this change. I've not dug into it in detail yet but I wanted to share the issue now I've isolated it. Cheers, Richard diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb index 9f02d90568..01a66ae21e 100644 --- a/meta/recipes-devtools/pseudo/pseudo_git.bb +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb @@ -1,6 +1,6 @@ require pseudo.inc -SRC_URI = "git://git.yoctoproject.org/pseudo;branch=master;protocol=https;branch=master \ +SRC_URI = "git://git.yoctoproject.org/pseudo;branch=master-next;protocol=https \ file://fallback-passwd \ file://fallback-group \ " @@ -12,8 +12,8 @@ SRC_URI:append:class-nativesdk = " \ file://older-glibc-symbols.patch" SRC_URI[prebuilt.sha256sum] = "ed9f456856e9d86359f169f46a70ad7be4190d6040282b84c8d97b99072485aa" -SRCREV = "ca47829825f297d7bf83665c0541a9de4aa78009" -PV = "1.9.8+git" +SRCREV = "7f509ca924d27109335c4575d1fb67745c7b3abd" +PV = "1.9.9+git" # largefile and 64bit time_t support adds these macros via compiler flags globally # remove them for pseudo since pseudo intercepts some of the functions which will be
On Tue, 2026-07-28 at 10:22 +0100, Richard Purdie via lists.yoctoproject.org wrote: > On Thu, 2026-07-16 at 05:56 +0000, Babanpreet Singh wrote: > > The close_range() wrapper added in 35433e6 ("ports/linux/guts: Add > > close_range wrapper for glibc 2.34") returns ENOSYS on the assumption > > that callers handle that. systemd v260 no longer does: it removed its > > /proc/self/fd fallback and treats a close_range() failure as fatal, so > > every fork+exec under pseudo aborts: > > > > Failed to close all file descriptors: Function not implemented > > '(mkfs)' failed with exit status 1. > > > > A client side op closes the low descriptors one at a time, skipping the > > ones pseudo needs for itself, and returns the first fd above pseudo's > > own so the caller can pass the rest of the range to the kernel directly. > > > > CLOSE_RANGE_UNSHARE is handled by calling unshare(CLONE_FILES) before > > closing anything, so the closes only affect the caller and not other > > processes sharing the descriptor table. Unknown flags and an inverted > > range are rejected with EINVAL before anything is closed. > > > > A range starting entirely above INT_MAX cannot contain any of pseudo's > > fds and is also passed straight through. > > > > [YOCTO #16339] > > > > AI-Generated: Uses Claude (claude-opus-4-8) > > Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com> > > --- > > enums/op.in | 1 + > > ports/linux/guts/close_range.c | 54 ++++++++++++++++++++++++++---- > > ports/linux/portdefs.h | 16 +++++++++ > > pseudo_client.c | 60 ++++++++++++++++++++++++++++++++++ > > 4 files changed, 124 insertions(+), 7 deletions(-) > > Unfortunately there is something wrong in this patch. I added this > series and your other series fixing the closed fds and with this patch > present, we see failures: > > https://autobuilder.yoctoproject.org/valkyrie/#/builders/50/builds/4259 > > i.e. linux-libc-headers do_install fails, which is one of the first > pieces of the OE build to use pseudo. This should be reproducible in a > OE build with "bitbake linux-libc-headers -c install", with a patch > like the one below applied to update pseudo. > > As soon as I revert this patch, the builds work ok. I also tested with > the second series applied without this patch (patches tweaked to apply) > and that also seems to build ok, so it would appear to be something in > this change. > > I've not dug into it in detail yet but I wanted to share the issue now > I've isolated it. I had a look at the code and had a theory about the type conversions on maxfd being an issue. This patch appears to help: diff --git a/ports/linux/guts/close_range.c b/ports/linux/guts/close_range.c index c615148..784d066 100644 --- a/ports/linux/guts/close_range.c +++ b/ports/linux/guts/close_range.c @@ -7,6 +7,7 @@ * int rc = -1; */ pseudo_msg_t *msg; + int maxintfd; /* The kernel rejects both of these outright and closes nothing when * it does, so validate before touching anything. @@ -43,17 +44,21 @@ */ if (lowfd > INT_MAX) return real_close_range(lowfd, maxfd, flags); + if (maxfd > INT_MAX) + maxintfd = INT_MAX; + else + maxintfd = (int) maxfd; /* Same shape as closefrom(): the client op closes the descriptors its * own are mixed in with by hand, stepping around the ones pseudo needs * to keep, and hands back the first fd the kernel can safely be turned * loose on. */ - //msg = pseudo_client_op(OP_CLOSE_RANGE, 0, lowfd, -1, 0, 0, maxfd); - //if (maxfd >= (unsigned int) msg->fd) - // rc = real_close_range(msg->fd, maxfd, flags); - //else - // rc = 0; + msg = pseudo_client_op(OP_CLOSE_RANGE, 0, lowfd, -1, 0, 0, maxintfd); + if (maxfd >= (unsigned int) msg->fd) + rc = real_close_range(msg->fd, maxfd, flags); + else + rc = 0; errno = ENOSYS; rc = -1; diff --git a/pseudo_client.c b/pseudo_client.c index 98112bc..e846dc9 100644 --- a/pseudo_client.c +++ b/pseudo_client.c @@ -1653,7 +1653,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path static size_t alloced_len = 0; int strip_slash; int startfd, i; - unsigned int close_range_maxfd = 0; + int close_range_maxfd = 0; #ifdef PSEUDO_PROFILING struct timeval tv1_op, tv2_op; @@ -1777,7 +1777,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path if (op == OP_CLOSE_RANGE) { va_list ap; va_start(ap, buf); - close_range_maxfd = va_arg(ap, unsigned int); + close_range_maxfd = va_arg(ap, int); va_end(ap); } @@ -2017,7 +2017,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path /* the fds below startfd are the ones our own are mixed in * with, so close those by hand and skip the ones we need */ - for (i = fd; i < startfd && (unsigned int) i <= close_range_maxfd; ++i) { + for (i = fd; i < startfd && i <= close_range_maxfd; ++i) { if (i == pseudo_util_debug_fd || i == pseudo_util_evlog_fd || i == pseudo_prefix_dir_fd || i == pseudo_localstate_dir_fd || i == pseudo_pwd_fd || i == pseudo_pwd_lck_fd || @@ -2026,7 +2026,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path pseudo_client_close(i); close(i); } - if (close_range_maxfd >= (unsigned int) startfd) + if (close_range_maxfd >= startfd) pseudo_client_close_range(startfd, close_range_maxfd); /* tell the caller to start at startfd instead of fd */ result = &msg; which is I suspect because there is code doing: close_range(3, ~0U, xxx); Whether we should just force maxfd to INT_MAX always, I'm not sure. I don't think the high fds are usable. Cheers, Richard
On Tue, 2026-07-28 at 11:15 +0100, Richard Purdie via lists.yoctoproject.org wrote: > close_range(3, ~0U, xxx); > > Whether we should just force maxfd to INT_MAX always, I'm not sure. I > don't think the high fds are usable. Checking into the kernel, it would never handle fds between INT_MAX and ~0U so I think we can just clamp to INT_MAX for our code. Also, I don't think pseudo needs both OP_CLOSEFROM and OP_CLOSERANGE. I'd propose we just add a max parameter to OP_CLOSEFROM so it effectively becomes OP_CLOSERANGE, I'm fine with renaming it too. We can then use it from close_from with INT_MAX as the upper limit. Are you ok to tweak the patches like that, handle the type conversions safely, add a test for close_range(3, ~0U, xxx) working (with ~0U) and then resend? Cheers, Richard
diff --git a/enums/op.in b/enums/op.in index 5b5e21b..5013892 100644 --- a/enums/op.in +++ b/enums/op.in @@ -28,3 +28,4 @@ set-xattr, 0 create-xattr, 1 replace-xattr, 1 closefrom, 0 +close-range, 0 diff --git a/ports/linux/guts/close_range.c b/ports/linux/guts/close_range.c index 4bd2fe1..c3ca71d 100644 --- a/ports/linux/guts/close_range.c +++ b/ports/linux/guts/close_range.c @@ -6,14 +6,54 @@ * int close_range(unsigned int lowfd, unsigned int maxfd, int flags) * int rc = -1; */ + pseudo_msg_t *msg; - (void) lowfd; - (void) maxfd; - (void) flags; - /* for now pretend the kernel doesn't support it regardless - which users are supposed to be able to handle */ - errno = ENOSYS; - rc = -1; + /* The kernel rejects both of these outright and closes nothing when + * it does, so validate before touching anything. + */ + if (flags & ~(CLOSE_RANGE_UNSHARE | CLOSE_RANGE_CLOEXEC)) { + errno = EINVAL; + return -1; + } + if (lowfd > maxfd) { + errno = EINVAL; + return -1; + } + + /* CLOSE_RANGE_UNSHARE has to take effect before anything is closed: + * while the descriptor table is still shared, closing a descriptor + * would close it for everyone sharing the table, not just for us. + */ + if (flags & CLOSE_RANGE_UNSHARE) { + if (unshare(CLONE_FILES) == -1) + return -1; + flags &= ~CLOSE_RANGE_UNSHARE; + } + + /* CLOSE_RANGE_CLOEXEC closes nothing, it only marks descriptors, and + * pseudo's own are close-on-exec already (pseudo_fd() sets that on + * every one of them), so there is nothing here to protect. + */ + if (flags & CLOSE_RANGE_CLOEXEC) + return real_close_range(lowfd, maxfd, flags); + + /* Descriptors are ints, so a range starting above INT_MAX cannot hold + * any of pseudo's own and there is nothing to step around. Worth its + * own case because pseudo_client_op() takes the low end as an int. + */ + if (lowfd > INT_MAX) + return real_close_range(lowfd, maxfd, flags); + + /* Same shape as closefrom(): the client op closes the descriptors its + * own are mixed in with by hand, stepping around the ones pseudo needs + * to keep, and hands back the first fd the kernel can safely be turned + * loose on. + */ + msg = pseudo_client_op(OP_CLOSE_RANGE, 0, lowfd, -1, 0, 0, maxfd); + if (maxfd >= (unsigned int) msg->fd) + rc = real_close_range(msg->fd, maxfd, flags); + else + rc = 0; /* return rc; * } diff --git a/ports/linux/portdefs.h b/ports/linux/portdefs.h index 19bb232..1f1a41a 100644 --- a/ports/linux/portdefs.h +++ b/ports/linux/portdefs.h @@ -35,6 +35,22 @@ GLIBC_COMPAT_SYMBOL(memcpy,2.0); #include <sys/prctl.h> #include <linux/seccomp.h> +/* close_range()'s flags, and unshare(), are only declared by glibc under + * _GNU_SOURCE, which pseudo does not build with. <linux/close_range.h> is + * not an option either: it is absent on hosts with pre-5.9 kernel headers, + * the same problem SYS_openat2 has below. Both values are kernel ABI. + */ +#ifndef CLOSE_RANGE_UNSHARE +#define CLOSE_RANGE_UNSHARE (1U << 1) +#endif +#ifndef CLOSE_RANGE_CLOEXEC +#define CLOSE_RANGE_CLOEXEC (1U << 2) +#endif +#ifndef CLONE_FILES +#define CLONE_FILES 0x00000400 +#endif +extern int unshare(int flags); + #ifndef _STAT_VER #if defined (__aarch64__) || defined (__riscv) #define _STAT_VER 0 diff --git a/pseudo_client.c b/pseudo_client.c index 7041366..1acd948 100644 --- a/pseudo_client.c +++ b/pseudo_client.c @@ -993,6 +993,28 @@ pseudo_client_closefrom(int fd) { } } +/* Like pseudo_client_closefrom(), but bounded: close_range() has a top end, + * so entries above it have to be left alone. + */ +static void +pseudo_client_close_range(int lowfd, unsigned int maxfd) { + int i, top; + + if (lowfd < 0 || lowfd >= nfds) + return; + + top = (maxfd >= (unsigned int) nfds) ? nfds - 1 : (int) maxfd; + for (i = lowfd; i <= top; ++i) { + free(fd_paths[i]); + fd_paths[i] = 0; + + if (i < linked_nfds) { + free(linked_fd_paths[i]); + linked_fd_paths[i] = 0; + } + } +} + /* spawn server */ static int client_spawn_server(void) { @@ -1633,6 +1655,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path static size_t alloced_len = 0; int strip_slash; int startfd, i; + unsigned int close_range_maxfd = 0; #ifdef PSEUDO_PROFILING struct timeval tv1_op, tv2_op; @@ -1753,6 +1776,13 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path } #endif + if (op == OP_CLOSE_RANGE) { + va_list ap; + va_start(ap, buf); + close_range_maxfd = va_arg(ap, unsigned int); + va_end(ap); + } + if (op == OP_RENAME) { va_list ap; if (!path) { @@ -1959,6 +1989,36 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path msg.fd = startfd; do_request = 0; break; + case OP_CLOSE_RANGE: + /* no request needed */ + startfd = fd; + if (pseudo_util_debug_fd > startfd) + startfd = pseudo_util_debug_fd + 1; + if (pseudo_localstate_dir_fd > startfd) + startfd = pseudo_localstate_dir_fd + 1; + if (pseudo_pwd_fd > startfd) + startfd = pseudo_pwd_fd + 1; + if (pseudo_grp_fd > startfd) + startfd = pseudo_grp_fd + 1; + 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 + */ + for (i = fd; i < startfd && (unsigned int) i <= close_range_maxfd; ++i) { + if (i == pseudo_util_debug_fd || i == pseudo_localstate_dir_fd || i == pseudo_pwd_fd || + i == pseudo_grp_fd || i == connect_fd) + continue; + pseudo_client_close(i); + close(i); + } + if (close_range_maxfd >= (unsigned int) startfd) + pseudo_client_close_range(startfd, close_range_maxfd); + /* tell the caller to start at startfd instead of fd */ + result = &msg; + msg.fd = startfd; + do_request = 0; + break; case OP_CLOSE: /* no request needed */ if (fd >= 0) {
The close_range() wrapper added in 35433e6 ("ports/linux/guts: Add close_range wrapper for glibc 2.34") returns ENOSYS on the assumption that callers handle that. systemd v260 no longer does: it removed its /proc/self/fd fallback and treats a close_range() failure as fatal, so every fork+exec under pseudo aborts: Failed to close all file descriptors: Function not implemented '(mkfs)' failed with exit status 1. A client side op closes the low descriptors one at a time, skipping the ones pseudo needs for itself, and returns the first fd above pseudo's own so the caller can pass the rest of the range to the kernel directly. CLOSE_RANGE_UNSHARE is handled by calling unshare(CLONE_FILES) before closing anything, so the closes only affect the caller and not other processes sharing the descriptor table. Unknown flags and an inverted range are rejected with EINVAL before anything is closed. A range starting entirely above INT_MAX cannot contain any of pseudo's fds and is also passed straight through. [YOCTO #16339] AI-Generated: Uses Claude (claude-opus-4-8) Signed-off-by: Babanpreet Singh <bbnpreetsingh@gmail.com> --- enums/op.in | 1 + ports/linux/guts/close_range.c | 54 ++++++++++++++++++++++++++---- ports/linux/portdefs.h | 16 +++++++++ pseudo_client.c | 60 ++++++++++++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 7 deletions(-)