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

[Andreas Gruenbacher]

On Wed, Jul 1, 2026 at 10:33 AM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> On 2026-06-30 15:05:03+02:00, Andreas Gruenbacher wrote:
> > On Tue, Jun 30, 2026 at 2:33 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >
> > > On 2026-06-30 14:06 +0200, Andreas Gruenbacher wrote:
> > >
> > > I'm very confused why the f*at() variants are supposed to accept O_PATH
> > > but the f*() aren't? What's the point?
> >
> > Oh, I see. To keep things consistent with fchmod() and fchown() vs.
> > fchmodat() and fchownat(): the former two also don't accept O_PATH
> > file descriptors, while the latter two do.
>
> Right, I saw that and wondered about that there too. I guess it makes
> some sense in that nothing about O_PATH at this point is consistent
> anymore. Will be fun to document what exactly works and doesn't work on
> the manpage...
>
> So then I think we should refactor fs/xattr.c similar to how we did it
> for fch*() and fch*at(). So that f*xattr() call CLASS(fd, ...) and
> f*xattrat() call CLASS(fd_raw, ...).
>
> This should also make it a lot easier to follow, hopefully.

That works for flistxattr(), but it creates a mess for fgetxattr() and
fsetxattr() due to the argument passing; see below.

That argument passing happenss at that level because the lower-level functions
filename_{get,set}xattr() and file_{get,set}xattr() are also used by io_uring,
which has slightly different argument passing semantics.

The attribute name copying in path_removexattr() could be pushed down into
removexattr() because io_uring doesn't use that, but that would make things
less orthogonal.

So that's why I've proposed to change path_{get,set,list,remove}xattr() to
either handle only non-O_PATH or all file descriptors depending on the caller.

Thanks,
Andreas


diff --git a/fs/xattr.c b/fs/xattr.c
index d58979115200..a111813be920 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -725,7 +725,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_raw, f)(dfd);
 		if (fd_empty(f))
 			error = -EBADF;
 		else
@@ -779,8 +779,30 @@ SYSCALL_DEFINE5(lsetxattr, const char __user *, pathname,
 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,
-			       value, size, flags);
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.cvalue	= value,
+		.kvalue	= NULL,
+		.size	= size,
+		.kname	= &kname,
+		.flags	= flags,
+	};
+	int error;
+
+	error = setxattr_copy(name, &ctx);
+	if (error)
+		return error;
+
+	CLASS(fd, f)(fd);
+	error = -EBADF;
+	if (fd_empty(f))
+		goto out;
+
+	error = file_setxattr(fd_file(f), &ctx);
+
+out:
+	kvfree(ctx.kvalue);
+	return error;
 }
 
 /*
@@ -865,7 +887,7 @@ 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);
@@ -918,7 +940,24 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 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);
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.value    = value,
+		.size     = size,
+		.kname    = &kname,
+		.flags    = 0,
+	};
+	int error;
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+
+	CLASS(fd, f)(fd);
+	if (fd_empty(f))
+		return -EBADF;
+
+	return file_getxattr(fd_file(f), &ctx);
 }
 
 /*
@@ -991,7 +1030,7 @@ 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);
+		CLASS(fd_raw, f)(dfd);
 		if (fd_empty(f))
 			return -EBADF;
 		return file_listxattr(fd_file(f), list, size);
@@ -1022,7 +1061,11 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	return path_listxattrat(fd, NULL, AT_EMPTY_PATH, list, size);
+	CLASS(fd, f)(fd);
+	if (fd_empty(f))
+		return -EBADF;
+
+	return file_listxattr(fd_file(f), list, size);
 }
 
 /*
@@ -1088,7 +1131,7 @@ static int path_removexattrat(int dfd, const char __user *pathname,
 
 	CLASS(filename_maybe_null, filename)(pathname, at_flags);
 	if (!filename) {
-		CLASS(fd, f)(dfd);
+		CLASS(fd_raw, f)(dfd);
 		if (fd_empty(f))
 			return -EBADF;
 		return file_removexattr(fd_file(f), &kname);
@@ -1117,7 +1160,18 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	return path_removexattrat(fd, NULL, AT_EMPTY_PATH, name);
+	struct xattr_name kname;
+	int error;
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+
+	CLASS(fd, f)(fd);
+	if (fd_empty(f))
+		return -EBADF;
+
+	return file_removexattr(fd_file(f), &kname);
 }
 
 int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)