| Message ID | 20260716055633.7-1-bbnpreetsingh@gmail.com |
|---|---|
| Headers | show |
| Series | close_range: implement it rather than return ENOSYS | expand |
On Thu, 2026-07-16 at 05:56 +0000, Babanpreet Singh wrote: > pseudo's close_range() wrapper has returned ENOSYS unconditionally since > 35433e6 ("ports/linux/guts: Add close_range wrapper for glibc 2.34"). > systemd v260 deleted its /proc/self/fd fallback and treats a failure as > fatal, so under pseudo every fork+exec dies. That is [YOCTO #16339]. Full > story in v1: > > https://lore.kernel.org/yocto-patches/20260715054142.7-1-bbnpreetsingh@gmail.com/ I had a quick look through this and it looks ok to me, thanks! > The v1 question about OP_CLOSEFROM stepping around five of pseudo's > descriptors while others exist (pseudo_prefix_dir_fd, pseudo_pwd_lck_fd, > pseudo_util_evlog_fd) still stands; happy to send a follow-up if that list > is short in both places. I had a quick look and yes, we should really be protecting these too. That said, I couldn't see what pseudo_prefix_dir_fd actually helps with and I couldn't see why we need that... Cheers, Richard
Thanks! > That said, I couldn't see what pseudo_prefix_dir_fd actually helps with > and I couldn't see why we need that... I flagged it since it is held open and a close_range() sweep will wipe it out. I hadn't checked if anything still reads it untill now. It seems like it is just kept open and alive and the socket connection + fchdir() has been moved to pseudo_localstate_dir_fd. So, for the follow-up I'd purpose: 1/2: remove pseudo_prefix_dir_fd 2/2: add pseudo_pwd_lck_fd and pseudo_util_evlog_fd to the descriptors OP_CLOSEFROM and OP_CLOSE_RANGE step around If you'd rather keep the fd, I'll drop the removal and add it to the protected set in 2/2 instead. Thanks for reviewing, Baban. On Thu, 16 Jul 2026 at 03:46, Richard Purdie < richard.purdie@linuxfoundation.org> wrote: > On Thu, 2026-07-16 at 05:56 +0000, Babanpreet Singh wrote: > > pseudo's close_range() wrapper has returned ENOSYS unconditionally since > > 35433e6 ("ports/linux/guts: Add close_range wrapper for glibc 2.34"). > > systemd v260 deleted its /proc/self/fd fallback and treats a failure as > > fatal, so under pseudo every fork+exec dies. That is [YOCTO #16339]. Full > > story in v1: > > > > > https://lore.kernel.org/yocto-patches/20260715054142.7-1-bbnpreetsingh@gmail.com/ > > I had a quick look through this and it looks ok to me, thanks! > > > The v1 question about OP_CLOSEFROM stepping around five of pseudo's > > descriptors while others exist (pseudo_prefix_dir_fd, pseudo_pwd_lck_fd, > > pseudo_util_evlog_fd) still stands; happy to send a follow-up if that > list > > is short in both places. > > I had a quick look and yes, we should really be protecting these too. > > That said, I couldn't see what pseudo_prefix_dir_fd actually helps with > and I couldn't see why we need that... > > Cheers, > > Richard >
On Thu, 2026-07-16 at 08:46 -0700, Babanpreet Singh wrote: > Thanks! > > > That said, I couldn't see what pseudo_prefix_dir_fd actually helps with > > and I couldn't see why we need that... > > I flagged it since it is held open and a close_range() sweep will wipe it out. I hadn't checked if anything still reads it untill now. It seems like it is just kept open and alive and the socket connection + fchdir() has been moved to pseudo_localstate_dir_fd. > > So, for the follow-up I'd purpose: > > 1/2: remove pseudo_prefix_dir_fd > 2/2: add pseudo_pwd_lck_fd and pseudo_util_evlog_fd to the descriptors OP_CLOSEFROM and OP_CLOSE_RANGE step around > > If you'd rather keep the fd, I'll drop the removal and add it to the protected set in 2/2 instead. I'd suggest making the series the other way around, protect it for now, send a removal patch on top and we can look at the removal patch and test and check it isn't really used anywhere. It is possible I'm missing some usage of it and I'm curious what Mark thinks. The protection patch makes sense and should be easy to merge compared to that. Cheers, Richard
On 7/16/26 5:46 AM, Richard Purdie via lists.yoctoproject.org wrote: > On Thu, 2026-07-16 at 05:56 +0000, Babanpreet Singh wrote: >> pseudo's close_range() wrapper has returned ENOSYS unconditionally since >> 35433e6 ("ports/linux/guts: Add close_range wrapper for glibc 2.34"). >> systemd v260 deleted its /proc/self/fd fallback and treats a failure as >> fatal, so under pseudo every fork+exec dies. That is [YOCTO #16339]. Full >> story in v1: >> >> https://lore.kernel.org/yocto-patches/20260715054142.7-1-bbnpreetsingh@gmail.com/ > > I had a quick look through this and it looks ok to me, thanks! > >> The v1 question about OP_CLOSEFROM stepping around five of pseudo's >> descriptors while others exist (pseudo_prefix_dir_fd, pseudo_pwd_lck_fd, >> pseudo_util_evlog_fd) still stands; happy to send a follow-up if that list >> is short in both places. > > I had a quick look and yes, we should really be protecting these too. > > That said, I couldn't see what pseudo_prefix_dir_fd actually helps with > and I couldn't see why we need that... This isn't a part of the system I've touched in a very long time. I THOUGHT it was the fd of the directory PSEUDO_PREFIX, so then when other files (sockets, etc) needed to be opened, it was all relative to the fd. This allowed the directory to 'move' if necessary, since we were sure we could still access it via the 'fd'. (or I'm completely wrong and remembering something different.) The only 'definitive' reference I can find doesn't explain the why, just the what doc/program_flow. I wonder if it was done to hold a file description on the directory so that if the PSEUDO_PATH was rm'd it wouldn't clear until pseudo itself was unloaded. --Mark > Cheers, > > Richard > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#4494): https://lists.yoctoproject.org/g/yocto-patches/message/4494 > Mute This Topic: https://lists.yoctoproject.org/mt/120294678/3616948 > Group Owner: yocto-patches+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org] > -=-=-=-=-=-=-=-=-=-=-=- > >
On 7/16/26 11:55 AM, Richard Purdie via lists.yoctoproject.org wrote: > On Thu, 2026-07-16 at 08:46 -0700, Babanpreet Singh wrote: >> Thanks! >> >>> That said, I couldn't see what pseudo_prefix_dir_fd actually helps with >>> and I couldn't see why we need that... >> >> I flagged it since it is held open and a close_range() sweep will wipe it out. I hadn't checked if anything still reads it untill now. It seems like it is just kept open and alive and the socket connection + fchdir() has been moved to pseudo_localstate_dir_fd. >> >> So, for the follow-up I'd purpose: >> >> 1/2: remove pseudo_prefix_dir_fd >> 2/2: add pseudo_pwd_lck_fd and pseudo_util_evlog_fd to the descriptors OP_CLOSEFROM and OP_CLOSE_RANGE step around >> >> If you'd rather keep the fd, I'll drop the removal and add it to the protected set in 2/2 instead. > > I'd suggest making the series the other way around, protect it for now, > send a removal patch on top and we can look at the removal patch and > test and check it isn't really used anywhere. It is possible I'm > missing some usage of it and I'm curious what Mark thinks. I agree. Maybe it's no longer needed, but we're not sure. The code was introduced VERY early in pseudo development, 2010 era. The variable, pseudo_prefix_dir_fd was introduced in 551bf567c171c9f6f475f02de80e35df9563dce7, but this is based on an earlier commit. ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 713) pseudo_path = pseudo_prefix_path(NULL); ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 714) if (pseudo_dir_fd == -1) { ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 715) if (pseudo_path) { ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 716) pseudo_dir_fd = open(pseudo_path, O_RDONLY); ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 717) pseudo_dir_fd = pseudo_fd(pseudo_dir_fd, COPY_FD); ^33d9386 (Peter Seebach 2010-03-16 19:26:24 -0500 718) free(pseudo_path); Unfortunately this was the _FIRST_ commit. So based on that, there really is no history why this was added to begin with. From looking at that first commit, it was using this fd to spawn the server and change the cwd. By holding an open fd on the path we were sure that the directory wouldn't be removed until all the FDs cleared. (Some filesystems you can't delete open directories, other just postpone the removal until the fds clear..) --Mark > The protection patch makes sense and should be easy to merge compared > to that. > > Cheers, > > Richard > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#4497): https://lists.yoctoproject.org/g/yocto-patches/message/4497 > Mute This Topic: https://lists.yoctoproject.org/mt/120294678/3616948 > Group Owner: yocto-patches+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org] > -=-=-=-=-=-=-=-=-=-=-=- > >
pseudo's close_range() wrapper has returned ENOSYS unconditionally since 35433e6 ("ports/linux/guts: Add close_range wrapper for glibc 2.34"). systemd v260 deleted its /proc/self/fd fallback and treats a failure as fatal, so under pseudo every fork+exec dies. That is [YOCTO #16339]. Full story in v1: https://lore.kernel.org/yocto-patches/20260715054142.7-1-bbnpreetsingh@gmail.com/ Paul, thanks for the review. Changes in v2, both from it: 1/2: no code change apart from one comment. The commit message is rewritten in plainer language throughout; the "takes descriptors with it on the way out" sentence and the comment that repeated it now simply say that invalid arguments are rejected with EINVAL before anything is closed. 2/2: two real gaps closed. - CLOSE_RANGE_CLOEXEC needs a 5.11 kernel while the syscall itself needs 5.9, so probing only for the syscall made the test fail on 5.9/5.10. The shell wrapper now probes for the flag separately, the same PSEUDO_DISABLED=1 way, and on such a kernel only the CLOEXEC part of the test is left out. - CLOSE_RANGE_UNSHARE is now tested against a real sharer, not argued from unshare() semantics: a child cloned with CLONE_FILES closes a shared descriptor with the flag set, and the parent must still hold it afterwards. A second child closes the same descriptor without the flag and the parent must see it gone, so a clone that quietly stopped sharing the table cannot make the first check pass vacuously. The new test was checked for sensitivity by deliberately removing the unshare() call from the wrapper: it fails with "child's CLOSE_RANGE_UNSHARE closed the parent's fd 3", and passes with the real implementation. The 5.9/5.10 probe path cannot be exercised end to end on this host (6.17 kernel); the --no-cloexec branch it selects was run directly under pseudo and passes. Full run_tests.sh is unchanged from master apart from the new test passing (the two pre-existing flaky parallel-* failures show up either way). The v1 question about OP_CLOSEFROM stepping around five of pseudo's descriptors while others exist (pseudo_prefix_dir_fd, pseudo_pwd_lck_fd, pseudo_util_evlog_fd) still stands; happy to send a follow-up if that list is short in both places. Babanpreet Singh (2): ports/linux/guts: Implement close_range() instead of returning ENOSYS tests: Add close_range() test enums/op.in | 1 + ports/linux/guts/close_range.c | 54 ++++++- ports/linux/portdefs.h | 16 ++ pseudo_client.c | 60 +++++++ test/test-close-range.c | 283 +++++++++++++++++++++++++++++++++ test/test-close-range.sh | 24 +++ 6 files changed, 431 insertions(+), 7 deletions(-) create mode 100644 test/test-close-range.c create mode 100755 test/test-close-range.sh