On Tue, Jun 24, 2025 at 11:30:42AM +0200, Jan Kara wrote:
> On Tue 24-06-25 10:29:13, Christian Brauner wrote:
> > Various filesystems such as pidfs (and likely drm in the future) have a
> > use-case to support opening files purely based on the handle without
> > having to require a file descriptor to another object. That's especially
> > the case for filesystems that don't do any lookup whatsoever and there's
> > zero relationship between the objects. Such filesystems are also
> > singletons that stay around for the lifetime of the system meaning that
> > they can be uniquely identified and accessed purely based on the file
> > handle type. Enable that so that userspace doesn't have to allocate an
> > object needlessly especially if they can't do that for whatever reason.
> >
> > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> > ---
> > fs/fhandle.c | 22 ++++++++++++++++++++--
> > fs/pidfs.c | 5 ++++-
> > 2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fhandle.c b/fs/fhandle.c
> > index ab4891925b52..54081e19f594 100644
> > --- a/fs/fhandle.c
> > +++ b/fs/fhandle.c
> > @@ -173,7 +173,7 @@ SYSCALL_DEFINE5(name_to_handle_at, int, dfd, const char __user *, name,
> > return err;
> > }
> >
> > -static int get_path_anchor(int fd, struct path *root)
> > +static int get_path_anchor(int fd, struct path *root, int handle_type)
> > {
> > if (fd >= 0) {
> > CLASS(fd, f)(fd);
> > @@ -193,6 +193,24 @@ static int get_path_anchor(int fd, struct path *root)
> > return 0;
> > }
> >
> > + /*
> > + * Only autonomous handles can be decoded without a file
> > + * descriptor.
> > + */
> > + if (!(handle_type & FILEID_IS_AUTONOMOUS))
> > + return -EOPNOTSUPP;
>
> This somewhat ties to my comment to patch 5 that if someone passed invalid
> fd < 0 before, we'd be returning -EBADF and now we'd be returning -EINVAL
> or -EOPNOTSUPP based on FILEID_IS_AUTONOMOUS setting. I don't care that
> much about it so feel free to ignore me but I think the following might be
> more sensible error codes:
>
> if (!(handle_type & FILEID_IS_AUTONOMOUS)) {
> if (fd == FD_INVALID)
> return -EOPNOTSUPP;
> return -EBADF;
> }
Yes, that makes sense. I'll take the suggestion.
>
> if (fd != FD_INVALID)
> return -EBADF; (or -EINVAL no strong preference here)
>
> Since I don't care that much feel free to add:
>
> Reviewed-by: Jan Kara <jack@xxxxxxx>
>
> Honza
>
> > +
> > + if (fd != FD_INVALID)
> > + return -EINVAL;
> > +
> > + switch (handle_type & ~FILEID_USER_FLAGS_MASK) {
> > + case FILEID_PIDFS:
> > + pidfs_get_root(root);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -347,7 +365,7 @@ static int handle_to_path(int mountdirfd, struct file_handle __user *ufh,
> > FILEID_USER_FLAGS(f_handle.handle_type) & ~FILEID_VALID_USER_FLAGS)
> > return -EINVAL;
> >
> > - retval = get_path_anchor(mountdirfd, &ctx.root);
> > + retval = get_path_anchor(mountdirfd, &ctx.root, f_handle.handle_type);
> > if (retval)
> > return retval;
> >
> > diff --git a/fs/pidfs.c b/fs/pidfs.c
> > index 69b0541042b5..2ab9b47fbfae 100644
> > --- a/fs/pidfs.c
> > +++ b/fs/pidfs.c
> > @@ -747,7 +747,7 @@ static int pidfs_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> >
> > *max_len = 2;
> > *(u64 *)fh = pid->ino;
> > - return FILEID_KERNFS;
> > + return FILEID_PIDFS;
> > }
> >
> > static int pidfs_ino_find(const void *key, const struct rb_node *node)
> > @@ -802,6 +802,8 @@ static struct dentry *pidfs_fh_to_dentry(struct super_block *sb,
> > return NULL;
> >
> > switch (fh_type) {
> > + case FILEID_PIDFS:
> > + fallthrough;
> > case FILEID_KERNFS:
> > pid_ino = *(u64 *)fid;
> > break;
> > @@ -860,6 +862,7 @@ static const struct export_operations pidfs_export_operations = {
> > .fh_to_dentry = pidfs_fh_to_dentry,
> > .open = pidfs_export_open,
> > .permission = pidfs_export_permission,
> > + .flags = EXPORT_OP_AUTONOMOUS_HANDLES,
> > };
> >
> > static int pidfs_init_inode(struct inode *inode, void *data)
> >
> > --
> > 2.47.2
> >
> --
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
