Re: [PATCH v2] xattrat: accept empty O_PATH file descriptors

On Thu, Jul 30, 2026 at 8:49 PM Christian Brauner <[email protected]> wrote:
> On 2026-07-22 17:10 +0200, Andreas Gruenbacher wrote:
> > Christian,
> >
> > here's an updated patch for making the *xattrat() system calls accept
> > O_PATH file descriptors.
> >
> > Compared to the previous version (see the discussion at [*]), this
> > version adds a fd_maybe_raw cleanup class that uses either fdget() or
> > fdget_raw() depending on which kinds of file descriptors are acceptable.
> > That's still a but ugly, but all the alternatives I could come up with
> > are much worse.
> >
> > [*] https://lore.kernel.org/linux-fsdevel/[email protected]/
>
> Thank you. How do you feel about?

I don't think it's an improvement; see below.

> ---
>  fs/xattr.c | 100 ++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 64 insertions(+), 36 deletions(-)
>
> diff --git a/fs/xattr.c b/fs/xattr.c
> index d58979115200..133bca3371ee 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -698,9 +698,10 @@ int filename_setxattr(int dfd, struct filename *filename,
>         return error;
>  }
>
> -static int path_setxattrat(int dfd, const char __user *pathname,
> -                          unsigned int at_flags, const char __user *name,
> -                          const void __user *value, size_t size, int flags)
> +static int path_setxattrat(int dfd, struct file *dfd_file,
> +                          const char __user *pathname, unsigned int at_flags,
> +                          const char __user *name, const void __user *value,
> +                          size_t size, int flags)
>  {
>         struct xattr_name kname;
>         struct kernel_xattr_ctx ctx = {
> @@ -725,11 +726,10 @@ static int path_setxattrat(int dfd, const char __user *pathname,
>
>         CLASS(filename_maybe_null, filename)(pathname, at_flags);
>         if (!filename && dfd >= 0) {
> -               CLASS(fd, f)(dfd);
> -               if (fd_empty(f))
> +               if (!dfd_file)
>                         error = -EBADF;

I don't particularly like that the initialization and the error
checking are being split up here. The result is the same, though.

>                 else
> -                       error = file_setxattr(fd_file(f), &ctx);
> +                       error = file_setxattr(dfd_file, &ctx);
>         } else {
>                 error = filename_setxattr(dfd, filename, lookup_flags, &ctx);
>         }
> @@ -756,7 +756,10 @@ SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned in
>         if (error)
>                 return error;
>
> -       return path_setxattrat(dfd, pathname, at_flags, name,
> +       /* setxattrat() accepts O_PATH file descriptors. */
> +       CLASS(fd_raw, f)(dfd);

On this path, fdget_raw(dfd) will now be called unnecessarily when
pathname is not NULL.

> +
> +       return path_setxattrat(dfd, fd_file(f), pathname, at_flags, name,
>                                u64_to_user_ptr(args.value), args.size,
>                                args.flags);
>  }
> @@ -765,21 +768,25 @@ SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
>                 const char __user *, name, const void __user *, value,
>                 size_t, size, int, flags)
>  {
> -       return path_setxattrat(AT_FDCWD, pathname, 0, name, value, size, flags);
> +       return path_setxattrat(AT_FDCWD, NULL, pathname, 0, name, value, size,
> +                              flags);
>  }
>
>  SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
>                 const char __user *, name, const void __user *, value,
>                 size_t, size, int, flags)
>  {
> -       return path_setxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
> -                              value, size, flags);
> +       return path_setxattrat(AT_FDCWD, NULL, pathname, AT_SYMLINK_NOFOLLOW,
> +                              name, value, size, flags);
>  }
>
>  SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
>                 const void __user *,value, size_t, size, int, flags)
>  {
> -       return path_setxattrat(fd, NULL, AT_EMPTY_PATH, name,
> +       /* fsetxattr() rejects O_PATH file descriptors. */
> +       CLASS(fd, f)(fd);
> +
> +       return path_setxattrat(fd, fd_file(f), NULL, AT_EMPTY_PATH, name,
>                                value, size, flags);
>  }
>
> @@ -843,7 +850,8 @@ ssize_t filename_getxattr(int dfd, struct filename *filename,
>         return error;
>  }
>
> -static ssize_t path_getxattrat(int dfd, const char __user *pathname,
> +static ssize_t path_getxattrat(int dfd, struct file *dfd_file,
> +                              const char __user *pathname,
>                                unsigned int at_flags, const char __user *name,
>                                void __user *value, size_t size)
>  {
> @@ -865,10 +873,9 @@ static ssize_t path_getxattrat(int dfd, const char __user *pathname,
>
>         CLASS(filename_maybe_null, filename)(pathname, at_flags);
>         if (!filename && dfd >= 0) {
> -               CLASS(fd, f)(dfd);
> -               if (fd_empty(f))
> +               if (!dfd_file)
>                         return -EBADF;
> -               return file_getxattr(fd_file(f), &ctx);
> +               return file_getxattr(dfd_file, &ctx);
>         } else {
>                 int lookup_flags = 0;
>                 if (!(at_flags & AT_SYMLINK_NOFOLLOW))
> @@ -898,27 +905,34 @@ SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned in
>         if (args.flags != 0)
>                 return -EINVAL;
>
> -       return path_getxattrat(dfd, pathname, at_flags, name,
> +       /* getxattrat() accepts O_PATH file descriptors. */
> +       CLASS(fd_raw, f)(dfd);

Same here.

> +
> +       return path_getxattrat(dfd, fd_file(f), pathname, at_flags, name,
>                                u64_to_user_ptr(args.value), args.size);
>  }
>
>  SYSCALL_DEFINE4(getxattr, const char __user *, pathname,
>                 const char __user *, name, void __user *, value, size_t, size)
>  {
> -       return path_getxattrat(AT_FDCWD, pathname, 0, name, value, size);
> +       return path_getxattrat(AT_FDCWD, NULL, pathname, 0, name, value, size);
>  }
>
>  SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
>                 const char __user *, name, void __user *, value, size_t, size)
>  {
> -       return path_getxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
> -                              value, size);
> +       return path_getxattrat(AT_FDCWD, NULL, pathname, AT_SYMLINK_NOFOLLOW,
> +                              name, value, size);
>  }
>
>  SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
>                 void __user *, value, size_t, size)
>  {
> -       return path_getxattrat(fd, NULL, AT_EMPTY_PATH, name, value, size);
> +       /* fgetxattr() rejects O_PATH file descriptors. */
> +       CLASS(fd, f)(fd);
> +
> +       return path_getxattrat(fd, fd_file(f), NULL, AT_EMPTY_PATH, name, value,
> +                              size);
>  }
>
>  /*
> @@ -980,7 +994,8 @@ ssize_t filename_listxattr(int dfd, struct filename *filename,
>         return error;
>  }
>
> -static ssize_t path_listxattrat(int dfd, const char __user *pathname,
> +static ssize_t path_listxattrat(int dfd, struct file *dfd_file,
> +                               const char __user *pathname,
>                                 unsigned int at_flags, char __user *list,
>                                 size_t size)
>  {
> @@ -991,10 +1006,9 @@ static ssize_t path_listxattrat(int dfd, const char __user *pathname,
>
>         CLASS(filename_maybe_null, filename)(pathname, at_flags);
>         if (!filename) {
> -               CLASS(fd, f)(dfd);
> -               if (fd_empty(f))
> +               if (!dfd_file)
>                         return -EBADF;
> -               return file_listxattr(fd_file(f), list, size);
> +               return file_listxattr(dfd_file, list, size);
>         }
>
>         lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> @@ -1005,24 +1019,31 @@ SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
>                 unsigned int, at_flags,
>                 char __user *, list, size_t, size)
>  {
> -       return path_listxattrat(dfd, pathname, at_flags, list, size);
> +       /* listxattrat() accepts O_PATH file descriptors. */
> +       CLASS(fd_raw, f)(dfd);

And here.

> +
> +       return path_listxattrat(dfd, fd_file(f), pathname, at_flags, list, size);
>  }
>
>  SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
>                 size_t, size)
>  {
> -       return path_listxattrat(AT_FDCWD, pathname, 0, list, size);
> +       return path_listxattrat(AT_FDCWD, NULL, pathname, 0, list, size);
>  }
>
>  SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
>                 size_t, size)
>  {
> -       return path_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
> +       return path_listxattrat(AT_FDCWD, NULL, pathname, AT_SYMLINK_NOFOLLOW,
> +                               list, size);
>  }
>
>  SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
>  {
> -       return path_listxattrat(fd, NULL, AT_EMPTY_PATH, list, size);
> +       /* flistxattr() rejects O_PATH file descriptors. */
> +       CLASS(fd, f)(fd);
> +
> +       return path_listxattrat(fd, fd_file(f), NULL, AT_EMPTY_PATH, list, size);
>  }
>
>  /*
> @@ -1072,7 +1093,8 @@ static int filename_removexattr(int dfd, struct filename *filename,
>         return error;
>  }
>
> -static int path_removexattrat(int dfd, const char __user *pathname,
> +static int path_removexattrat(int dfd, struct file *dfd_file,
> +                             const char __user *pathname,
>                               unsigned int at_flags, const char __user *name)
>  {
>         struct xattr_name kname;
> @@ -1088,10 +1110,9 @@ static int path_removexattrat(int dfd, const char __user *pathname,
>
>         CLASS(filename_maybe_null, filename)(pathname, at_flags);
>         if (!filename) {
> -               CLASS(fd, f)(dfd);
> -               if (fd_empty(f))
> +               if (!dfd_file)
>                         return -EBADF;
> -               return file_removexattr(fd_file(f), &kname);
> +               return file_removexattr(dfd_file, &kname);
>         }
>         lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
>         return filename_removexattr(dfd, filename, lookup_flags, &kname);
> @@ -1100,24 +1121,31 @@ static int path_removexattrat(int dfd, const char __user *pathname,
>  SYSCALL_DEFINE4(removexattrat, int, dfd, const char __user *, pathname,
>                 unsigned int, at_flags, const char __user *, name)
>  {
> -       return path_removexattrat(dfd, pathname, at_flags, name);
> +       /* removexattrat() accepts O_PATH file descriptors. */
> +       CLASS(fd_raw, f)(dfd);

And here.

> +
> +       return path_removexattrat(dfd, fd_file(f), pathname, at_flags, name);
>  }
>
>  SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
>                 const char __user *, name)
>  {
> -       return path_removexattrat(AT_FDCWD, pathname, 0, name);
> +       return path_removexattrat(AT_FDCWD, NULL, pathname, 0, name);
>  }
>
>  SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
>                 const char __user *, name)
>  {
> -       return path_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
> +       return path_removexattrat(AT_FDCWD, NULL, pathname, AT_SYMLINK_NOFOLLOW,
> +                                 name);
>  }
>
>  SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
>  {
> -       return path_removexattrat(fd, NULL, AT_EMPTY_PATH, name);
> +       /* fremovexattr() rejects O_PATH file descriptors. */
> +       CLASS(fd, f)(fd);
> +
> +       return path_removexattrat(fd, fd_file(f), NULL, AT_EMPTY_PATH, name);
>  }
>
>  int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
>

Thanks,
Andreas