On Wed 22-07-26 14:07:37, Christian Brauner wrote: > On 2026-07-16 13:11 +0200, Jan Kara wrote: > > On Thu 16-07-26 13:27:22, Li Chen wrote: > > > PIDFD_THREAD shares O_EXCL. do_dentry_open() clears O_EXCL after > > > pidfs_export_open() validates the flags, so open_by_handle_at() > > > silently turns a thread pidfd into a process pidfd. > > > > > > Restore PIDFD_THREAD on the opened file, matching pidfs_alloc_file(), > > > and cover the restored flag with F_GETFL. > > > > > > Signed-off-by: Li Chen <[email protected]> > > > > Makes sense. Just looking at this code, what about PIDFD_AUTOKILL (aka > > O_TRUNC?)? Reading the commit introducing it 07c3ef58223e it appears it was > > never meant to be directly requested by userspace so probably > > pidfs_export_open() needs to reject it. But that's unrelated issue to your > > patch so feel free to add: > > Good point. But pidfs_export_open() is guarded by > pidfs_export_permission() which does: > > #define VALID_FILE_HANDLE_OPEN_FLAGS \ > (O_RDONLY | O_WRONLY | O_RDWR | O_NONBLOCK | O_CLOEXEC | O_EXCL) > > if (oflags & ~(VALID_FILE_HANDLE_OPEN_FLAGS | O_LARGEFILE)) > return -EINVAL; > > and so is safe. But I've added the following patch on top which also > splats if that is passed! > > From 99a859485394eca8cf6271de09ae72ac3b846bda Mon Sep 17 00:00:00 2001 > From: Christian Brauner <[email protected]> > Date: Wed, 22 Jul 2026 13:56:28 +0200 > Subject: [PATCH] pidfs: add pidfs_dentry_open() helper > > Both pidfs_alloc_file() and pidfs_export_open() need to force O_RDWR > and reapply the pidfd flags that do_dentry_open() strips. Move the > common logic into a helper. > > PIDFD_AUTOKILL is now part of the restore mask in the file handle path > as well, but pidfs_export_permission() rejects O_TRUNC, so this is a > no-op there. But warn nonetheless. > > Signed-off-by: Christian Brauner (Amutable) <[email protected]>
Nice! Feel free to add: Reviewed-by: Jan Kara <[email protected]> Honza > diff --git a/fs/pidfs.c b/fs/pidfs.c > index c20ffd747ff5..695215aa2a58 100644 > --- a/fs/pidfs.c > +++ b/fs/pidfs.c > @@ -915,6 +915,20 @@ static struct dentry *pidfs_fh_to_dentry(struct > super_block *sb, > return path.dentry; > } > > +static struct file *pidfs_dentry_open(const struct path *path, > + unsigned int flags, > + const struct cred *cred) > +{ > + struct file *file; > + > + /* pidfds are always O_RDWR. */ > + file = dentry_open(path, flags | O_RDWR, cred); > + /* do_dentry_open() strips O_EXCL and O_TRUNC. */ > + if (!IS_ERR(file)) > + file->f_flags |= flags & (PIDFD_THREAD | PIDFD_AUTOKILL); > + return file; > +} > + > /* > * Make sure that we reject any nonsensical flags that users pass via > * open_by_handle_at(). Note that PIDFD_THREAD is defined as O_EXCL, and > @@ -939,18 +953,14 @@ static int pidfs_export_permission(struct > handle_to_path_ctx *ctx, > > static struct file *pidfs_export_open(const struct path *path, unsigned int > oflags) > { > - struct file *file; > - > /* > - * Clear O_LARGEFILE as open_by_handle_at() forces it and raise > - * O_RDWR as pidfds always are. > + * Opening via file handle may never raise PIDFD_AUTOKILL. That can > + * only be done at task creation! > */ > - oflags &= ~O_LARGEFILE; > - file = dentry_open(path, oflags | O_RDWR, current_cred()); > - /* do_dentry_open() strips O_EXCL, which encodes PIDFD_THREAD. */ > - if (!IS_ERR(file)) > - file->f_flags |= oflags & PIDFD_THREAD; > - return file; > + if (WARN_ON_ONCE(oflags & PIDFD_AUTOKILL)) > + return ERR_PTR(-EINVAL); > + /* Clear O_LARGEFILE as open_by_handle_at() forces it. */ > + return pidfs_dentry_open(path, oflags & ~O_LARGEFILE, current_cred()); > } > > static const struct export_operations pidfs_export_operations = { > @@ -1114,7 +1124,6 @@ static struct file_system_type pidfs_type = { > > struct file *pidfs_alloc_file(struct pid *pid, unsigned int flags) > { > - struct file *pidfd_file; > struct path path __free(path_put) = {}; > int ret; > > @@ -1132,16 +1141,7 @@ struct file *pidfs_alloc_file(struct pid *pid, > unsigned int flags) > VFS_WARN_ON_ONCE(!pid->attr); > > flags &= ~PIDFD_STALE; > - flags |= O_RDWR; > - pidfd_file = dentry_open(&path, flags, current_cred()); > - /* > - * Raise PIDFD_THREAD and PIDFD_AUTOKILL explicitly as > - * do_dentry_open() strips O_EXCL and O_TRUNC. > - */ > - if (!IS_ERR(pidfd_file)) > - pidfd_file->f_flags |= (flags & (PIDFD_THREAD | > PIDFD_AUTOKILL)); > - > - return pidfd_file; > + return pidfs_dentry_open(&path, flags, current_cred()); > } > > void __init pidfs_init(void) > -- > 2.53.0 > > -- Jan Kara <[email protected]> SUSE Labs, CR
