Re: [PATCH 2/3] xattrat: accept empty O_PATH file descriptors

[Andreas Gruenbacher]

On Tue, Jun 30, 2026 at 11:12 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > Right now, the setxattrat(), getxattrat(), listxattrat(), and removexattrat()
> > system calls fail with -EBADF when dfd is an O_PATH file descriptor, pathname
> > is an empty string or NULL, and the AT_EMPTY_PATH flag in at_flags is set.
> > (Regular non-O_PATH file descriptors are accepted.)  This is inconsistent with
> > the behavior of system calls like fstatat() and fchmodat() which do accept
> > those file descriptors.
> >
> > The same operations can also be carried out indirectly by using
> > "/proc/self/fd/<dfd>" as the pathname, but that is only more cumbersome, and
> > for no good reason.  Fix that by changing the xattrat() system calls to accept
> > these file descriptors directly.
> >
> > We stick with the existing practice of leaving the behavior of the fsetxattr(),
> > fgetxattr(), flistxattr(), and fremovexattr() system calls unchanged: those
> > will still reject O_PATH file descriptors.
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> >
> > diff --git a/fs/xattr.c b/fs/xattr.c
> > index d58979115200..4737554df5fa 100644
> > --- a/fs/xattr.c
> > +++ b/fs/xattr.c
> > @@ -700,7 +700,8 @@ int filename_setxattr(int dfd, struct filename *filename,
> >
> >  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)
> > +                        const void __user *value, size_t size, int flags,
> > +                        fmode_t mask)
> >  {
> >       struct xattr_name kname;
> >       struct kernel_xattr_ctx ctx = {
> > @@ -725,7 +726,7 @@ 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);
> > +             CLASS(fd_except, f)(dfd, mask);
>
> So I can live with the O_PATH change, I guess...
> but I really dislike fd_except. The code should never bother to pass
> FMODE_PATH directly which is just rather ugly and we've taken pain to
> make it work without passing it.
>
> I also don't yet understand why this would be necessary at all.

So we've got some system calls that should accept O_PATH file descriptors
({get,set,list,remove}xattrat), and others that should not
(f{get,set,list,remove}xattr).

We could implement that by using either CLASS(fd) or CLASS(fd_raw) in
path_{get,set,list,remove}xattrat depending on which case we want, but that
would duplicate unrelated code.  To avoid that, this patch took the approach of
extending the class with a parameter that defines which file descriptors to
accept.  But we could also accept all file descriptors and filter out the bad
ones later like in the untested patch below.  Either approach will get us
there; just pick one ...

Thanks,
Andreas

diff --git a/fs/xattr.c b/fs/xattr.c
index d58979115200..8e4495585fad 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -845,7 +845,8 @@ ssize_t filename_getxattr(int dfd, struct filename *filename,
 
 static ssize_t path_getxattrat(int dfd, const char __user *pathname,
 			       unsigned int at_flags, const char __user *name,
-			       void __user *value, size_t size)
+			       void __user *value, size_t size,
+			       unsigned int reject_flags)
 {
 	struct xattr_name kname;
 	struct kernel_xattr_ctx ctx = {
@@ -865,10 +866,13 @@ 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);
+		CLASS(fd_raw, f)(dfd);
 		if (fd_empty(f))
 			return -EBADF;
-		return file_getxattr(fd_file(f), &ctx);
+		struct file *file = fd_file(f);
+		if (file->f_flags & reject_flags)
+			return -EBADF;
+		return file_getxattr(file, &ctx);
 	} else {
 		int lookup_flags = 0;
 		if (!(at_flags & AT_SYMLINK_NOFOLLOW))
@@ -899,26 +903,27 @@ SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned in
 		return -EINVAL;
 
 	return path_getxattrat(dfd, pathname, at_flags, name,
-			       u64_to_user_ptr(args.value), args.size);
+			       u64_to_user_ptr(args.value), args.size, 0);
 }
 
 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, pathname, 0, name, value, size, 0);
 }
 
 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);
+			       value, size, 0);
 }
 
 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);
+	return path_getxattrat(fd, NULL, AT_EMPTY_PATH, name, value, size,
+			       O_PATH);
 }
 
 /*