[RFC][PATCHES] xattr stuff and interactions with io_uring

[RFC][PATCHES] xattr stuff and interactions with io_uring

[Al Viro]

	The series below is a small-scale attempt at sanitizing the
interplay between io_uring and normal syscalls.  It is limited to
xattr-related syscalls and I would like to have a similar approach taken
a lot further than that.

	Right now we have an ad-hoc mess, with io_uring stuff hooked
at different layers to syscall guts.  And it keeps growing and getting
messier.

	As far as I can tell, the general rules are

* we want arguments copied in when request is submitted to io_uring.
Rationale: userland caller should be able to have that stuff on his
stack frame, rather than keeping it around until the request completion.
Fair enough.

* destination for the stuff we want copied _out_ (results of read(),
etc.)  has to stay around until the IO completion.  In other words, such
references remain as userland pointers until the request is executed.
Fair enough.

* the things get less clear-cut where we are talking about bulk data
copied _in_ - write() arguments, for example.  In some cases we do
handle that at request execution time, in some we do not (setsockopt
vs. setxattr vs. write, for example).  Decided on individual basis?

* some argument validation is done when request is submitted; however,
anything related to resolving pathnames (at least - there may be other
such areas) is done only at the time request is processed.  Rationale:
previous requests might alter the state in question and we want the
effects of such changes visible to our request.

* pathnames are copied in at submission time and resolved at execution
time; descriptors are resolved at submission time, except when used in
dirfd role.

	What I propose for xattr stuff (and what could serve as a model
for other cases) is two families of helpers; one takes struct file
reference + whatever other arguments we want, another - dfd + filename +
lookup_flags + other arguments; filename is passed as a struct filename
reference, which is consumed by the helper.

	Normal syscalls are easily expressed via those; io_uring side is
also apparently OK.  Not sure if it's flexible enough, though - e.g.
IORING_OP_MKDIRAT et.al. end up resolving the descriptor _twice_
and can't have it coming from io_uring analogue of descriptor table.
It might make more sense to allow a variant that would have dirfd already
resolved to file reference.  It's not that hard to do on fs/namei.c side
(set_nameidata() and path_init() would have to be taught about that,
and that's more of less it), but... we need to figure out whether it
makes better sense to have descriptor resolution prompt or delayed on
the io_uring side - i.e. at submission or at execution time.

	Anyway, for now I'm following the model we have for do_mkdirat()
et.al.  It's definitely flexible enough on the syscall side; in particular,
rebasing ...xattrat(2) patch on top of that had been trivial.  It also
promises some fairly nice simplifications wrt struct filename vs. audit,
but that's a separate story.

	Currently the branch lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.xattr
Individual patches in followups.

1/9)	switch to CLASS(fd) use.  Obvious.

2/9)	rename struct xattr_ctx to kernel_xattr_ctx
prep from the ...xattrat() series, to reduce the PITA for rebase

3/9)	io_[gs]etxattr_prep(): just use getname()
getname_flags(_, LOOKUP_FOLLOW) is ridiculous.

4/9)	new helper: import_xattr_name()
exact same logics for copying the xattr name in, dealing with
overflows, etc. in a lot of places.  

5/9)	replace do_setxattr() with saner helpers
file_setxattr(file, ctx) and filename_setxattr(dfd, filename, lookup_flags, ctx).
Don't mess with do_setxattr() directly - use those.  In particular, don't
bother with the ESTALE loop in io_uring - it doesn't belong there.

6/9)	replace do_getxattr() with saner helpers
Same on getxattr() side.

7/9)	new helpers: file_listxattr(), filename_listxattr()
Same, except that io_uring doesn't use those, so the helpers are left static.

8/9)	new helpers: file_removexattr(), filename_removexattr()
Ditto

9/9)	fs/xattr: add *at family syscalls
Rebased patch introducing those, with a bunch of fixes folded in so we don't
create bisect hazard there.  Potentially interesting bit is the pathname-handling
logics - getname_xattr(pathname, flags) returns a struct filename reference
if no AT_EMPTY_PATH had been given or if the pathname was non-NULL and turned out
to be non-empty.  With (NULL,AT_EMPTY_PATH) or (empty string, AT_EMPTY_PATH) it
returns NULL (and it tries to skip the allocation using the same trick with
get_user() that vfs_empty_path() uses).  That helper simplifies a lot of things,
and it should be cheap enough to convert fsetxattr(2) et.al. to the unified
path_setxattrat() and its ilk.  IF we get a slowdown on those, we can always
expand and simplify the calls in fsetxattr(2) and friends.

Anyway, comments, review and testing would be very welcome.

[PATCH 1/9] xattr: switch to CLASS(fd)

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 05ec7e7d9e87..0fc813cb005c 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -697,9 +697,9 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 	int error;
 
 	CLASS(fd, f)(fd);
-	if (!fd_file(f))
-		return -EBADF;
 
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
 	error = setxattr_copy(name, &ctx);
 	if (error)
@@ -809,16 +809,13 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
-	error = getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
+	return getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
 			 name, value, size);
-	fdput(f);
-	return error;
 }
 
 /*
@@ -885,15 +882,12 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
-	error = listxattr(fd_file(f)->f_path.dentry, list, size);
-	fdput(f);
-	return error;
+	return listxattr(fd_file(f)->f_path.dentry, list, size);
 }
 
 /*
@@ -950,12 +944,12 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	struct fd f = fdget(fd);
+	CLASS(fd, f)(fd);
 	char kname[XATTR_NAME_MAX + 1];
-	int error = -EBADF;
+	int error;
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
 
 	error = strncpy_from_user(kname, name, sizeof(kname));
@@ -970,7 +964,6 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 				    fd_file(f)->f_path.dentry, kname);
 		mnt_drop_write_file(fd_file(f));
 	}
-	fdput(f);
 	return error;
 }
 
-- 
2.39.5

[PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx

[Al Viro]

From: Christian Göttsche <cgzones@googlemail.com>

Rename the struct xattr_ctx to increase distinction with the about to be
added user API struct xattr_args.

No functional change.

Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Link: https://lore.kernel.org/r/20240426162042.191916-2-cgoettsche@seltendoof.de
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/internal.h    |  8 ++++----
 fs/xattr.c       | 12 ++++++------
 io_uring/xattr.c |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 8c1b7acbbe8f..81c7a085355c 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -267,7 +267,7 @@ struct xattr_name {
 	char name[XATTR_NAME_MAX + 1];
 };
 
-struct xattr_ctx {
+struct kernel_xattr_ctx {
 	/* Value of attribute */
 	union {
 		const void __user *cvalue;
@@ -283,11 +283,11 @@ struct xattr_ctx {
 
 ssize_t do_getxattr(struct mnt_idmap *idmap,
 		    struct dentry *d,
-		    struct xattr_ctx *ctx);
+		    struct kernel_xattr_ctx *ctx);
 
-int setxattr_copy(const char __user *name, struct xattr_ctx *ctx);
+int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct xattr_ctx *ctx);
+		struct kernel_xattr_ctx *ctx);
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
 
 #ifdef CONFIG_FS_POSIX_ACL
diff --git a/fs/xattr.c b/fs/xattr.c
index 0fc813cb005c..1214ae7e71db 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -590,7 +590,7 @@ EXPORT_SYMBOL_GPL(vfs_removexattr);
  * Extended attribute SET operations
  */
 
-int setxattr_copy(const char __user *name, struct xattr_ctx *ctx)
+int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 {
 	int error;
 
@@ -620,7 +620,7 @@ int setxattr_copy(const char __user *name, struct xattr_ctx *ctx)
 }
 
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct xattr_ctx *ctx)
+		struct kernel_xattr_ctx *ctx)
 {
 	if (is_posix_acl_xattr(ctx->kname->name))
 		return do_set_acl(idmap, dentry, ctx->kname->name,
@@ -635,7 +635,7 @@ static int path_setxattr(const char __user *pathname,
 			 size_t size, int flags, unsigned int lookup_flags)
 {
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.cvalue   = value,
 		.kvalue   = NULL,
 		.size     = size,
@@ -687,7 +687,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 		const void __user *,value, size_t, size, int, flags)
 {
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.cvalue   = value,
 		.kvalue   = NULL,
 		.size     = size,
@@ -720,7 +720,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
  */
 ssize_t
 do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
-	struct xattr_ctx *ctx)
+	struct kernel_xattr_ctx *ctx)
 {
 	ssize_t error;
 	char *kname = ctx->kname->name;
@@ -755,7 +755,7 @@ getxattr(struct mnt_idmap *idmap, struct dentry *d,
 {
 	ssize_t error;
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.value    = value,
 		.kvalue   = NULL,
 		.size     = size,
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 6cf41c3bc369..5b4594ede935 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -18,7 +18,7 @@
 
 struct io_xattr {
 	struct file			*file;
-	struct xattr_ctx		ctx;
+	struct kernel_xattr_ctx		ctx;
 	struct filename			*filename;
 };
 
-- 
2.39.5

Re: [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:23AM GMT, Al Viro wrote:
> From: Christian Göttsche <cgzones@googlemail.com>
> 
> Rename the struct xattr_ctx to increase distinction with the about to be
> added user API struct xattr_args.
> 
> No functional change.
> 
> Suggested-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> Link: https://lore.kernel.org/r/20240426162042.191916-2-cgoettsche@seltendoof.de
> Reviewed-by: Jan Kara <jack@suse.cz>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 3/9] io_[gs]etxattr_prep(): just use getname()

[Al Viro]

getname_flags(pathname, LOOKUP_FOLLOW) is obviously bogus - following
trailing symlinks has no impact on how to copy the pathname from userland...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 io_uring/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 5b4594ede935..04abf0739668 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -96,7 +96,7 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
-	ix->filename = getname_flags(path, LOOKUP_FOLLOW);
+	ix->filename = getname(path);
 	if (IS_ERR(ix->filename)) {
 		ret = PTR_ERR(ix->filename);
 		ix->filename = NULL;
@@ -189,7 +189,7 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
-	ix->filename = getname_flags(path, LOOKUP_FOLLOW);
+	ix->filename = getname(path);
 	if (IS_ERR(ix->filename)) {
 		ret = PTR_ERR(ix->filename);
 		ix->filename = NULL;
-- 
2.39.5

Re: [PATCH 3/9] io_[gs]etxattr_prep(): just use getname()

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:24AM GMT, Al Viro wrote:
> getname_flags(pathname, LOOKUP_FOLLOW) is obviously bogus - following
> trailing symlinks has no impact on how to copy the pathname from userland...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 4/9] new helper: import_xattr_name()

[Al Viro]

common logics for marshalling xattr names.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  3 +++
 fs/xattr.c       | 45 +++++++++++++++++++++++----------------------
 io_uring/xattr.c |  7 ++-----
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 81c7a085355c..b9f5ac4d39fc 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -288,6 +288,9 @@ ssize_t do_getxattr(struct mnt_idmap *idmap,
 int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		struct kernel_xattr_ctx *ctx);
+
+int import_xattr_name(struct xattr_name *kname, const char __user *name);
+
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
 
 #ifdef CONFIG_FS_POSIX_ACL
diff --git a/fs/xattr.c b/fs/xattr.c
index 1214ae7e71db..d8f7c766f28a 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -586,6 +586,17 @@ vfs_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
 }
 EXPORT_SYMBOL_GPL(vfs_removexattr);
 
+int import_xattr_name(struct xattr_name *kname, const char __user *name)
+{
+	int error = strncpy_from_user(kname->name, name,
+					sizeof(kname->name));
+	if (error == 0 || error == sizeof(kname->name))
+		return -ERANGE;
+	if (error < 0)
+		return error;
+	return 0;
+}
+
 /*
  * Extended attribute SET operations
  */
@@ -597,14 +608,10 @@ int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 	if (ctx->flags & ~(XATTR_CREATE|XATTR_REPLACE))
 		return -EINVAL;
 
-	error = strncpy_from_user(ctx->kname->name, name,
-				sizeof(ctx->kname->name));
-	if (error == 0 || error == sizeof(ctx->kname->name))
-		return  -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(ctx->kname, name);
+	if (error)
 		return error;
 
-	error = 0;
 	if (ctx->size) {
 		if (ctx->size > XATTR_SIZE_MAX)
 			return -E2BIG;
@@ -763,10 +770,8 @@ getxattr(struct mnt_idmap *idmap, struct dentry *d,
 		.flags    = 0,
 	};
 
-	error = strncpy_from_user(kname.name, name, sizeof(kname.name));
-	if (error == 0 || error == sizeof(kname.name))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 
 	error =  do_getxattr(idmap, d, &ctx);
@@ -906,12 +911,10 @@ static int path_removexattr(const char __user *pathname,
 {
 	struct path path;
 	int error;
-	char kname[XATTR_NAME_MAX + 1];
+	struct xattr_name kname;
 
-	error = strncpy_from_user(kname, name, sizeof(kname));
-	if (error == 0 || error == sizeof(kname))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 retry:
 	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
@@ -919,7 +922,7 @@ static int path_removexattr(const char __user *pathname,
 		return error;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname);
+		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname.name);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -945,23 +948,21 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
 	CLASS(fd, f)(fd);
-	char kname[XATTR_NAME_MAX + 1];
+	struct xattr_name kname;
 	int error;
 
 	if (fd_empty(f))
 		return -EBADF;
 	audit_file(fd_file(f));
 
-	error = strncpy_from_user(kname, name, sizeof(kname));
-	if (error == 0 || error == sizeof(kname))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 
 	error = mnt_want_write_file(fd_file(f));
 	if (!error) {
 		error = removexattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, kname);
+				    fd_file(f)->f_path.dentry, kname.name);
 		mnt_drop_write_file(fd_file(f));
 	}
 	return error;
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 04abf0739668..71d9e2569a2f 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -65,11 +65,8 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	if (!ix->ctx.kname)
 		return -ENOMEM;
 
-	ret = strncpy_from_user(ix->ctx.kname->name, name,
-				sizeof(ix->ctx.kname->name));
-	if (!ret || ret == sizeof(ix->ctx.kname->name))
-		ret = -ERANGE;
-	if (ret < 0) {
+	ret = import_xattr_name(ix->ctx.kname, name);
+	if (ret) {
 		kfree(ix->ctx.kname);
 		return ret;
 	}
-- 
2.39.5

Re: [PATCH 4/9] new helper: import_xattr_name()

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:25AM GMT, Al Viro wrote:
> common logics for marshalling xattr names.
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

io_uring setxattr logics duplicates stuff from fs/xattr.c; provide
saner helpers (filename_setxattr() and file_setxattr() resp.) and
use them.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  6 ++---
 fs/xattr.c       | 68 ++++++++++++++++++++++++++++++------------------
 io_uring/xattr.c | 32 +++--------------------
 3 files changed, 49 insertions(+), 57 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index b9f5ac4d39fc..be7c0da3bcec 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -285,10 +285,10 @@ ssize_t do_getxattr(struct mnt_idmap *idmap,
 		    struct dentry *d,
 		    struct kernel_xattr_ctx *ctx);
 
+int file_setxattr(struct file *file, struct kernel_xattr_ctx *ctx);
+int filename_setxattr(int dfd, struct filename *filename,
+		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
 int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
-int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct kernel_xattr_ctx *ctx);
-
 int import_xattr_name(struct xattr_name *kname, const char __user *name);
 
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
diff --git a/fs/xattr.c b/fs/xattr.c
index d8f7c766f28a..6326a1ea28e9 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -626,7 +626,7 @@ int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 	return error;
 }
 
-int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
+static int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		struct kernel_xattr_ctx *ctx)
 {
 	if (is_posix_acl_xattr(ctx->kname->name))
@@ -637,32 +637,31 @@ int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			ctx->kvalue, ctx->size, ctx->flags);
 }
 
-static int path_setxattr(const char __user *pathname,
-			 const char __user *name, const void __user *value,
-			 size_t size, int flags, unsigned int lookup_flags)
+int file_setxattr(struct file *f, struct kernel_xattr_ctx *ctx)
+{
+	int error = mnt_want_write_file(f);
+
+	if (!error) {
+		audit_file(f);
+		error = do_setxattr(file_mnt_idmap(f), f->f_path.dentry, ctx);
+		mnt_drop_write_file(f);
+	}
+	return error;
+}
+
+int filename_setxattr(int dfd, struct filename *filename,
+		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx)
 {
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
-	};
 	struct path path;
 	int error;
 
-	error = setxattr_copy(name, &ctx);
-	if (error)
-		return error;
-
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
 		goto out;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = do_setxattr(mnt_idmap(path.mnt), path.dentry, &ctx);
+		error = do_setxattr(mnt_idmap(path.mnt), path.dentry, ctx);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -672,6 +671,30 @@ static int path_setxattr(const char __user *pathname,
 	}
 
 out:
+	putname(filename);
+	return error;
+}
+
+static int path_setxattr(const char __user *pathname,
+			 const char __user *name, const void __user *value,
+			 size_t size, int flags, unsigned int lookup_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;
+
+	error = filename_setxattr(AT_FDCWD, getname(pathname), lookup_flags,
+				  &ctx);
 	kvfree(ctx.kvalue);
 	return error;
 }
@@ -707,17 +730,12 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
+
 	error = setxattr_copy(name, &ctx);
 	if (error)
 		return error;
 
-	error = mnt_want_write_file(fd_file(f));
-	if (!error) {
-		error = do_setxattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, &ctx);
-		mnt_drop_write_file(fd_file(f));
-	}
+	error = file_setxattr(fd_file(f), &ctx);
 	kvfree(ctx.kvalue);
 	return error;
 }
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 71d9e2569a2f..7f6bbfd846b9 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -200,28 +200,14 @@ int io_fsetxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	return __io_setxattr_prep(req, sqe);
 }
 
-static int __io_setxattr(struct io_kiocb *req, unsigned int issue_flags,
-			const struct path *path)
-{
-	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	int ret;
-
-	ret = mnt_want_write(path->mnt);
-	if (!ret) {
-		ret = do_setxattr(mnt_idmap(path->mnt), path->dentry, &ix->ctx);
-		mnt_drop_write(path->mnt);
-	}
-
-	return ret;
-}
-
 int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
+	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-	ret = __io_setxattr(req, issue_flags, &req->file->f_path);
+	ret = file_setxattr(req->file, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
@@ -229,23 +215,11 @@ int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
 int io_setxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
-	struct path path;
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-retry:
-	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
-	if (!ret) {
-		ret = __io_setxattr(req, issue_flags, &path);
-		path_put(&path);
-		if (retry_estale(ret, lookup_flags)) {
-			lookup_flags |= LOOKUP_REVAL;
-			goto retry;
-		}
-	}
-
+	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
-- 
2.39.5

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/1/24 7:22 PM, Al Viro wrote:
> diff --git a/io_uring/xattr.c b/io_uring/xattr.c
> index 71d9e2569a2f..7f6bbfd846b9 100644
> --- a/io_uring/xattr.c
> +++ b/io_uring/xattr.c
>  int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
>  {
> +	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
>  	int ret;
>  
>  	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
>  
> -	ret = __io_setxattr(req, issue_flags, &req->file->f_path);
> +	ret = file_setxattr(req->file, &ix->ctx);
>  	io_xattr_finish(req, ret);
>  	return IOU_OK;

This and ... ->

> -retry:
> -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
> -	if (!ret) {
> -		ret = __io_setxattr(req, issue_flags, &path);
> -		path_put(&path);
> -		if (retry_estale(ret, lookup_flags)) {
> -			lookup_flags |= LOOKUP_REVAL;
> -			goto retry;
> -		}
> -	}
> -
> +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
>  	io_xattr_finish(req, ret);
>  	return IOU_OK;

this looks like it needs an ix->filename = NULL, as
filename_{s,g}xattr() drops the reference. The previous internal helper
did not, and hence the cleanup always did it. But should work fine if
->filename is just zeroed.

Otherwise looks good. I've skimmed the other patches and didn't see
anything odd, I'll take a closer look tomorrow.

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Tue, Oct 01, 2024 at 07:34:12PM -0600, Jens Axboe wrote:

> > -retry:
> > -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
> > -	if (!ret) {
> > -		ret = __io_setxattr(req, issue_flags, &path);
> > -		path_put(&path);
> > -		if (retry_estale(ret, lookup_flags)) {
> > -			lookup_flags |= LOOKUP_REVAL;
> > -			goto retry;
> > -		}
> > -	}
> > -
> > +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
> >  	io_xattr_finish(req, ret);
> >  	return IOU_OK;
> 
> this looks like it needs an ix->filename = NULL, as
> filename_{s,g}xattr() drops the reference. The previous internal helper
> did not, and hence the cleanup always did it. But should work fine if
> ->filename is just zeroed.
> 
> Otherwise looks good. I've skimmed the other patches and didn't see
> anything odd, I'll take a closer look tomorrow.

Hmm...  I wonder if we would be better off with file{,name}_setxattr()
doing kvfree(cxt->kvalue) - it makes things easier both on the syscall
and on io_uring side.

I've added minimal fixes (zeroing ix->filename after filename_[sg]etxattr())
to 5/9 and 6/9 *and* added a followup calling conventions change at the end
of the branch.  See #work.xattr2 in the same tree; FWIW, the followup
cleanup is below; note that putname(ERR_PTR(-Ewhatever)) is an explicit
no-op, so there's no need to zero on getname() failures.

commit 67896be9ac99b3fdef82708dd06e720332c13cdc
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Tue Oct 1 22:03:16 2024 -0400

    saner calling conventions for file{,name}_setxattr()
    
    Have them consume ctx->kvalue.  That simplifies both the path_setxattrat()
    and io_uring side of things - there io_xattr_finish() is just left to
    free the xattr name and that's it.
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

diff --git a/fs/xattr.c b/fs/xattr.c
index 59cdb524412e..6bb656941bce 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -672,6 +672,7 @@ int file_setxattr(struct file *f, struct kernel_xattr_ctx *ctx)
 		error = do_setxattr(file_mnt_idmap(f), f->f_path.dentry, ctx);
 		mnt_drop_write_file(f);
 	}
+	kvfree(ctx->kvalue);
 	return error;
 }
 
@@ -697,6 +698,7 @@ int filename_setxattr(int dfd, struct filename *filename,
 	}
 
 out:
+	kvfree(ctx->kvalue);
 	putname(filename);
 	return error;
 }
@@ -731,14 +733,10 @@ static int path_setxattrat(int dfd, const char __user *pathname,
 	if (!filename) {
 		CLASS(fd, f)(dfd);
 		if (fd_empty(f))
-			error = -EBADF;
-		else
-			error = file_setxattr(fd_file(f), &ctx);
-	} else {
-		error = filename_setxattr(dfd, filename, lookup_flags, &ctx);
+			return -EBADF;
+		return file_setxattr(fd_file(f), &ctx);
 	}
-	kvfree(ctx.kvalue);
-	return error;
+	return filename_setxattr(dfd, filename, lookup_flags, &ctx);
 }
 
 SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 90277246dbea..9f3ea12f628d 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -35,9 +35,10 @@ void io_xattr_cleanup(struct io_kiocb *req)
 
 static void io_xattr_finish(struct io_kiocb *req, int ret)
 {
-	req->flags &= ~REQ_F_NEED_CLEANUP;
+	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
 
-	io_xattr_cleanup(req);
+	kfree(ix->ctx.kname);
+	req->flags &= ~REQ_F_NEED_CLEANUP;
 	io_req_set_res(req, ret, 0);
 }
 
@@ -94,12 +95,9 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
 	ix->filename = getname(path);
-	if (IS_ERR(ix->filename)) {
-		ret = PTR_ERR(ix->filename);
-		ix->filename = NULL;
-	}
-
-	return ret;
+	if (IS_ERR(ix->filename))
+		return PTR_ERR(ix->filename);
+	return 0;
 }
 
 int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
@@ -122,7 +120,6 @@ int io_getxattr(struct io_kiocb *req, unsigned int issue_flags)
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
 	ret = filename_getxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
-	ix->filename = NULL;
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
@@ -172,12 +169,9 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
 	ix->filename = getname(path);
-	if (IS_ERR(ix->filename)) {
-		ret = PTR_ERR(ix->filename);
-		ix->filename = NULL;
-	}
-
-	return ret;
+	if (IS_ERR(ix->filename))
+		return PTR_ERR(ix->filename);
+	return 0;
 }
 
 int io_fsetxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
@@ -205,7 +199,6 @@ int io_setxattr(struct io_kiocb *req, unsigned int issue_flags)
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
 	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
-	ix->filename = NULL;
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/1/24 8:08 PM, Al Viro wrote:
> On Tue, Oct 01, 2024 at 07:34:12PM -0600, Jens Axboe wrote:
> 
>>> -retry:
>>> -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
>>> -	if (!ret) {
>>> -		ret = __io_setxattr(req, issue_flags, &path);
>>> -		path_put(&path);
>>> -		if (retry_estale(ret, lookup_flags)) {
>>> -			lookup_flags |= LOOKUP_REVAL;
>>> -			goto retry;
>>> -		}
>>> -	}
>>> -
>>> +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
>>>  	io_xattr_finish(req, ret);
>>>  	return IOU_OK;
>>
>> this looks like it needs an ix->filename = NULL, as
>> filename_{s,g}xattr() drops the reference. The previous internal helper
>> did not, and hence the cleanup always did it. But should work fine if
>> ->filename is just zeroed.
>>
>> Otherwise looks good. I've skimmed the other patches and didn't see
>> anything odd, I'll take a closer look tomorrow.
> 
> Hmm...  I wonder if we would be better off with file{,name}_setxattr()
> doing kvfree(cxt->kvalue) - it makes things easier both on the syscall
> and on io_uring side.
> 
> I've added minimal fixes (zeroing ix->filename after filename_[sg]etxattr())
> to 5/9 and 6/9 *and* added a followup calling conventions change at the end
> of the branch.  See #work.xattr2 in the same tree; FWIW, the followup
> cleanup is below; note that putname(ERR_PTR(-Ewhatever)) is an explicit
> no-op, so there's no need to zero on getname() failures.

Looks good to me, thanks Al!

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Wed, Oct 02, 2024 at 12:00:45PM -0600, Jens Axboe wrote:
> On 10/1/24 8:08 PM, Al Viro wrote:
> > On Tue, Oct 01, 2024 at 07:34:12PM -0600, Jens Axboe wrote:
> > 
> >>> -retry:
> >>> -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
> >>> -	if (!ret) {
> >>> -		ret = __io_setxattr(req, issue_flags, &path);
> >>> -		path_put(&path);
> >>> -		if (retry_estale(ret, lookup_flags)) {
> >>> -			lookup_flags |= LOOKUP_REVAL;
> >>> -			goto retry;
> >>> -		}
> >>> -	}
> >>> -
> >>> +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
> >>>  	io_xattr_finish(req, ret);
> >>>  	return IOU_OK;
> >>
> >> this looks like it needs an ix->filename = NULL, as
> >> filename_{s,g}xattr() drops the reference. The previous internal helper
> >> did not, and hence the cleanup always did it. But should work fine if
> >> ->filename is just zeroed.
> >>
> >> Otherwise looks good. I've skimmed the other patches and didn't see
> >> anything odd, I'll take a closer look tomorrow.
> > 
> > Hmm...  I wonder if we would be better off with file{,name}_setxattr()
> > doing kvfree(cxt->kvalue) - it makes things easier both on the syscall
> > and on io_uring side.
> > 
> > I've added minimal fixes (zeroing ix->filename after filename_[sg]etxattr())
> > to 5/9 and 6/9 *and* added a followup calling conventions change at the end
> > of the branch.  See #work.xattr2 in the same tree; FWIW, the followup
> > cleanup is below; note that putname(ERR_PTR(-Ewhatever)) is an explicit
> > no-op, so there's no need to zero on getname() failures.
> 
> Looks good to me, thanks Al!

I'm still not sure if the calling conventions change is right - in the current
form the last commit in there leaks ctx.kvalue in -EBADF case.  It's easy to
fix up, but... as far as I'm concerned, a large part of the point of the
exercise is to come up with the right model for the calling conventions
for that family of APIs.

I really want to get rid of that ad-hoc crap.  If we are to have what amounts
to the alternative syscall interface, we'd better get it right.  I'm perfectly
fine with having a set of "this is what the syscall is doing past marshalling
arguments" primitives, but let's make sure they are properly documented and
do not have landmines for callers to step into...

Questions on the io_uring side:
	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
Am I missing something subtle here?
	* what's to guarantee that pointers fetched by io_file_get_fixed()
called from io_assing_file() will stay valid?  You do not bump the struct
file refcount in this case, after all; what's to prevent unregistration
from the main thread while the worker is getting through your request?
Is that what the break on node->refs in the loop in io_rsrc_node_ref_zero()
is about?  Or am I barking at the wrong tree here?  I realize that I'm about
the last person to complain about the lack of documentation, but...

	FWIW, my impression is that you have a list of nodes corresponding
to overall resource states (which includes the file reference table) and
have each borrow bump the use count on the node corresponding to the current
state (at the tail of the list?)
	Each removal adds new node to the tail of the list, sticks the
file reference there and tries to trigger io_rsrc_node_ref_zero() (which,
for some reason, takes node instead of the node->ctx, even though it
doesn't give a rat's arse about anything else in its argument).
	If there are nodes at the head of the list with zero use count,
that takes them out, stopping at the first in-use node.  File reference
stashed in a node is dropped when it's taken out.

	If the above is more or less correct (and I'm pretty sure that it
misses quite a few critical points), the rules would be equivalent to
	+ there is a use count associated with the table state.
	+ before we borrow a file reference from the table, we must bump
that use count (see the call of __io_req_set_rsrc_node() in
io_file_get_fixed()) and arrange for dropping it once we are done with
the reference (io_put_rsrc_node() when freeing request, in io_free_batch_list())
	+ any removals from the table will switch to new state; dropping
the removed reference is guaranteed to be delayed until use counts on
all earlier states drop to zero.

	How far are those rules from being accurate and how incomplete
they are?  I hadn't looked into the quiescence-related stuff, which might
or might not be relevant...

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/2/24 3:19 PM, Al Viro wrote:
> On Wed, Oct 02, 2024 at 12:00:45PM -0600, Jens Axboe wrote:
>> On 10/1/24 8:08 PM, Al Viro wrote:
>>> On Tue, Oct 01, 2024 at 07:34:12PM -0600, Jens Axboe wrote:
>>>
>>>>> -retry:
>>>>> -	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
>>>>> -	if (!ret) {
>>>>> -		ret = __io_setxattr(req, issue_flags, &path);
>>>>> -		path_put(&path);
>>>>> -		if (retry_estale(ret, lookup_flags)) {
>>>>> -			lookup_flags |= LOOKUP_REVAL;
>>>>> -			goto retry;
>>>>> -		}
>>>>> -	}
>>>>> -
>>>>> +	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
>>>>>  	io_xattr_finish(req, ret);
>>>>>  	return IOU_OK;
>>>>
>>>> this looks like it needs an ix->filename = NULL, as
>>>> filename_{s,g}xattr() drops the reference. The previous internal helper
>>>> did not, and hence the cleanup always did it. But should work fine if
>>>> ->filename is just zeroed.
>>>>
>>>> Otherwise looks good. I've skimmed the other patches and didn't see
>>>> anything odd, I'll take a closer look tomorrow.
>>>
>>> Hmm...  I wonder if we would be better off with file{,name}_setxattr()
>>> doing kvfree(cxt->kvalue) - it makes things easier both on the syscall
>>> and on io_uring side.
>>>
>>> I've added minimal fixes (zeroing ix->filename after filename_[sg]etxattr())
>>> to 5/9 and 6/9 *and* added a followup calling conventions change at the end
>>> of the branch.  See #work.xattr2 in the same tree; FWIW, the followup
>>> cleanup is below; note that putname(ERR_PTR(-Ewhatever)) is an explicit
>>> no-op, so there's no need to zero on getname() failures.
>>
>> Looks good to me, thanks Al!
> 
> I'm still not sure if the calling conventions change is right - in the
> current form the last commit in there leaks ctx.kvalue in -EBADF case.
> It's easy to fix up, but... as far as I'm concerned, a large part of
> the point of the exercise is to come up with the right model for the
> calling conventions for that family of APIs.

The reason I liked the putname() is that it's unconditional - the caller
can rely on it being put, regardless of the return value. So I'd say the
same should be true for ctx.kvalue, and if not, the caller should still
free it. That's the path of least surprise - no leak for the least
tested error path, and no UAF in the success case.

For the put case, most other abstractions end up being something ala:

helper(struct file *file, ...)
{
	actual actions
}

regular_sys_call(int fd, ...)
{
	struct fd f;
	int ret = -EBADF;

	f = fdget(fd);
	if (f.file) {
		ret = helper(f.file, ...);
		fdput(f();
	}

	return ret;
}

where io_uring will use helper(), and where the file reference is
assumed to be valid for helper() and helper() will not put a reference
to it.

That's a bit different than your putname() case, but I think as long as
it's consistent regardless of return value, then either approach is
fine. Maybe just add a comment about that? At least for the consistent
case, if it blows up, it'll blow up instantly rather than be a surprise
down the line for "case x,y,z doesn't put it" or "case x,y,z always puts
in, normal one does not".

> I really want to get rid of that ad-hoc crap.  If we are to have what
> amounts to the alternative syscall interface, we'd better get it
> right.  I'm perfectly fine with having a set of "this is what the
> syscall is doing past marshalling arguments" primitives, but let's
> make sure they are properly documented and do not have landmines for
> callers to step into...

Fully agree.

> Questions on the io_uring side:
> 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
> Am I missing something subtle here?

Right, it could be allowed for fgetxattr on the io_uring side. Anything
that passes in a struct file would be fair game to enable it on.
Anything that passes in a path (eg a non-fd value), it obviously
wouldn't make sense anyway.

> 	* what's to guarantee that pointers fetched by io_file_get_fixed()
> called from io_assing_file() will stay valid?  You do not bump the struct
> file refcount in this case, after all; what's to prevent unregistration
> from the main thread while the worker is getting through your request?
> Is that what the break on node->refs in the loop in io_rsrc_node_ref_zero()
> is about?  Or am I barking at the wrong tree here?  I realize that I'm about
> the last person to complain about the lack of documentation, but...
> 
> 	FWIW, my impression is that you have a list of nodes corresponding
> to overall resource states (which includes the file reference table) and
> have each borrow bump the use count on the node corresponding to the current
> state (at the tail of the list?)
> 	Each removal adds new node to the tail of the list, sticks the
> file reference there and tries to trigger io_rsrc_node_ref_zero() (which,
> for some reason, takes node instead of the node->ctx, even though it
> doesn't give a rat's arse about anything else in its argument).
> 	If there are nodes at the head of the list with zero use count,
> that takes them out, stopping at the first in-use node.  File reference
> stashed in a node is dropped when it's taken out.
> 
> 	If the above is more or less correct (and I'm pretty sure that it
> misses quite a few critical points), the rules would be equivalent to
> 	+ there is a use count associated with the table state.
> 	+ before we borrow a file reference from the table, we must bump
> that use count (see the call of __io_req_set_rsrc_node() in
> io_file_get_fixed()) and arrange for dropping it once we are done with
> the reference (io_put_rsrc_node() when freeing request, in io_free_batch_list())
> 	+ any removals from the table will switch to new state; dropping
> the removed reference is guaranteed to be delayed until use counts on
> all earlier states drop to zero.
> 
> 	How far are those rules from being accurate and how incomplete
> they are?  I hadn't looked into the quiescence-related stuff, which might
> or might not be relevant...

That is pretty darn accurate. The ordering of the rsrc nodes and the
break ensure that it stays valid until anything using it has completed.
And yes it would be nice to document that code a bit, but honestly I'd
much rather just make it more obviously referenced if that can be done
cheaply enough. For now, I'll add some comments, and hope you do the
same on your side! Because I don't ever remember seeing an Al comment.
Great emails, for sure, but not really comments.

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Wed, Oct 02, 2024 at 04:55:22PM -0600, Jens Axboe wrote:

> The reason I liked the putname() is that it's unconditional - the caller
> can rely on it being put, regardless of the return value. So I'd say the
> same should be true for ctx.kvalue, and if not, the caller should still
> free it. That's the path of least surprise - no leak for the least
> tested error path, and no UAF in the success case.

The problem with ctx.kvalue is that on the syscall side there's a case when
we do not call either file_setxattr() or filename_setxattr() - -EBADF.
And it's a lot more convenient to do setxattr_copy() first, so we end
up with a lovely landmine:
        filename = getname_xattr(pathname, at_flags);
	if (!filename) {
		CLASS(fd, f)(dfd);
		if (fd_empty(f)) {
			kfree(ctx.kvalue); // lest we leak
			return -EBADF;
		}
		return file_setxattr(fd_file(f), &ctx);
	}
	return filename_setxattr(dfd, filename, lookup_flags, &ctx);

That's asking for trouble, obviously.  So I think we ought to consume
filename (in filename_...()) case, leave struct file reference alone
(we have to - it might have been borrowed rather than cloned) and leave
->kvalue unchanged.  Yes, it ends up being more clumsy, but at least
it's consistent between the cases...

As for consuming filename...  On the syscall side it allows things like
SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
{
        return do_mkdirat(dfd, getname(pathname), mode);
}  
which is better than the alternatives - I mean, that's
SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
{
	struct filename *filename = getname(pathname);
	int res = do_mkdirat(dfd, filename, mode);
	putname(filename);
	return ret;
}  
or 
SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
{
	struct filename *filename __free(putname) = getname(pathname);
	return do_mkdirat(dfd, filename, mode);
}
and both stink, if for different reasons ;-/  Having those things consume
(unconditionally) is better, IMO.

Hell knows; let's go with what I described above for now and see where it leads
when more such helpers are regularized.

> That's a bit different than your putname() case, but I think as long as
> it's consistent regardless of return value, then either approach is
> fine. Maybe just add a comment about that? At least for the consistent
> case, if it blows up, it'll blow up instantly rather than be a surprise
> down the line for "case x,y,z doesn't put it" or "case x,y,z always puts
> in, normal one does not".

Obviously.

> > Questions on the io_uring side:
> > 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
> > Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
> > Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
> > Am I missing something subtle here?
> 
> Right, it could be allowed for fgetxattr on the io_uring side. Anything
> that passes in a struct file would be fair game to enable it on.
> Anything that passes in a path (eg a non-fd value), it obviously
> wouldn't make sense anyway.

OK, done and force-pushed into #work.xattr.

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/5/24 11:28 PM, Al Viro wrote:
> On Wed, Oct 02, 2024 at 04:55:22PM -0600, Jens Axboe wrote:
> 
>> The reason I liked the putname() is that it's unconditional - the caller
>> can rely on it being put, regardless of the return value. So I'd say the
>> same should be true for ctx.kvalue, and if not, the caller should still
>> free it. That's the path of least surprise - no leak for the least
>> tested error path, and no UAF in the success case.
> 
> The problem with ctx.kvalue is that on the syscall side there's a case when
> we do not call either file_setxattr() or filename_setxattr() - -EBADF.
> And it's a lot more convenient to do setxattr_copy() first, so we end
> up with a lovely landmine:
>         filename = getname_xattr(pathname, at_flags);
> 	if (!filename) {
> 		CLASS(fd, f)(dfd);
> 		if (fd_empty(f)) {
> 			kfree(ctx.kvalue); // lest we leak
> 			return -EBADF;
> 		}
> 		return file_setxattr(fd_file(f), &ctx);
> 	}
> 	return filename_setxattr(dfd, filename, lookup_flags, &ctx);
> 
> That's asking for trouble, obviously.  So I think we ought to consume
> filename (in filename_...()) case, leave struct file reference alone
> (we have to - it might have been borrowed rather than cloned) and leave
> ->kvalue unchanged.  Yes, it ends up being more clumsy, but at least
> it's consistent between the cases...
> 
> As for consuming filename...  On the syscall side it allows things like
> SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
> {
>         return do_mkdirat(dfd, getname(pathname), mode);
> }  
> which is better than the alternatives - I mean, that's
> SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
> {
> 	struct filename *filename = getname(pathname);
> 	int res = do_mkdirat(dfd, filename, mode);
> 	putname(filename);
> 	return ret;
> }  
> or 
> SYSCALL_DEFINE3(mkdirat, int, dfd, const char __user *, pathname, umode_t, mode)
> {
> 	struct filename *filename __free(putname) = getname(pathname);
> 	return do_mkdirat(dfd, filename, mode);
> }
> and both stink, if for different reasons ;-/  Having those things consume
> (unconditionally) is better, IMO.
> 
> Hell knows; let's go with what I described above for now and see where
> it leads when more such helpers are regularized.

Sounds like a plan.

>>> Questions on the io_uring side:
>>> 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
>>> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
>>> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
>>> Am I missing something subtle here?
>>
>> Right, it could be allowed for fgetxattr on the io_uring side. Anything
>> that passes in a struct file would be fair game to enable it on.
>> Anything that passes in a path (eg a non-fd value), it obviously
>> wouldn't make sense anyway.
> 
> OK, done and force-pushed into #work.xattr.

I just checked, and while I think this is fine to do for the 'fd' taking
{s,g}etxattr, I don't think the path taking ones should allow
IOSQE_FIXED_FILE being set. It's nonsensical, as they don't take a file
descriptor. So I'd prefer if we kept it to just the f* variants. I can
just make this tweak in my io_uring 6.12 branch and get it upstream this
week, that'll take it out of your hands.

What do you think?

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/7/24 12:09 PM, Jens Axboe wrote:
>>>> Questions on the io_uring side:
>>>> 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
>>>> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
>>>> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
>>>> Am I missing something subtle here?
>>>
>>> Right, it could be allowed for fgetxattr on the io_uring side. Anything
>>> that passes in a struct file would be fair game to enable it on.
>>> Anything that passes in a path (eg a non-fd value), it obviously
>>> wouldn't make sense anyway.
>>
>> OK, done and force-pushed into #work.xattr.
> 
> I just checked, and while I think this is fine to do for the 'fd' taking
> {s,g}etxattr, I don't think the path taking ones should allow
> IOSQE_FIXED_FILE being set. It's nonsensical, as they don't take a file
> descriptor. So I'd prefer if we kept it to just the f* variants. I can
> just make this tweak in my io_uring 6.12 branch and get it upstream this
> week, that'll take it out of your hands.
> 
> What do you think?

Like the below. You can update yours if you want, or I can shove this
into 6.12, whatever is the easiest for you.


diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 6cf41c3bc369..4b68c282c91a 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -48,9 +48,6 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	const char __user *name;
 	int ret;
 
-	if (unlikely(req->flags & REQ_F_FIXED_FILE))
-		return -EBADF;
-
 	ix->filename = NULL;
 	ix->ctx.kvalue = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
@@ -90,6 +87,9 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	const char __user *path;
 	int ret;
 
+	if (unlikely(req->flags & REQ_F_FIXED_FILE))
+		return -EBADF;
+
 	ret = __io_getxattr_prep(req, sqe);
 	if (ret)
 		return ret;
@@ -152,9 +152,6 @@ static int __io_setxattr_prep(struct io_kiocb *req,
 	const char __user *name;
 	int ret;
 
-	if (unlikely(req->flags & REQ_F_FIXED_FILE))
-		return -EBADF;
-
 	ix->filename = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	ix->ctx.cvalue = u64_to_user_ptr(READ_ONCE(sqe->addr2));
@@ -183,6 +180,9 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	const char __user *path;
 	int ret;
 
+	if (unlikely(req->flags & REQ_F_FIXED_FILE))
+		return -EBADF;
+
 	ret = __io_setxattr_prep(req, sqe);
 	if (ret)
 		return ret;

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Mon, Oct 07, 2024 at 12:20:20PM -0600, Jens Axboe wrote:
> On 10/7/24 12:09 PM, Jens Axboe wrote:
> >>>> Questions on the io_uring side:
> >>>> 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
> >>>> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
> >>>> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
> >>>> Am I missing something subtle here?
> >>>
> >>> Right, it could be allowed for fgetxattr on the io_uring side. Anything
> >>> that passes in a struct file would be fair game to enable it on.
> >>> Anything that passes in a path (eg a non-fd value), it obviously
> >>> wouldn't make sense anyway.
> >>
> >> OK, done and force-pushed into #work.xattr.
> > 
> > I just checked, and while I think this is fine to do for the 'fd' taking
> > {s,g}etxattr, I don't think the path taking ones should allow
> > IOSQE_FIXED_FILE being set. It's nonsensical, as they don't take a file
> > descriptor. So I'd prefer if we kept it to just the f* variants. I can
> > just make this tweak in my io_uring 6.12 branch and get it upstream this
> > week, that'll take it out of your hands.
> > 
> > What do you think?
> 
> Like the below. You can update yours if you want, or I can shove this
> into 6.12, whatever is the easiest for you.

Can I put your s-o-b on that, with e.g.

io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE

Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
is fine - these do not take a file descriptor, so such combination
makes no sense.  The checks are misplaced, though - as it is, they
triggers on IORING_OP_F[GS]ETXATTR as well, and those do take 
a file reference, no matter the origin. 

as commit message?

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/7/24 3:20 PM, Al Viro wrote:
> On Mon, Oct 07, 2024 at 12:20:20PM -0600, Jens Axboe wrote:
>> On 10/7/24 12:09 PM, Jens Axboe wrote:
>>>>>> Questions on the io_uring side:
>>>>>> 	* you usually reject REQ_F_FIXED_FILE for ...at() at ->prep() time.
>>>>>> Fine, but... what's the point of doing that in IORING_OP_FGETXATTR case?
>>>>>> Or IORING_OP_GETXATTR, for that matter, since you pass AT_FDCWD anyway...
>>>>>> Am I missing something subtle here?
>>>>>
>>>>> Right, it could be allowed for fgetxattr on the io_uring side. Anything
>>>>> that passes in a struct file would be fair game to enable it on.
>>>>> Anything that passes in a path (eg a non-fd value), it obviously
>>>>> wouldn't make sense anyway.
>>>>
>>>> OK, done and force-pushed into #work.xattr.
>>>
>>> I just checked, and while I think this is fine to do for the 'fd' taking
>>> {s,g}etxattr, I don't think the path taking ones should allow
>>> IOSQE_FIXED_FILE being set. It's nonsensical, as they don't take a file
>>> descriptor. So I'd prefer if we kept it to just the f* variants. I can
>>> just make this tweak in my io_uring 6.12 branch and get it upstream this
>>> week, that'll take it out of your hands.
>>>
>>> What do you think?
>>
>> Like the below. You can update yours if you want, or I can shove this
>> into 6.12, whatever is the easiest for you.
> 
> Can I put your s-o-b on that, with e.g.
> 
> io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
> 
> Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
> is fine - these do not take a file descriptor, so such combination
> makes no sense.  The checks are misplaced, though - as it is, they
> triggers on IORING_OP_F[GS]ETXATTR as well, and those do take 
> a file reference, no matter the origin. 

Yep that's perfect, officially:

Signed-off-by: Jens Axboe <axboe@kernel.dk>

Thanks Al!

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Mon, Oct 07, 2024 at 04:29:29PM -0600, Jens Axboe wrote:
> > Can I put your s-o-b on that, with e.g.
> > 
> > io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
> > 
> > Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
> > is fine - these do not take a file descriptor, so such combination
> > makes no sense.  The checks are misplaced, though - as it is, they
> > triggers on IORING_OP_F[GS]ETXATTR as well, and those do take 
> > a file reference, no matter the origin. 
> 
> Yep that's perfect, officially:
> 
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> 
> Thanks Al!

OK, updated and force-pushed (with slight reordering).  I can almost
promise no-rebase mode for that thing from now on, as long as nobody
on fsdevel objects to fs/xattr.c part of things after I repost the
series in the current form.

One possible exception: I'm not sure that fs/internal.h is a good
place for those primitives.  OTOH, any bikeshedding in that direction
can be delayed until the next cycle...

To expand the circle of potential bikeshedders: s/do_mkdirat/filename_mkdirat/
is a reasonable idea for this series, innit?  How about turning e.g.

int __init init_mkdir(const char *pathname, umode_t mode)
{
        struct dentry *dentry;
        struct path path;
        int error;

        dentry = kern_path_create(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY);
        if (IS_ERR(dentry))
                return PTR_ERR(dentry);
        mode = mode_strip_umask(d_inode(path.dentry), mode);
        error = security_path_mkdir(&path, dentry, mode);
        if (!error)
                error = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
                                  dentry, mode);
        done_path_create(&path, dentry);
        return error;
}

into

int __init init_mkdir(const char *pathname, umode_t mode)
{
	return filename_mkdirat(AT_FDCWD, getname_kernel(pathname), mode);
}

reducing the duplication?  It really should not be accessible to random
places in the kernel, but syscalls in core VFS + io_uring interface for
the same + possibly init/*.c...

OTOH, I'm afraid to let the "but our driver is sooo special!" crowd play
with the full set of syscalls...  init_syscalls.h is already bad enough.
Hell knows, fs/internal.h just might be a bit of deterrent...

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Jens Axboe]

On 10/7/24 5:58 PM, Al Viro wrote:
> On Mon, Oct 07, 2024 at 04:29:29PM -0600, Jens Axboe wrote:
>>> Can I put your s-o-b on that, with e.g.
>>>
>>> io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
>>>
>>> Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
>>> is fine - these do not take a file descriptor, so such combination
>>> makes no sense.  The checks are misplaced, though - as it is, they
>>> triggers on IORING_OP_F[GS]ETXATTR as well, and those do take 
>>> a file reference, no matter the origin. 
>>
>> Yep that's perfect, officially:
>>
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>
>> Thanks Al!
> 
> OK, updated and force-pushed (with slight reordering).  I can almost
> promise no-rebase mode for that thing from now on, as long as nobody
> on fsdevel objects to fs/xattr.c part of things after I repost the
> series in the current form.

No worries on my end in terms of rebasing, I have no plans to touch
xattr.c for the coming series. Risk of conflict should be very low, so I
don't even need to pull that in.

> One possible exception: I'm not sure that fs/internal.h is a good
> place for those primitives.  OTOH, any bikeshedding in that direction
> can be delayed until the next cycle...

It ended up just being the defacto place to shove declarations for
things like that. But it always felt a bit dirty, particularly needing
to include that from the io_uring side as it moved out of fs/ as well.
Would indeed be nice to get that cleaned up a bit.

> To expand the circle of potential bikeshedders: s/do_mkdirat/filename_mkdirat/
> is a reasonable idea for this series, innit?  How about turning e.g.
> 
> int __init init_mkdir(const char *pathname, umode_t mode)
> {
>         struct dentry *dentry;
>         struct path path;
>         int error;
> 
>         dentry = kern_path_create(AT_FDCWD, pathname, &path, LOOKUP_DIRECTORY);
>         if (IS_ERR(dentry))
>                 return PTR_ERR(dentry);
>         mode = mode_strip_umask(d_inode(path.dentry), mode);
>         error = security_path_mkdir(&path, dentry, mode);
>         if (!error)
>                 error = vfs_mkdir(mnt_idmap(path.mnt), path.dentry->d_inode,
>                                   dentry, mode);
>         done_path_create(&path, dentry);
>         return error;
> }
> 
> into
> 
> int __init init_mkdir(const char *pathname, umode_t mode)
> {
> 	return filename_mkdirat(AT_FDCWD, getname_kernel(pathname), mode);
> }
> 
> reducing the duplication?  It really should not be accessible to random
> places in the kernel, but syscalls in core VFS + io_uring interface for
> the same + possibly init/*.c...
> 
> OTOH, I'm afraid to let the "but our driver is sooo special!" crowd play
> with the full set of syscalls...  init_syscalls.h is already bad enough.
> Hell knows, fs/internal.h just might be a bit of deterrent...

Deduping it is a good thing, suggestion looks good to me. For random
drivers, very much agree. But are there any of these symbols we end up
exporting? That tends to put a damper on the enthusiasm...

-- 
Jens Axboe

Re: [PATCH 5/9] replace do_setxattr() with saner helpers.

[Al Viro]

On Mon, Oct 07, 2024 at 07:58:15PM -0600, Jens Axboe wrote:

> > OTOH, I'm afraid to let the "but our driver is sooo special!" crowd play
> > with the full set of syscalls...  init_syscalls.h is already bad enough.
> > Hell knows, fs/internal.h just might be a bit of deterrent...
> 
> Deduping it is a good thing, suggestion looks good to me. For random
> drivers, very much agree. But are there any of these symbols we end up
> exporting? That tends to put a damper on the enthusiasm...

	You wish...  init_unlink() et.al. are not just not exported, they
are freed once the system is booted.  Doesn't stop that kind of magic being
attempted.

[PATCH 6/9] replace do_getxattr() with saner helpers.

[Al Viro]

similar to do_setxattr() in the previous commit...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  8 ++---
 fs/xattr.c       | 87 ++++++++++++++++++++++++++++--------------------
 io_uring/xattr.c | 22 ++----------
 3 files changed, 56 insertions(+), 61 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index be7c0da3bcec..8001efd1f047 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -280,11 +280,9 @@ struct kernel_xattr_ctx {
 	unsigned int flags;
 };
 
-
-ssize_t do_getxattr(struct mnt_idmap *idmap,
-		    struct dentry *d,
-		    struct kernel_xattr_ctx *ctx);
-
+ssize_t file_getxattr(struct file *file, struct kernel_xattr_ctx *ctx);
+ssize_t filename_getxattr(int dfd, struct filename *filename,
+			  unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
 int file_setxattr(struct file *file, struct kernel_xattr_ctx *ctx);
 int filename_setxattr(int dfd, struct filename *filename,
 		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
diff --git a/fs/xattr.c b/fs/xattr.c
index 6326a1ea28e9..a0e304c65d51 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -743,27 +743,28 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 /*
  * Extended attribute GET operations
  */
-ssize_t
+static ssize_t
 do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
 	struct kernel_xattr_ctx *ctx)
 {
 	ssize_t error;
 	char *kname = ctx->kname->name;
+	void *kvalue = NULL;
 
 	if (ctx->size) {
 		if (ctx->size > XATTR_SIZE_MAX)
 			ctx->size = XATTR_SIZE_MAX;
-		ctx->kvalue = kvzalloc(ctx->size, GFP_KERNEL);
-		if (!ctx->kvalue)
+		kvalue = kvzalloc(ctx->size, GFP_KERNEL);
+		if (!kvalue)
 			return -ENOMEM;
 	}
 
-	if (is_posix_acl_xattr(ctx->kname->name))
-		error = do_get_acl(idmap, d, kname, ctx->kvalue, ctx->size);
+	if (is_posix_acl_xattr(kname))
+		error = do_get_acl(idmap, d, kname, kvalue, ctx->size);
 	else
-		error = vfs_getxattr(idmap, d, kname, ctx->kvalue, ctx->size);
+		error = vfs_getxattr(idmap, d, kname, kvalue, ctx->size);
 	if (error > 0) {
-		if (ctx->size && copy_to_user(ctx->value, ctx->kvalue, error))
+		if (ctx->size && copy_to_user(ctx->value, kvalue, error))
 			error = -EFAULT;
 	} else if (error == -ERANGE && ctx->size >= XATTR_SIZE_MAX) {
 		/* The file system tried to returned a value bigger
@@ -771,52 +772,55 @@ do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
 		error = -E2BIG;
 	}
 
+	kvfree(kvalue);
 	return error;
 }
 
-static ssize_t
-getxattr(struct mnt_idmap *idmap, struct dentry *d,
-	 const char __user *name, void __user *value, size_t size)
+ssize_t file_getxattr(struct file *f, struct kernel_xattr_ctx *ctx)
 {
-	ssize_t error;
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.value    = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = 0,
-	};
-
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-
-	error =  do_getxattr(idmap, d, &ctx);
-
-	kvfree(ctx.kvalue);
-	return error;
+	audit_file(f);
+	return do_getxattr(file_mnt_idmap(f), f->f_path.dentry, ctx);
 }
 
-static ssize_t path_getxattr(const char __user *pathname,
-			     const char __user *name, void __user *value,
-			     size_t size, unsigned int lookup_flags)
+ssize_t filename_getxattr(int dfd, struct filename *filename,
+			  unsigned int lookup_flags, struct kernel_xattr_ctx *ctx)
 {
 	struct path path;
 	ssize_t error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
-	error = getxattr(mnt_idmap(path.mnt), path.dentry, name, value, size);
+		goto out;
+	error = do_getxattr(mnt_idmap(path.mnt), path.dentry, ctx);
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static ssize_t path_getxattr(const char __user *pathname,
+			     const char __user *name, void __user *value,
+			     size_t size, unsigned int lookup_flags)
+{
+	ssize_t error;
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.value    = value,
+		.size     = size,
+		.kname    = &kname,
+		.flags    = 0,
+	};
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return filename_getxattr(AT_FDCWD, getname(pathname), lookup_flags, &ctx);
+}
+
 SYSCALL_DEFINE4(getxattr, const char __user *, pathname,
 		const char __user *, name, void __user *, value, size_t, size)
 {
@@ -832,13 +836,22 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
+	ssize_t error;
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.value    = value,
+		.size     = size,
+		.kname    = &kname,
+		.flags    = 0,
+	};
 	CLASS(fd, f)(fd);
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
-	return getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
-			 name, value, size);
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return file_getxattr(fd_file(f), &ctx);
 }
 
 /*
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 7f6bbfd846b9..0eaeaed39649 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -54,7 +54,7 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	ix->filename = NULL;
 	ix->ctx.kvalue = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
-	ix->ctx.cvalue = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+	ix->ctx.value = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	ix->ctx.size = READ_ONCE(sqe->len);
 	ix->ctx.flags = READ_ONCE(sqe->xattr_flags);
 
@@ -109,10 +109,7 @@ int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-	ret = do_getxattr(file_mnt_idmap(req->file),
-			req->file->f_path.dentry,
-			&ix->ctx);
-
+	ret = file_getxattr(req->file, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
@@ -120,24 +117,11 @@ int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
 int io_getxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
-	struct path path;
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-retry:
-	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
-	if (!ret) {
-		ret = do_getxattr(mnt_idmap(path.mnt), path.dentry, &ix->ctx);
-
-		path_put(&path);
-		if (retry_estale(ret, lookup_flags)) {
-			lookup_flags |= LOOKUP_REVAL;
-			goto retry;
-		}
-	}
-
+	ret = filename_getxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
-- 
2.39.5

Re: [PATCH 6/9] replace do_getxattr() with saner helpers.

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:27AM GMT, Al Viro wrote:
> similar to do_setxattr() in the previous commit...
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 7/9] new helpers: file_listxattr(), filename_listxattr()

[Al Viro]

switch path_listxattr() and flistxattr(2) to those

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index a0e304c65d51..0a1da16f74b1 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -886,24 +886,42 @@ listxattr(struct dentry *d, char __user *list, size_t size)
 	return error;
 }
 
-static ssize_t path_listxattr(const char __user *pathname, char __user *list,
-			      size_t size, unsigned int lookup_flags)
+static
+ssize_t file_listxattr(struct file *f, char __user *list, size_t size)
+{
+	audit_file(f);
+	return listxattr(f->f_path.dentry, list, size);
+}
+
+static
+ssize_t filename_listxattr(int dfd, struct filename *filename,
+			   unsigned int lookup_flags,
+			   char __user *list, size_t size)
 {
 	struct path path;
 	ssize_t error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
+		goto out;
 	error = listxattr(path.dentry, list, size);
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static ssize_t path_listxattr(const char __user *pathname, char __user *list,
+			      size_t size, unsigned int lookup_flags)
+{
+	return filename_listxattr(AT_FDCWD, getname(pathname), lookup_flags,
+				  list, size);
+}
+
 SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
@@ -922,8 +940,7 @@ SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
-	return listxattr(fd_file(f)->f_path.dentry, list, size);
+	return file_listxattr(fd_file(f), list, size);
 }
 
 /*
-- 
2.39.5

Re: [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr()

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:28AM GMT, Al Viro wrote:
> switch path_listxattr() and flistxattr(2) to those
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 8/9] new helpers: file_removexattr(), filename_removexattr()

[Al Viro]

switch path_removexattrat() and fremovexattr(2) to those

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 52 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 34 insertions(+), 18 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 0a1da16f74b1..6f87f23c0e84 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -954,23 +954,32 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d, const char *name)
 	return vfs_removexattr(idmap, d, name);
 }
 
-static int path_removexattr(const char __user *pathname,
-			    const char __user *name, unsigned int lookup_flags)
+static int file_removexattr(struct file *f, struct xattr_name *kname)
+{
+	int error = mnt_want_write_file(f);
+
+	if (!error) {
+		audit_file(f);
+		error = removexattr(file_mnt_idmap(f),
+				    f->f_path.dentry, kname->name);
+		mnt_drop_write_file(f);
+	}
+	return error;
+}
+
+static int filename_removexattr(int dfd, struct filename *filename,
+				unsigned int lookup_flags, struct xattr_name *kname)
 {
 	struct path path;
 	int error;
-	struct xattr_name kname;
 
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
+		goto out;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname.name);
+		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname->name);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -978,9 +987,24 @@ static int path_removexattr(const char __user *pathname,
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static int path_removexattr(const char __user *pathname,
+			    const char __user *name, unsigned int lookup_flags)
+{
+	struct xattr_name kname;
+	int error;
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return filename_removexattr(AT_FDCWD, getname(pathname), lookup_flags,
+				    &kname);
+}
+
 SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
 		const char __user *, name)
 {
@@ -1001,19 +1025,11 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
 
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-
-	error = mnt_want_write_file(fd_file(f));
-	if (!error) {
-		error = removexattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, kname.name);
-		mnt_drop_write_file(fd_file(f));
-	}
-	return error;
+	return file_removexattr(fd_file(f), &kname);
 }
 
 int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
-- 
2.39.5

Re: [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr()

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:29AM GMT, Al Viro wrote:
> switch path_removexattrat() and fremovexattr(2) to those
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[PATCH 9/9] fs/xattr: add *at family syscalls

[Al Viro]

From: Christian Göttsche <cgzones@googlemail.com>

Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
removexattrat().  Those can be used to operate on extended attributes,
especially security related ones, either relative to a pinned directory
or on a file descriptor without read access, avoiding a
/proc/<pid>/fd/<fd> detour, requiring a mounted procfs.

One use case will be setfiles(8) setting SELinux file contexts
("security.selinux") without race conditions and without a file
descriptor opened with read access requiring SELinux read permission.

Use the do_{name}at() pattern from fs/open.c.

Pass the value of the extended attribute, its length, and for
setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
struct xattr_args to not exceed six syscall arguments and not
merging the AT_* and XATTR_* flags.

[AV: fixes by Christian Brauner folded in, the entire thing rebased on
top of {filename,file}_...xattr() primitives, treatment of empty
pathnames regularized.  As the result, AT_EMPTY_PATH+NULL handling
is cheap, so f...(2) can use it]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Link: https://lore.kernel.org/r/20240426162042.191916-1-cgoettsche@seltendoof.de
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
CC: x86@kernel.org
CC: linux-alpha@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-ia64@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: linux-mips@vger.kernel.org
CC: linux-parisc@vger.kernel.org
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-s390@vger.kernel.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: audit@vger.kernel.org
CC: linux-arch@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
[brauner: slight tweaks]
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 arch/alpha/kernel/syscalls/syscall.tbl      |   4 +
 arch/arm/tools/syscall.tbl                  |   4 +
 arch/m68k/kernel/syscalls/syscall.tbl       |   4 +
 arch/microblaze/kernel/syscalls/syscall.tbl |   4 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   |   4 +
 arch/parisc/kernel/syscalls/syscall.tbl     |   4 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |   4 +
 arch/s390/kernel/syscalls/syscall.tbl       |   4 +
 arch/sh/kernel/syscalls/syscall.tbl         |   4 +
 arch/sparc/kernel/syscalls/syscall.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_32.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_64.tbl      |   4 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |   4 +
 fs/xattr.c                                  | 271 ++++++++++++++------
 include/asm-generic/audit_change_attr.h     |   6 +
 include/linux/syscalls.h                    |  13 +
 include/linux/xattr.h                       |   4 +
 include/uapi/asm-generic/unistd.h           |  11 +-
 include/uapi/linux/xattr.h                  |   7 +
 21 files changed, 286 insertions(+), 86 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 74720667fe09..c59d53d6d3f3 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -502,3 +502,7 @@
 570	common	lsm_set_self_attr		sys_lsm_set_self_attr
 571	common	lsm_list_modules		sys_lsm_list_modules
 572	common  mseal				sys_mseal
+573	common	setxattrat			sys_setxattrat
+574	common	getxattrat			sys_getxattrat
+575	common	listxattrat			sys_listxattrat
+576	common	removexattrat			sys_removexattrat
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 23c98203c40f..49eeb2ad8dbd 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -477,3 +477,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 22a3cbd4c602..f5ed71f1910d 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -462,3 +462,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 2b81a6bd78b2..680f568b77f2 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -468,3 +468,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 953f5b7dc723..0b9b7e25b69a 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -401,3 +401,7 @@
 460	n32	lsm_set_self_attr		sys_lsm_set_self_attr
 461	n32	lsm_list_modules		sys_lsm_list_modules
 462	n32	mseal				sys_mseal
+463	n32	setxattrat			sys_setxattrat
+464	n32	getxattrat			sys_getxattrat
+465	n32	listxattrat			sys_listxattrat
+466	n32	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 1464c6be6eb3..c844cd5cda62 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -377,3 +377,7 @@
 460	n64	lsm_set_self_attr		sys_lsm_set_self_attr
 461	n64	lsm_list_modules		sys_lsm_list_modules
 462	n64	mseal				sys_mseal
+463	n64	setxattrat			sys_setxattrat
+464	n64	getxattrat			sys_getxattrat
+465	n64	listxattrat			sys_listxattrat
+466	n64	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 2439a2491cff..349b8aad1159 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -450,3 +450,7 @@
 460	o32	lsm_set_self_attr		sys_lsm_set_self_attr
 461	o32	lsm_list_modules		sys_lsm_list_modules
 462	o32	mseal				sys_mseal
+463	o32	setxattrat			sys_setxattrat
+464	o32	getxattrat			sys_getxattrat
+465	o32	listxattrat			sys_listxattrat
+466	o32	removexattrat			sys_removexattrat
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 66dc406b12e4..d9fc94c86965 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -461,3 +461,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index ebae8415dfbb..d8b4ab78bef0 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -553,3 +553,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 01071182763e..e9115b4d8b63 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -465,3 +465,7 @@
 460  common	lsm_set_self_attr	sys_lsm_set_self_attr		sys_lsm_set_self_attr
 461  common	lsm_list_modules	sys_lsm_list_modules		sys_lsm_list_modules
 462  common	mseal			sys_mseal			sys_mseal
+463  common	setxattrat		sys_setxattrat			sys_setxattrat
+464  common	getxattrat		sys_getxattrat			sys_getxattrat
+465  common	listxattrat		sys_listxattrat			sys_listxattrat
+466  common	removexattrat		sys_removexattrat		sys_removexattrat
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c55fd7696d40..c8cad33bf250 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -466,3 +466,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index cfdfb3707c16..727f99d333b3 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -508,3 +508,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal 				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 534c74b14fab..4d0fb2fba7e2 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -468,3 +468,7 @@
 460	i386	lsm_set_self_attr	sys_lsm_set_self_attr
 461	i386	lsm_list_modules	sys_lsm_list_modules
 462	i386	mseal 			sys_mseal
+463	i386	setxattrat		sys_setxattrat
+464	i386	getxattrat		sys_getxattrat
+465	i386	listxattrat		sys_listxattrat
+466	i386	removexattrat		sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7093ee21c0d1..5eb708bff1c7 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -386,6 +386,10 @@
 460	common	lsm_set_self_attr	sys_lsm_set_self_attr
 461	common	lsm_list_modules	sys_lsm_list_modules
 462 	common  mseal			sys_mseal
+463	common	setxattrat		sys_setxattrat
+464	common	getxattrat		sys_getxattrat
+465	common	listxattrat		sys_listxattrat
+466	common	removexattrat		sys_removexattrat
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 67083fc1b2f5..37effc1b134e 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -433,3 +433,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal 				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/fs/xattr.c b/fs/xattr.c
index 6f87f23c0e84..59cdb524412e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -597,6 +597,32 @@ int import_xattr_name(struct xattr_name *kname, const char __user *name)
 	return 0;
 }
 
+static struct filename *getname_xattr(const char __user *pathname,
+				      unsigned int at_flags)
+{
+	struct filename *name;
+	char c;
+
+	if (!(at_flags & AT_EMPTY_PATH))
+		return getname(pathname);
+
+	if (!pathname)
+		return NULL;
+
+	/* try to save on allocations; will suck on s390 and um, though */
+	if (get_user(c, pathname))
+		return ERR_PTR(-EFAULT);
+	if (!c)
+		return NULL;
+
+	name = getname_flags(pathname, LOOKUP_EMPTY);
+	if (!IS_ERR(name) && !(name->name[0])) {
+		putname(name);
+		name = NULL;
+	}
+	return name;
+}
+
 /*
  * Extended attribute SET operations
  */
@@ -675,69 +701,90 @@ int filename_setxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static int path_setxattr(const char __user *pathname,
-			 const char __user *name, const void __user *value,
-			 size_t size, int flags, unsigned int lookup_flags)
+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)
 {
 	struct xattr_name kname;
 	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
+		.cvalue	= value,
+		.kvalue	= NULL,
+		.size	= size,
+		.kname	= &kname,
+		.flags	= flags,
 	};
+	struct filename *filename;
+	unsigned int lookup_flags = 0;
 	int error;
 
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
+	if (at_flags & AT_SYMLINK_NOFOLLOW)
+		lookup_flags = LOOKUP_FOLLOW;
+
 	error = setxattr_copy(name, &ctx);
 	if (error)
 		return error;
 
-	error = filename_setxattr(AT_FDCWD, getname(pathname), lookup_flags,
-				  &ctx);
+	filename = getname_xattr(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			error = -EBADF;
+		else
+			error = file_setxattr(fd_file(f), &ctx);
+	} else {
+		error = filename_setxattr(dfd, filename, lookup_flags, &ctx);
+	}
 	kvfree(ctx.kvalue);
 	return error;
 }
 
+SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+		const char __user *, name, const struct xattr_args __user *, uargs,
+		size_t, usize)
+{
+	struct xattr_args args = {};
+	int error;
+
+	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
+	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
+
+	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
+		return -EINVAL;
+	if (usize > PAGE_SIZE)
+		return -E2BIG;
+
+	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+	if (error)
+		return error;
+
+	return path_setxattrat(dfd, pathname, at_flags, name,
+			       u64_to_user_ptr(args.value), args.size,
+			       args.flags);
+}
+
 SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
 		const char __user *, name, const void __user *, value,
 		size_t, size, int, flags)
 {
-	return path_setxattr(pathname, name, value, size, flags, LOOKUP_FOLLOW);
+	return path_setxattrat(AT_FDCWD, 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_setxattr(pathname, name, value, size, flags, 0);
+	return path_setxattrat(AT_FDCWD, 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)
 {
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
-	};
-	int error;
-
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-
-	error = setxattr_copy(name, &ctx);
-	if (error)
-		return error;
-
-	error = file_setxattr(fd_file(f), &ctx);
-	kvfree(ctx.kvalue);
-	return error;
+	return path_setxattrat(fd, NULL, AT_EMPTY_PATH, name,
+			       value, size, flags);
 }
 
 /*
@@ -802,11 +849,10 @@ ssize_t filename_getxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static ssize_t path_getxattr(const char __user *pathname,
-			     const char __user *name, void __user *value,
-			     size_t size, unsigned int lookup_flags)
+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)
 {
-	ssize_t error;
 	struct xattr_name kname;
 	struct kernel_xattr_ctx ctx = {
 		.value    = value,
@@ -814,44 +860,72 @@ static ssize_t path_getxattr(const char __user *pathname,
 		.kname    = &kname,
 		.flags    = 0,
 	};
+	struct filename *filename;
+	ssize_t error;
+
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
 
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-	return filename_getxattr(AT_FDCWD, getname(pathname), lookup_flags, &ctx);
+
+	filename = getname_xattr(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_getxattr(fd_file(f), &ctx);
+	} else {
+		int lookup_flags = 0;
+		if (at_flags & AT_SYMLINK_NOFOLLOW)
+			lookup_flags = LOOKUP_FOLLOW;
+		return filename_getxattr(dfd, filename, lookup_flags, &ctx);
+	}
+}
+
+SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+		const char __user *, name, struct xattr_args __user *, uargs, size_t, usize)
+{
+	struct xattr_args args = {};
+	int error;
+
+	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
+	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
+
+	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
+		return -EINVAL;
+	if (usize > PAGE_SIZE)
+		return -E2BIG;
+
+	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+	if (error)
+		return error;
+
+	if (args.flags != 0)
+		return -EINVAL;
+
+	return path_getxattrat(dfd, 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_getxattr(pathname, name, value, size, LOOKUP_FOLLOW);
+	return path_getxattrat(AT_FDCWD, pathname, 0, name, value, size);
 }
 
 SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 		const char __user *, name, void __user *, value, size_t, size)
 {
-	return path_getxattr(pathname, name, value, size, 0);
+	return path_getxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
+			       value, size);
 }
 
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	ssize_t error;
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.value    = value,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = 0,
-	};
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-	return file_getxattr(fd_file(f), &ctx);
+	return path_getxattrat(fd, NULL, AT_EMPTY_PATH, name, value, size);
 }
 
 /*
@@ -915,32 +989,50 @@ ssize_t filename_listxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static ssize_t path_listxattr(const char __user *pathname, char __user *list,
-			      size_t size, unsigned int lookup_flags)
+static ssize_t path_listxattrat(int dfd, const char __user *pathname,
+				unsigned int at_flags, char __user *list,
+				size_t size)
+{
+	struct filename *filename;
+	int lookup_flags;
+
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
+	filename = getname_xattr(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_listxattr(fd_file(f), list, size);
+	}
+
+	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	return filename_listxattr(dfd, filename, lookup_flags, list, size);
+}
+
+SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
+		unsigned int, at_flags,
+		char __user *, list, size_t, size)
 {
-	return filename_listxattr(AT_FDCWD, getname(pathname), lookup_flags,
-				  list, size);
+	return path_listxattrat(dfd, pathname, at_flags, list, size);
 }
 
 SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
-	return path_listxattr(pathname, list, size, LOOKUP_FOLLOW);
+	return path_listxattrat(AT_FDCWD, pathname, 0, list, size);
 }
 
 SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
-	return path_listxattr(pathname, list, size, 0);
+	return path_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
 }
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-	return file_listxattr(fd_file(f), list, size);
+	return path_listxattrat(fd, NULL, AT_EMPTY_PATH, list, size);
 }
 
 /*
@@ -992,44 +1084,53 @@ static int filename_removexattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static int path_removexattr(const char __user *pathname,
-			    const char __user *name, unsigned int lookup_flags)
+static int path_removexattrat(int dfd, const char __user *pathname,
+			      unsigned int at_flags, const char __user *name)
 {
 	struct xattr_name kname;
+	struct filename *filename;
+	unsigned int lookup_flags;
 	int error;
 
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-	return filename_removexattr(AT_FDCWD, getname(pathname), lookup_flags,
-				    &kname);
+
+	filename = getname_xattr(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_removexattr(fd_file(f), &kname);
+	}
+	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	return filename_removexattr(dfd, filename, lookup_flags, &kname);
+}
+
+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);
 }
 
 SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
 		const char __user *, name)
 {
-	return path_removexattr(pathname, name, LOOKUP_FOLLOW);
+	return path_removexattrat(AT_FDCWD, pathname, 0, name);
 }
 
 SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 		const char __user *, name)
 {
-	return path_removexattr(pathname, name, 0);
+	return path_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
 }
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	CLASS(fd, f)(fd);
-	struct xattr_name kname;
-	int error;
-
-	if (fd_empty(f))
-		return -EBADF;
-
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-	return file_removexattr(fd_file(f), &kname);
+	return path_removexattrat(fd, NULL, AT_EMPTY_PATH, name);
 }
 
 int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
diff --git a/include/asm-generic/audit_change_attr.h b/include/asm-generic/audit_change_attr.h
index 331670807cf0..cc840537885f 100644
--- a/include/asm-generic/audit_change_attr.h
+++ b/include/asm-generic/audit_change_attr.h
@@ -11,9 +11,15 @@ __NR_lchown,
 __NR_fchown,
 #endif
 __NR_setxattr,
+#ifdef __NR_setxattrat
+__NR_setxattrat,
+#endif
 __NR_lsetxattr,
 __NR_fsetxattr,
 __NR_removexattr,
+#ifdef __NR_removexattrat
+__NR_removexattrat,
+#endif
 __NR_lremovexattr,
 __NR_fremovexattr,
 #ifdef __NR_fchownat
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 5758104921e6..c6333204d451 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -77,6 +77,7 @@ struct cachestat_range;
 struct cachestat;
 struct statmount;
 struct mnt_id_req;
+struct xattr_args;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -338,23 +339,35 @@ asmlinkage long sys_io_uring_register(unsigned int fd, unsigned int op,
 				void __user *arg, unsigned int nr_args);
 asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
 			     const void __user *value, size_t size, int flags);
+asmlinkage long sys_setxattrat(int dfd, const char __user *path, unsigned int at_flags,
+			       const char __user *name,
+			       const struct xattr_args __user *args, size_t size);
 asmlinkage long sys_lsetxattr(const char __user *path, const char __user *name,
 			      const void __user *value, size_t size, int flags);
 asmlinkage long sys_fsetxattr(int fd, const char __user *name,
 			      const void __user *value, size_t size, int flags);
 asmlinkage long sys_getxattr(const char __user *path, const char __user *name,
 			     void __user *value, size_t size);
+asmlinkage long sys_getxattrat(int dfd, const char __user *path, unsigned int at_flags,
+			       const char __user *name,
+			       struct xattr_args __user *args, size_t size);
 asmlinkage long sys_lgetxattr(const char __user *path, const char __user *name,
 			      void __user *value, size_t size);
 asmlinkage long sys_fgetxattr(int fd, const char __user *name,
 			      void __user *value, size_t size);
 asmlinkage long sys_listxattr(const char __user *path, char __user *list,
 			      size_t size);
+asmlinkage long sys_listxattrat(int dfd, const char __user *path,
+				unsigned int at_flags,
+				char __user *list, size_t size);
 asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
 			       size_t size);
 asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
 asmlinkage long sys_removexattr(const char __user *path,
 				const char __user *name);
+asmlinkage long sys_removexattrat(int dfd, const char __user *path,
+				  unsigned int at_flags,
+				  const char __user *name);
 asmlinkage long sys_lremovexattr(const char __user *path,
 				 const char __user *name);
 asmlinkage long sys_fremovexattr(int fd, const char __user *name);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d20051865800..86b0d47984a1 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -19,6 +19,10 @@
 #include <linux/user_namespace.h>
 #include <uapi/linux/xattr.h>
 
+/* List of all open_how "versions". */
+#define XATTR_ARGS_SIZE_VER0	16 /* sizeof first published struct */
+#define XATTR_ARGS_SIZE_LATEST	XATTR_ARGS_SIZE_VER0
+
 struct inode;
 struct dentry;
 
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 5bf6148cac2b..88dc393c2bca 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -841,8 +841,17 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
 #define __NR_mseal 462
 __SYSCALL(__NR_mseal, sys_mseal)
 
+#define __NR_setxattrat 463
+__SYSCALL(__NR_setxattrat, sys_setxattrat)
+#define __NR_getxattrat 464
+__SYSCALL(__NR_getxattrat, sys_getxattrat)
+#define __NR_listxattrat 465
+__SYSCALL(__NR_listxattrat, sys_listxattrat)
+#define __NR_removexattrat 466
+__SYSCALL(__NR_removexattrat, sys_removexattrat)
+
 #undef __NR_syscalls
-#define __NR_syscalls 463
+#define __NR_syscalls 467
 
 /*
  * 32 bit systems traditionally used different
diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 9463db2dfa9d..9854f9cff3c6 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -11,6 +11,7 @@
 */
 
 #include <linux/libc-compat.h>
+#include <linux/types.h>
 
 #ifndef _UAPI_LINUX_XATTR_H
 #define _UAPI_LINUX_XATTR_H
@@ -20,6 +21,12 @@
 
 #define XATTR_CREATE	0x1	/* set value, fail if attr already exists */
 #define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
+
+struct xattr_args {
+	__aligned_u64 __user value;
+	__u32 size;
+	__u32 flags;
+};
 #endif
 
 /* Namespaces */
-- 
2.39.5

Re: [PATCH 9/9] fs/xattr: add *at family syscalls

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:30AM GMT, Al Viro wrote:
> From: Christian Göttsche <cgzones@googlemail.com>
> 
> Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
> removexattrat().  Those can be used to operate on extended attributes,
> especially security related ones, either relative to a pinned directory
> or on a file descriptor without read access, avoiding a
> /proc/<pid>/fd/<fd> detour, requiring a mounted procfs.
> 
> One use case will be setfiles(8) setting SELinux file contexts
> ("security.selinux") without race conditions and without a file
> descriptor opened with read access requiring SELinux read permission.
> 
> Use the do_{name}at() pattern from fs/open.c.
> 
> Pass the value of the extended attribute, its length, and for
> setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
> struct xattr_args to not exceed six syscall arguments and not
> merging the AT_* and XATTR_* flags.
> 
> [AV: fixes by Christian Brauner folded in, the entire thing rebased on
> top of {filename,file}_...xattr() primitives, treatment of empty
> pathnames regularized.  As the result, AT_EMPTY_PATH+NULL handling
> is cheap, so f...(2) can use it]
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> Link: https://lore.kernel.org/r/20240426162042.191916-1-cgoettsche@seltendoof.de
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> CC: x86@kernel.org
> CC: linux-alpha@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-ia64@vger.kernel.org
> CC: linux-m68k@lists.linux-m68k.org
> CC: linux-mips@vger.kernel.org
> CC: linux-parisc@vger.kernel.org
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-s390@vger.kernel.org
> CC: linux-sh@vger.kernel.org
> CC: sparclinux@vger.kernel.org
> CC: linux-fsdevel@vger.kernel.org
> CC: audit@vger.kernel.org
> CC: linux-arch@vger.kernel.org
> CC: linux-api@vger.kernel.org
> CC: linux-security-module@vger.kernel.org
> CC: selinux@vger.kernel.org
> [brauner: slight tweaks]
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

>  arch/alpha/kernel/syscalls/syscall.tbl      |   4 +
>  arch/arm/tools/syscall.tbl                  |   4 +
>  arch/m68k/kernel/syscalls/syscall.tbl       |   4 +
>  arch/microblaze/kernel/syscalls/syscall.tbl |   4 +
>  arch/mips/kernel/syscalls/syscall_n32.tbl   |   4 +
>  arch/mips/kernel/syscalls/syscall_n64.tbl   |   4 +
>  arch/mips/kernel/syscalls/syscall_o32.tbl   |   4 +
>  arch/parisc/kernel/syscalls/syscall.tbl     |   4 +
>  arch/powerpc/kernel/syscalls/syscall.tbl    |   4 +
>  arch/s390/kernel/syscalls/syscall.tbl       |   4 +
>  arch/sh/kernel/syscalls/syscall.tbl         |   4 +
>  arch/sparc/kernel/syscalls/syscall.tbl      |   4 +
>  arch/x86/entry/syscalls/syscall_32.tbl      |   4 +
>  arch/x86/entry/syscalls/syscall_64.tbl      |   4 +
>  arch/xtensa/kernel/syscalls/syscall.tbl     |   4 +
>  fs/xattr.c                                  | 271 ++++++++++++++------
>  include/asm-generic/audit_change_attr.h     |   6 +
>  include/linux/syscalls.h                    |  13 +
>  include/linux/xattr.h                       |   4 +
>  include/uapi/asm-generic/unistd.h           |  11 +-
>  include/uapi/linux/xattr.h                  |   7 +
>  21 files changed, 286 insertions(+), 86 deletions(-)
> 
> diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
> index 74720667fe09..c59d53d6d3f3 100644
> --- a/arch/alpha/kernel/syscalls/syscall.tbl
> +++ b/arch/alpha/kernel/syscalls/syscall.tbl
> @@ -502,3 +502,7 @@
>  570	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  571	common	lsm_list_modules		sys_lsm_list_modules
>  572	common  mseal				sys_mseal
> +573	common	setxattrat			sys_setxattrat
> +574	common	getxattrat			sys_getxattrat
> +575	common	listxattrat			sys_listxattrat
> +576	common	removexattrat			sys_removexattrat
> diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> index 23c98203c40f..49eeb2ad8dbd 100644
> --- a/arch/arm/tools/syscall.tbl
> +++ b/arch/arm/tools/syscall.tbl
> @@ -477,3 +477,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
> index 22a3cbd4c602..f5ed71f1910d 100644
> --- a/arch/m68k/kernel/syscalls/syscall.tbl
> +++ b/arch/m68k/kernel/syscalls/syscall.tbl
> @@ -462,3 +462,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
> index 2b81a6bd78b2..680f568b77f2 100644
> --- a/arch/microblaze/kernel/syscalls/syscall.tbl
> +++ b/arch/microblaze/kernel/syscalls/syscall.tbl
> @@ -468,3 +468,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
> index 953f5b7dc723..0b9b7e25b69a 100644
> --- a/arch/mips/kernel/syscalls/syscall_n32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
> @@ -401,3 +401,7 @@
>  460	n32	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	n32	lsm_list_modules		sys_lsm_list_modules
>  462	n32	mseal				sys_mseal
> +463	n32	setxattrat			sys_setxattrat
> +464	n32	getxattrat			sys_getxattrat
> +465	n32	listxattrat			sys_listxattrat
> +466	n32	removexattrat			sys_removexattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
> index 1464c6be6eb3..c844cd5cda62 100644
> --- a/arch/mips/kernel/syscalls/syscall_n64.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
> @@ -377,3 +377,7 @@
>  460	n64	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	n64	lsm_list_modules		sys_lsm_list_modules
>  462	n64	mseal				sys_mseal
> +463	n64	setxattrat			sys_setxattrat
> +464	n64	getxattrat			sys_getxattrat
> +465	n64	listxattrat			sys_listxattrat
> +466	n64	removexattrat			sys_removexattrat
> diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
> index 2439a2491cff..349b8aad1159 100644
> --- a/arch/mips/kernel/syscalls/syscall_o32.tbl
> +++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
> @@ -450,3 +450,7 @@
>  460	o32	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	o32	lsm_list_modules		sys_lsm_list_modules
>  462	o32	mseal				sys_mseal
> +463	o32	setxattrat			sys_setxattrat
> +464	o32	getxattrat			sys_getxattrat
> +465	o32	listxattrat			sys_listxattrat
> +466	o32	removexattrat			sys_removexattrat
> diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
> index 66dc406b12e4..d9fc94c86965 100644
> --- a/arch/parisc/kernel/syscalls/syscall.tbl
> +++ b/arch/parisc/kernel/syscalls/syscall.tbl
> @@ -461,3 +461,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
> index ebae8415dfbb..d8b4ab78bef0 100644
> --- a/arch/powerpc/kernel/syscalls/syscall.tbl
> +++ b/arch/powerpc/kernel/syscalls/syscall.tbl
> @@ -553,3 +553,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
> index 01071182763e..e9115b4d8b63 100644
> --- a/arch/s390/kernel/syscalls/syscall.tbl
> +++ b/arch/s390/kernel/syscalls/syscall.tbl
> @@ -465,3 +465,7 @@
>  460  common	lsm_set_self_attr	sys_lsm_set_self_attr		sys_lsm_set_self_attr
>  461  common	lsm_list_modules	sys_lsm_list_modules		sys_lsm_list_modules
>  462  common	mseal			sys_mseal			sys_mseal
> +463  common	setxattrat		sys_setxattrat			sys_setxattrat
> +464  common	getxattrat		sys_getxattrat			sys_getxattrat
> +465  common	listxattrat		sys_listxattrat			sys_listxattrat
> +466  common	removexattrat		sys_removexattrat		sys_removexattrat
> diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
> index c55fd7696d40..c8cad33bf250 100644
> --- a/arch/sh/kernel/syscalls/syscall.tbl
> +++ b/arch/sh/kernel/syscalls/syscall.tbl
> @@ -466,3 +466,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
> index cfdfb3707c16..727f99d333b3 100644
> --- a/arch/sparc/kernel/syscalls/syscall.tbl
> +++ b/arch/sparc/kernel/syscalls/syscall.tbl
> @@ -508,3 +508,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal 				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 534c74b14fab..4d0fb2fba7e2 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -468,3 +468,7 @@
>  460	i386	lsm_set_self_attr	sys_lsm_set_self_attr
>  461	i386	lsm_list_modules	sys_lsm_list_modules
>  462	i386	mseal 			sys_mseal
> +463	i386	setxattrat		sys_setxattrat
> +464	i386	getxattrat		sys_getxattrat
> +465	i386	listxattrat		sys_listxattrat
> +466	i386	removexattrat		sys_removexattrat
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index 7093ee21c0d1..5eb708bff1c7 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -386,6 +386,10 @@
>  460	common	lsm_set_self_attr	sys_lsm_set_self_attr
>  461	common	lsm_list_modules	sys_lsm_list_modules
>  462 	common  mseal			sys_mseal
> +463	common	setxattrat		sys_setxattrat
> +464	common	getxattrat		sys_getxattrat
> +465	common	listxattrat		sys_listxattrat
> +466	common	removexattrat		sys_removexattrat
>  
>  #
>  # Due to a historical design error, certain syscalls are numbered differently
> diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
> index 67083fc1b2f5..37effc1b134e 100644
> --- a/arch/xtensa/kernel/syscalls/syscall.tbl
> +++ b/arch/xtensa/kernel/syscalls/syscall.tbl
> @@ -433,3 +433,7 @@
>  460	common	lsm_set_self_attr		sys_lsm_set_self_attr
>  461	common	lsm_list_modules		sys_lsm_list_modules
>  462	common	mseal 				sys_mseal
> +463	common	setxattrat			sys_setxattrat
> +464	common	getxattrat			sys_getxattrat
> +465	common	listxattrat			sys_listxattrat
> +466	common	removexattrat			sys_removexattrat
> diff --git a/fs/xattr.c b/fs/xattr.c
> index 6f87f23c0e84..59cdb524412e 100644
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -597,6 +597,32 @@ int import_xattr_name(struct xattr_name *kname, const char __user *name)
>  	return 0;
>  }
>  
> +static struct filename *getname_xattr(const char __user *pathname,
> +				      unsigned int at_flags)
> +{
> +	struct filename *name;
> +	char c;
> +
> +	if (!(at_flags & AT_EMPTY_PATH))
> +		return getname(pathname);
> +
> +	if (!pathname)
> +		return NULL;
> +
> +	/* try to save on allocations; will suck on s390 and um, though */
> +	if (get_user(c, pathname))
> +		return ERR_PTR(-EFAULT);
> +	if (!c)
> +		return NULL;
> +
> +	name = getname_flags(pathname, LOOKUP_EMPTY);
> +	if (!IS_ERR(name) && !(name->name[0])) {
> +		putname(name);
> +		name = NULL;
> +	}
> +	return name;
> +}
> +
>  /*
>   * Extended attribute SET operations
>   */
> @@ -675,69 +701,90 @@ int filename_setxattr(int dfd, struct filename *filename,
>  	return error;
>  }
>  
> -static int path_setxattr(const char __user *pathname,
> -			 const char __user *name, const void __user *value,
> -			 size_t size, int flags, unsigned int lookup_flags)
> +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)
>  {
>  	struct xattr_name kname;
>  	struct kernel_xattr_ctx ctx = {
> -		.cvalue   = value,
> -		.kvalue   = NULL,
> -		.size     = size,
> -		.kname    = &kname,
> -		.flags    = flags,
> +		.cvalue	= value,
> +		.kvalue	= NULL,
> +		.size	= size,
> +		.kname	= &kname,
> +		.flags	= flags,
>  	};
> +	struct filename *filename;
> +	unsigned int lookup_flags = 0;
>  	int error;
>  
> +	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> +		return -EINVAL;
> +
> +	if (at_flags & AT_SYMLINK_NOFOLLOW)
> +		lookup_flags = LOOKUP_FOLLOW;
> +
>  	error = setxattr_copy(name, &ctx);
>  	if (error)
>  		return error;
>  
> -	error = filename_setxattr(AT_FDCWD, getname(pathname), lookup_flags,
> -				  &ctx);
> +	filename = getname_xattr(pathname, at_flags);
> +	if (!filename) {
> +		CLASS(fd, f)(dfd);
> +		if (fd_empty(f))
> +			error = -EBADF;
> +		else
> +			error = file_setxattr(fd_file(f), &ctx);
> +	} else {
> +		error = filename_setxattr(dfd, filename, lookup_flags, &ctx);
> +	}
>  	kvfree(ctx.kvalue);
>  	return error;
>  }
>  
> +SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
> +		const char __user *, name, const struct xattr_args __user *, uargs,
> +		size_t, usize)
> +{
> +	struct xattr_args args = {};
> +	int error;
> +
> +	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
> +	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
> +
> +	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
> +		return -EINVAL;
> +	if (usize > PAGE_SIZE)
> +		return -E2BIG;
> +
> +	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
> +	if (error)
> +		return error;
> +
> +	return path_setxattrat(dfd, pathname, at_flags, name,
> +			       u64_to_user_ptr(args.value), args.size,
> +			       args.flags);
> +}
> +
>  SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
>  		const char __user *, name, const void __user *, value,
>  		size_t, size, int, flags)
>  {
> -	return path_setxattr(pathname, name, value, size, flags, LOOKUP_FOLLOW);
> +	return path_setxattrat(AT_FDCWD, 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_setxattr(pathname, name, value, size, flags, 0);
> +	return path_setxattrat(AT_FDCWD, 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)
>  {
> -	struct xattr_name kname;
> -	struct kernel_xattr_ctx ctx = {
> -		.cvalue   = value,
> -		.kvalue   = NULL,
> -		.size     = size,
> -		.kname    = &kname,
> -		.flags    = flags,
> -	};
> -	int error;
> -
> -	CLASS(fd, f)(fd);
> -
> -	if (fd_empty(f))
> -		return -EBADF;
> -
> -	error = setxattr_copy(name, &ctx);
> -	if (error)
> -		return error;
> -
> -	error = file_setxattr(fd_file(f), &ctx);
> -	kvfree(ctx.kvalue);
> -	return error;
> +	return path_setxattrat(fd, NULL, AT_EMPTY_PATH, name,
> +			       value, size, flags);
>  }
>  
>  /*
> @@ -802,11 +849,10 @@ ssize_t filename_getxattr(int dfd, struct filename *filename,
>  	return error;
>  }
>  
> -static ssize_t path_getxattr(const char __user *pathname,
> -			     const char __user *name, void __user *value,
> -			     size_t size, unsigned int lookup_flags)
> +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)
>  {
> -	ssize_t error;
>  	struct xattr_name kname;
>  	struct kernel_xattr_ctx ctx = {
>  		.value    = value,
> @@ -814,44 +860,72 @@ static ssize_t path_getxattr(const char __user *pathname,
>  		.kname    = &kname,
>  		.flags    = 0,
>  	};
> +	struct filename *filename;
> +	ssize_t error;
> +
> +	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> +		return -EINVAL;
>  
>  	error = import_xattr_name(&kname, name);
>  	if (error)
>  		return error;
> -	return filename_getxattr(AT_FDCWD, getname(pathname), lookup_flags, &ctx);
> +
> +	filename = getname_xattr(pathname, at_flags);
> +	if (!filename) {
> +		CLASS(fd, f)(dfd);
> +		if (fd_empty(f))
> +			return -EBADF;
> +		return file_getxattr(fd_file(f), &ctx);
> +	} else {
> +		int lookup_flags = 0;
> +		if (at_flags & AT_SYMLINK_NOFOLLOW)
> +			lookup_flags = LOOKUP_FOLLOW;
> +		return filename_getxattr(dfd, filename, lookup_flags, &ctx);
> +	}
> +}
> +
> +SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
> +		const char __user *, name, struct xattr_args __user *, uargs, size_t, usize)
> +{
> +	struct xattr_args args = {};
> +	int error;
> +
> +	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
> +	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
> +
> +	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
> +		return -EINVAL;
> +	if (usize > PAGE_SIZE)
> +		return -E2BIG;
> +
> +	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
> +	if (error)
> +		return error;
> +
> +	if (args.flags != 0)
> +		return -EINVAL;
> +
> +	return path_getxattrat(dfd, 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_getxattr(pathname, name, value, size, LOOKUP_FOLLOW);
> +	return path_getxattrat(AT_FDCWD, pathname, 0, name, value, size);
>  }
>  
>  SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
>  		const char __user *, name, void __user *, value, size_t, size)
>  {
> -	return path_getxattr(pathname, name, value, size, 0);
> +	return path_getxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
> +			       value, size);
>  }
>  
>  SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
>  		void __user *, value, size_t, size)
>  {
> -	ssize_t error;
> -	struct xattr_name kname;
> -	struct kernel_xattr_ctx ctx = {
> -		.value    = value,
> -		.size     = size,
> -		.kname    = &kname,
> -		.flags    = 0,
> -	};
> -	CLASS(fd, f)(fd);
> -
> -	if (fd_empty(f))
> -		return -EBADF;
> -	error = import_xattr_name(&kname, name);
> -	if (error)
> -		return error;
> -	return file_getxattr(fd_file(f), &ctx);
> +	return path_getxattrat(fd, NULL, AT_EMPTY_PATH, name, value, size);
>  }
>  
>  /*
> @@ -915,32 +989,50 @@ ssize_t filename_listxattr(int dfd, struct filename *filename,
>  	return error;
>  }
>  
> -static ssize_t path_listxattr(const char __user *pathname, char __user *list,
> -			      size_t size, unsigned int lookup_flags)
> +static ssize_t path_listxattrat(int dfd, const char __user *pathname,
> +				unsigned int at_flags, char __user *list,
> +				size_t size)
> +{
> +	struct filename *filename;
> +	int lookup_flags;
> +
> +	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> +		return -EINVAL;
> +
> +	filename = getname_xattr(pathname, at_flags);
> +	if (!filename) {
> +		CLASS(fd, f)(dfd);
> +		if (fd_empty(f))
> +			return -EBADF;
> +		return file_listxattr(fd_file(f), list, size);
> +	}
> +
> +	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> +	return filename_listxattr(dfd, filename, lookup_flags, list, size);
> +}
> +
> +SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
> +		unsigned int, at_flags,
> +		char __user *, list, size_t, size)
>  {
> -	return filename_listxattr(AT_FDCWD, getname(pathname), lookup_flags,
> -				  list, size);
> +	return path_listxattrat(dfd, pathname, at_flags, list, size);
>  }
>  
>  SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
>  		size_t, size)
>  {
> -	return path_listxattr(pathname, list, size, LOOKUP_FOLLOW);
> +	return path_listxattrat(AT_FDCWD, pathname, 0, list, size);
>  }
>  
>  SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
>  		size_t, size)
>  {
> -	return path_listxattr(pathname, list, size, 0);
> +	return path_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
>  }
>  
>  SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
>  {
> -	CLASS(fd, f)(fd);
> -
> -	if (fd_empty(f))
> -		return -EBADF;
> -	return file_listxattr(fd_file(f), list, size);
> +	return path_listxattrat(fd, NULL, AT_EMPTY_PATH, list, size);
>  }
>  
>  /*
> @@ -992,44 +1084,53 @@ static int filename_removexattr(int dfd, struct filename *filename,
>  	return error;
>  }
>  
> -static int path_removexattr(const char __user *pathname,
> -			    const char __user *name, unsigned int lookup_flags)
> +static int path_removexattrat(int dfd, const char __user *pathname,
> +			      unsigned int at_flags, const char __user *name)
>  {
>  	struct xattr_name kname;
> +	struct filename *filename;
> +	unsigned int lookup_flags;
>  	int error;
>  
> +	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
> +		return -EINVAL;
> +
>  	error = import_xattr_name(&kname, name);
>  	if (error)
>  		return error;
> -	return filename_removexattr(AT_FDCWD, getname(pathname), lookup_flags,
> -				    &kname);
> +
> +	filename = getname_xattr(pathname, at_flags);
> +	if (!filename) {
> +		CLASS(fd, f)(dfd);
> +		if (fd_empty(f))
> +			return -EBADF;
> +		return file_removexattr(fd_file(f), &kname);
> +	}
> +	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
> +	return filename_removexattr(dfd, filename, lookup_flags, &kname);
> +}
> +
> +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);
>  }
>  
>  SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
>  		const char __user *, name)
>  {
> -	return path_removexattr(pathname, name, LOOKUP_FOLLOW);
> +	return path_removexattrat(AT_FDCWD, pathname, 0, name);
>  }
>  
>  SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
>  		const char __user *, name)
>  {
> -	return path_removexattr(pathname, name, 0);
> +	return path_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
>  }
>  
>  SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
>  {
> -	CLASS(fd, f)(fd);
> -	struct xattr_name kname;
> -	int error;
> -
> -	if (fd_empty(f))
> -		return -EBADF;
> -
> -	error = import_xattr_name(&kname, name);
> -	if (error)
> -		return error;
> -	return file_removexattr(fd_file(f), &kname);
> +	return path_removexattrat(fd, NULL, AT_EMPTY_PATH, name);
>  }
>  
>  int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
> diff --git a/include/asm-generic/audit_change_attr.h b/include/asm-generic/audit_change_attr.h
> index 331670807cf0..cc840537885f 100644
> --- a/include/asm-generic/audit_change_attr.h
> +++ b/include/asm-generic/audit_change_attr.h
> @@ -11,9 +11,15 @@ __NR_lchown,
>  __NR_fchown,
>  #endif
>  __NR_setxattr,
> +#ifdef __NR_setxattrat
> +__NR_setxattrat,
> +#endif
>  __NR_lsetxattr,
>  __NR_fsetxattr,
>  __NR_removexattr,
> +#ifdef __NR_removexattrat
> +__NR_removexattrat,
> +#endif
>  __NR_lremovexattr,
>  __NR_fremovexattr,
>  #ifdef __NR_fchownat
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 5758104921e6..c6333204d451 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -77,6 +77,7 @@ struct cachestat_range;
>  struct cachestat;
>  struct statmount;
>  struct mnt_id_req;
> +struct xattr_args;
>  
>  #include <linux/types.h>
>  #include <linux/aio_abi.h>
> @@ -338,23 +339,35 @@ asmlinkage long sys_io_uring_register(unsigned int fd, unsigned int op,
>  				void __user *arg, unsigned int nr_args);
>  asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
>  			     const void __user *value, size_t size, int flags);
> +asmlinkage long sys_setxattrat(int dfd, const char __user *path, unsigned int at_flags,
> +			       const char __user *name,
> +			       const struct xattr_args __user *args, size_t size);
>  asmlinkage long sys_lsetxattr(const char __user *path, const char __user *name,
>  			      const void __user *value, size_t size, int flags);
>  asmlinkage long sys_fsetxattr(int fd, const char __user *name,
>  			      const void __user *value, size_t size, int flags);
>  asmlinkage long sys_getxattr(const char __user *path, const char __user *name,
>  			     void __user *value, size_t size);
> +asmlinkage long sys_getxattrat(int dfd, const char __user *path, unsigned int at_flags,
> +			       const char __user *name,
> +			       struct xattr_args __user *args, size_t size);
>  asmlinkage long sys_lgetxattr(const char __user *path, const char __user *name,
>  			      void __user *value, size_t size);
>  asmlinkage long sys_fgetxattr(int fd, const char __user *name,
>  			      void __user *value, size_t size);
>  asmlinkage long sys_listxattr(const char __user *path, char __user *list,
>  			      size_t size);
> +asmlinkage long sys_listxattrat(int dfd, const char __user *path,
> +				unsigned int at_flags,
> +				char __user *list, size_t size);
>  asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
>  			       size_t size);
>  asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
>  asmlinkage long sys_removexattr(const char __user *path,
>  				const char __user *name);
> +asmlinkage long sys_removexattrat(int dfd, const char __user *path,
> +				  unsigned int at_flags,
> +				  const char __user *name);
>  asmlinkage long sys_lremovexattr(const char __user *path,
>  				 const char __user *name);
>  asmlinkage long sys_fremovexattr(int fd, const char __user *name);
> diff --git a/include/linux/xattr.h b/include/linux/xattr.h
> index d20051865800..86b0d47984a1 100644
> --- a/include/linux/xattr.h
> +++ b/include/linux/xattr.h
> @@ -19,6 +19,10 @@
>  #include <linux/user_namespace.h>
>  #include <uapi/linux/xattr.h>
>  
> +/* List of all open_how "versions". */
> +#define XATTR_ARGS_SIZE_VER0	16 /* sizeof first published struct */
> +#define XATTR_ARGS_SIZE_LATEST	XATTR_ARGS_SIZE_VER0
> +
>  struct inode;
>  struct dentry;
>  
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 5bf6148cac2b..88dc393c2bca 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -841,8 +841,17 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
>  #define __NR_mseal 462
>  __SYSCALL(__NR_mseal, sys_mseal)
>  
> +#define __NR_setxattrat 463
> +__SYSCALL(__NR_setxattrat, sys_setxattrat)
> +#define __NR_getxattrat 464
> +__SYSCALL(__NR_getxattrat, sys_getxattrat)
> +#define __NR_listxattrat 465
> +__SYSCALL(__NR_listxattrat, sys_listxattrat)
> +#define __NR_removexattrat 466
> +__SYSCALL(__NR_removexattrat, sys_removexattrat)
> +
>  #undef __NR_syscalls
> -#define __NR_syscalls 463
> +#define __NR_syscalls 467
>  
>  /*
>   * 32 bit systems traditionally used different
> diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
> index 9463db2dfa9d..9854f9cff3c6 100644
> --- a/include/uapi/linux/xattr.h
> +++ b/include/uapi/linux/xattr.h
> @@ -11,6 +11,7 @@
>  */
>  
>  #include <linux/libc-compat.h>
> +#include <linux/types.h>
>  
>  #ifndef _UAPI_LINUX_XATTR_H
>  #define _UAPI_LINUX_XATTR_H
> @@ -20,6 +21,12 @@
>  
>  #define XATTR_CREATE	0x1	/* set value, fail if attr already exists */
>  #define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
> +
> +struct xattr_args {
> +	__aligned_u64 __user value;
> +	__u32 size;
> +	__u32 flags;
> +};
>  #endif
>  
>  /* Namespaces */
> -- 
> 2.39.5
> 

Re: [PATCH 1/9] xattr: switch to CLASS(fd)

[Christian Brauner]

On Wed, Oct 02, 2024 at 02:22:22AM GMT, Al Viro wrote:
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---

Reviewed-by: Christian Brauner <brauner@kernel.org>

[RFC][PATCHES v2] xattr stuff and interactions with io_uring

[Al Viro]

	Changes since v1:
* moved on top of (and makes use of) getname_maybe_null() stuff
(first two commits here, form #base.getname-fixed)
* fixed a leak on io_uring side spotted by Jens
* putname(ERR_PTR(-E...)) is a no-op; allows to simplify things on
io_uring side.
* applied reviewed-by
* picked a generic_listxattr() cleanup from Colin Ian King

	Help with review and testing would be welcome; if nobody objects,
to #for-next it goes...

The branch lives in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #work.xattr2
Individual patches in followups.
 
Diffstat:
 arch/alpha/kernel/syscalls/syscall.tbl      |   4 +
 arch/arm/tools/syscall.tbl                  |   4 +
 arch/m68k/kernel/syscalls/syscall.tbl       |   4 +
 arch/microblaze/kernel/syscalls/syscall.tbl |   4 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   |   4 +
 arch/parisc/kernel/syscalls/syscall.tbl     |   4 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |   4 +
 arch/s390/kernel/syscalls/syscall.tbl       |   4 +
 arch/sh/kernel/syscalls/syscall.tbl         |   4 +
 arch/sparc/kernel/syscalls/syscall.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_32.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_64.tbl      |   4 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |   4 +
 fs/internal.h                               |  17 +-
 fs/namei.c                                  |  34 ++-
 fs/stat.c                                   |  28 +-
 fs/xattr.c                                  | 446 ++++++++++++++++++----------
 include/asm-generic/audit_change_attr.h     |   6 +
 include/linux/fs.h                          |  10 +
 include/linux/syscalls.h                    |  13 +
 include/linux/xattr.h                       |   4 +
 include/uapi/asm-generic/unistd.h           |  11 +-
 include/uapi/linux/xattr.h                  |   7 +
 io_uring/xattr.c                            |  97 ++----
 26 files changed, 466 insertions(+), 267 deletions(-)

Shortog:

Al Viro (9):
      teach filename_lookup() to treat NULL filename as ""
      getname_maybe_null() - the third variant of pathname copy-in
      io_[gs]etxattr_prep(): just use getname()
      xattr: switch to CLASS(fd)
      new helper: import_xattr_name()
      replace do_setxattr() with saner helpers.
      replace do_getxattr() with saner helpers.
      new helpers: file_listxattr(), filename_listxattr()
      new helpers: file_removexattr(), filename_removexattr()

Christian Göttsche (2):
      fs: rename struct xattr_ctx to kernel_xattr_ctx
      fs/xattr: add *at family syscalls

Colin Ian King (1):
      xattr: remove redundant check on variable err

Jens Axboe (1):
      io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE

Patch summaries:

getname_maybe_null() introduction (#getname.base-fixed):
01/13) teach filename_lookup() to treat NULL filename as ""
02/13) getname_maybe_null() - the third variant of pathname copy-in

io_uring-side prep:
03/13) io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE
04/13) io_[gs]etxattr_prep(): just use getname()
	getname_flags(_, LOOKUP_FOLLOW) is ridiculous.

05/13)	switch to CLASS(fd) use.  Obvious.

06/13)	rename struct xattr_ctx to kernel_xattr_ctx
prep from the ...xattrat() series, to reduce the PITA for rebase

07/13)	new helper: import_xattr_name()
exact same logics for copying the xattr name in, dealing with
overflows, etc. in a lot of places.  

08/13)	replace do_setxattr() with saner helpers
file_setxattr(file, ctx) and filename_setxattr(dfd, filename, lookup_flags, ctx).
Don't mess with do_setxattr() directly - use those.  In particular, don't
bother with the ESTALE loop in io_uring - it doesn't belong there.
 
09/13)	replace do_getxattr() with saner helpers
Same on getxattr() side.

10/13)	new helpers: file_listxattr(), filename_listxattr()
Same, except that io_uring doesn't use those, so the helpers are left static.

11/13)	new helpers: file_removexattr(), filename_removexattr()
Ditto

12/13)	fs/xattr: add *at family syscalls
Rebased patch introducing those, with a bunch of fixes folded in so we don't
create bisect hazard there.  Potentially interesting bit is the pathname-handling
logics - getname_maybe_null(pathname, flags) returns a struct filename reference
if no AT_EMPTY_PATH had been given or if the pathname was non-NULL and turned out
to be non-empty.  With (NULL,AT_EMPTY_PATH) or (empty string, AT_EMPTY_PATH) it
returns NULL (and it tries to skip the allocation using the same trick with
get_user() that vfs_empty_path() uses).  That helper simplifies a lot of things,
and it should be cheap enough to convert fsetxattr(2) et.al. to the unified
path_setxattrat() and its ilk.  IF we get a slowdown on those, we can always
expand and simplify the calls in fsetxattr(2) and friends.

13/13) xattr: remove redundant check on variable err

[PATCH v2 01/13] teach filename_lookup() to treat NULL filename as ""

[Al Viro]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 4a4a22a08ac2..aaf3cd6c802f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -588,6 +588,7 @@ struct nameidata {
 		unsigned seq;
 	} *stack, internal[EMBEDDED_LEVELS];
 	struct filename	*name;
+	const char *pathname;
 	struct nameidata *saved;
 	unsigned	root_seq;
 	int		dfd;
@@ -606,6 +607,7 @@ static void __set_nameidata(struct nameidata *p, int dfd, struct filename *name)
 	p->depth = 0;
 	p->dfd = dfd;
 	p->name = name;
+	p->pathname = likely(name) ? name->name : "";
 	p->path.mnt = NULL;
 	p->path.dentry = NULL;
 	p->total_link_count = old ? old->total_link_count : 0;
@@ -2439,7 +2441,7 @@ static int link_path_walk(const char *name, struct nameidata *nd)
 static const char *path_init(struct nameidata *nd, unsigned flags)
 {
 	int error;
-	const char *s = nd->name->name;
+	const char *s = nd->pathname;
 
 	/* LOOKUP_CACHED requires RCU, ask caller to retry */
 	if ((flags & (LOOKUP_RCU | LOOKUP_CACHED)) == LOOKUP_CACHED)
-- 
2.39.5

[PATCH v2 02/13] getname_maybe_null() - the third variant of pathname copy-in

[Al Viro]

Semantics used by statx(2) (and later *xattrat(2)): without AT_EMPTY_PATH
it's standard getname() (i.e. ERR_PTR(-ENOENT) on empty string,
ERR_PTR(-EFAULT) on NULL), with AT_EMPTY_PATH both empty string and
NULL are accepted.

Calling conventions: getname_maybe_null(user_pointer, flags) returns
	* pointer to struct filename when non-empty string had been
successfully read
	* ERR_PTR(...) on error
	* NULL if an empty string or NULL pointer had been given
with AT_EMPTY_PATH in the flags argument.

It tries to avoid allocation in the last case; it's not always
able to do so, in which case the temporary struct filename instance
is freed and NULL returned anyway.

Fast path is inlined.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c         | 30 +++++++++++++++++++++++-------
 fs/stat.c          | 28 ++++------------------------
 include/linux/fs.h | 10 ++++++++++
 3 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index aaf3cd6c802f..2bfe476c3bd0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -211,22 +211,38 @@ getname_flags(const char __user *filename, int flags)
 	return result;
 }
 
-struct filename *
-getname_uflags(const char __user *filename, int uflags)
+struct filename *getname_uflags(const char __user *filename, int uflags)
 {
 	int flags = (uflags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
 
 	return getname_flags(filename, flags);
 }
 
-struct filename *
-getname(const char __user * filename)
+struct filename *getname(const char __user * filename)
 {
 	return getname_flags(filename, 0);
 }
 
-struct filename *
-getname_kernel(const char * filename)
+struct filename *__getname_maybe_null(const char __user *pathname)
+{
+	struct filename *name;
+	char c;
+
+	/* try to save on allocations; loss on um, though */
+	if (get_user(c, pathname))
+		return ERR_PTR(-EFAULT);
+	if (!c)
+		return NULL;
+
+	name = getname_flags(pathname, LOOKUP_EMPTY);
+	if (!IS_ERR(name) && !(name->name[0])) {
+		putname(name);
+		name = NULL;
+	}
+	return name;
+}
+
+struct filename *getname_kernel(const char * filename)
 {
 	struct filename *result;
 	int len = strlen(filename) + 1;
@@ -264,7 +280,7 @@ EXPORT_SYMBOL(getname_kernel);
 
 void putname(struct filename *name)
 {
-	if (IS_ERR(name))
+	if (IS_ERR_OR_NULL(name))
 		return;
 
 	if (WARN_ON_ONCE(!atomic_read(&name->refcnt)))
diff --git a/fs/stat.c b/fs/stat.c
index 41e598376d7e..b74831dc7ae6 100644
--- a/fs/stat.c
+++ b/fs/stat.c
@@ -326,18 +326,11 @@ int vfs_fstatat(int dfd, const char __user *filename,
 {
 	int ret;
 	int statx_flags = flags | AT_NO_AUTOMOUNT;
-	struct filename *name;
+	struct filename *name = getname_maybe_null(filename, flags);
 
-	/*
-	 * Work around glibc turning fstat() into fstatat(AT_EMPTY_PATH)
-	 *
-	 * If AT_EMPTY_PATH is set, we expect the common case to be that
-	 * empty path, and avoid doing all the extra pathname work.
-	 */
-	if (flags == AT_EMPTY_PATH && vfs_empty_path(dfd, filename))
+	if (!name && dfd >= 0)
 		return vfs_fstat(dfd, stat);
 
-	name = getname_flags(filename, getname_statx_lookup_flags(statx_flags));
 	ret = vfs_statx(dfd, name, statx_flags, stat, STATX_BASIC_STATS);
 	putname(name);
 
@@ -774,24 +767,11 @@ SYSCALL_DEFINE5(statx,
 		struct statx __user *, buffer)
 {
 	int ret;
-	unsigned lflags;
-	struct filename *name;
+	struct filename *name = getname_maybe_null(filename, flags);
 
-	/*
-	 * Short-circuit handling of NULL and "" paths.
-	 *
-	 * For a NULL path we require and accept only the AT_EMPTY_PATH flag
-	 * (possibly |'d with AT_STATX flags).
-	 *
-	 * However, glibc on 32-bit architectures implements fstatat as statx
-	 * with the "" pathname and AT_NO_AUTOMOUNT | AT_EMPTY_PATH flags.
-	 * Supporting this results in the uglification below.
-	 */
-	lflags = flags & ~(AT_NO_AUTOMOUNT | AT_STATX_SYNC_TYPE);
-	if (lflags == AT_EMPTY_PATH && vfs_empty_path(dfd, filename))
+	if (!name && dfd >= 0)
 		return do_statx_fd(dfd, flags & ~AT_NO_AUTOMOUNT, mask, buffer);
 
-	name = getname_flags(filename, getname_statx_lookup_flags(flags));
 	ret = do_statx(dfd, name, flags, mask, buffer);
 	putname(name);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e3c603d01337..403258ac2ea2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2766,6 +2766,16 @@ extern struct filename *getname_flags(const char __user *, int);
 extern struct filename *getname_uflags(const char __user *, int);
 extern struct filename *getname(const char __user *);
 extern struct filename *getname_kernel(const char *);
+extern struct filename *__getname_maybe_null(const char __user *);
+static inline struct filename *getname_maybe_null(const char __user *name, int flags)
+{
+	if (!(flags & AT_EMPTY_PATH))
+		return getname(name);
+
+	if (!name)
+		return NULL;
+	return __getname_maybe_null(name);
+}
 extern void putname(struct filename *name);
 
 extern int finish_open(struct file *file, struct dentry *dentry,
-- 
2.39.5

[PATCH v2 03/13] io_uring: IORING_OP_F[GS]ETXATTR is fine with REQ_F_FIXED_FILE

[Al Viro]

From: Jens Axboe <axboe@kernel.dk>

Rejection of IOSQE_FIXED_FILE combined with IORING_OP_[GS]ETXATTR
is fine - these do not take a file descriptor, so such combination
makes no sense.  The checks are misplaced, though - as it is, they
triggers on IORING_OP_F[GS]ETXATTR as well, and those do take
a file reference, no matter the origin.

Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 io_uring/xattr.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 6cf41c3bc369..4b68c282c91a 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -48,9 +48,6 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	const char __user *name;
 	int ret;
 
-	if (unlikely(req->flags & REQ_F_FIXED_FILE))
-		return -EBADF;
-
 	ix->filename = NULL;
 	ix->ctx.kvalue = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
@@ -90,6 +87,9 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	const char __user *path;
 	int ret;
 
+	if (unlikely(req->flags & REQ_F_FIXED_FILE))
+		return -EBADF;
+
 	ret = __io_getxattr_prep(req, sqe);
 	if (ret)
 		return ret;
@@ -152,9 +152,6 @@ static int __io_setxattr_prep(struct io_kiocb *req,
 	const char __user *name;
 	int ret;
 
-	if (unlikely(req->flags & REQ_F_FIXED_FILE))
-		return -EBADF;
-
 	ix->filename = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	ix->ctx.cvalue = u64_to_user_ptr(READ_ONCE(sqe->addr2));
@@ -183,6 +180,9 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	const char __user *path;
 	int ret;
 
+	if (unlikely(req->flags & REQ_F_FIXED_FILE))
+		return -EBADF;
+
 	ret = __io_setxattr_prep(req, sqe);
 	if (ret)
 		return ret;
-- 
2.39.5

[PATCH v2 04/13] io_[gs]etxattr_prep(): just use getname()

[Al Viro]

getname_flags(pathname, LOOKUP_FOLLOW) is obviously bogus - following
trailing symlinks has no impact on how to copy the pathname from userland...

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 io_uring/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 4b68c282c91a..967c5d8da061 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -96,7 +96,7 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
-	ix->filename = getname_flags(path, LOOKUP_FOLLOW);
+	ix->filename = getname(path);
 	if (IS_ERR(ix->filename)) {
 		ret = PTR_ERR(ix->filename);
 		ix->filename = NULL;
@@ -189,7 +189,7 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
-	ix->filename = getname_flags(path, LOOKUP_FOLLOW);
+	ix->filename = getname(path);
 	if (IS_ERR(ix->filename)) {
 		ret = PTR_ERR(ix->filename);
 		ix->filename = NULL;
-- 
2.39.5

Re: [PATCH v2 04/13] io_[gs]etxattr_prep(): just use getname()

[Jens Axboe]

On 11/2/24 1:31 AM, Al Viro wrote:
> getname_flags(pathname, LOOKUP_FOLLOW) is obviously bogus - following
> trailing symlinks has no impact on how to copy the pathname from userland...

Reviewed-by: Jens Axboe <axboe@kernel.dk>

-- 
Jens Axboe

[PATCH v2 05/13] xattr: switch to CLASS(fd)

[Al Viro]

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 35 ++++++++++++++---------------------
 1 file changed, 14 insertions(+), 21 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 05ec7e7d9e87..0fc813cb005c 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -697,9 +697,9 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 	int error;
 
 	CLASS(fd, f)(fd);
-	if (!fd_file(f))
-		return -EBADF;
 
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
 	error = setxattr_copy(name, &ctx);
 	if (error)
@@ -809,16 +809,13 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
-	error = getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
+	return getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
 			 name, value, size);
-	fdput(f);
-	return error;
 }
 
 /*
@@ -885,15 +882,12 @@ SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	struct fd f = fdget(fd);
-	ssize_t error = -EBADF;
+	CLASS(fd, f)(fd);
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
-	error = listxattr(fd_file(f)->f_path.dentry, list, size);
-	fdput(f);
-	return error;
+	return listxattr(fd_file(f)->f_path.dentry, list, size);
 }
 
 /*
@@ -950,12 +944,12 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	struct fd f = fdget(fd);
+	CLASS(fd, f)(fd);
 	char kname[XATTR_NAME_MAX + 1];
-	int error = -EBADF;
+	int error;
 
-	if (!fd_file(f))
-		return error;
+	if (fd_empty(f))
+		return -EBADF;
 	audit_file(fd_file(f));
 
 	error = strncpy_from_user(kname, name, sizeof(kname));
@@ -970,7 +964,6 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 				    fd_file(f)->f_path.dentry, kname);
 		mnt_drop_write_file(fd_file(f));
 	}
-	fdput(f);
 	return error;
 }
 
-- 
2.39.5

[PATCH v2 06/13] fs: rename struct xattr_ctx to kernel_xattr_ctx

[Al Viro]

From: Christian Göttsche <cgzones@googlemail.com>

Rename the struct xattr_ctx to increase distinction with the about to be
added user API struct xattr_args.

No functional change.

Suggested-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Link: https://lore.kernel.org/r/20240426162042.191916-2-cgoettsche@seltendoof.de
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/internal.h    |  8 ++++----
 fs/xattr.c       | 12 ++++++------
 io_uring/xattr.c |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 8c1b7acbbe8f..81c7a085355c 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -267,7 +267,7 @@ struct xattr_name {
 	char name[XATTR_NAME_MAX + 1];
 };
 
-struct xattr_ctx {
+struct kernel_xattr_ctx {
 	/* Value of attribute */
 	union {
 		const void __user *cvalue;
@@ -283,11 +283,11 @@ struct xattr_ctx {
 
 ssize_t do_getxattr(struct mnt_idmap *idmap,
 		    struct dentry *d,
-		    struct xattr_ctx *ctx);
+		    struct kernel_xattr_ctx *ctx);
 
-int setxattr_copy(const char __user *name, struct xattr_ctx *ctx);
+int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct xattr_ctx *ctx);
+		struct kernel_xattr_ctx *ctx);
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
 
 #ifdef CONFIG_FS_POSIX_ACL
diff --git a/fs/xattr.c b/fs/xattr.c
index 0fc813cb005c..1214ae7e71db 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -590,7 +590,7 @@ EXPORT_SYMBOL_GPL(vfs_removexattr);
  * Extended attribute SET operations
  */
 
-int setxattr_copy(const char __user *name, struct xattr_ctx *ctx)
+int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 {
 	int error;
 
@@ -620,7 +620,7 @@ int setxattr_copy(const char __user *name, struct xattr_ctx *ctx)
 }
 
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct xattr_ctx *ctx)
+		struct kernel_xattr_ctx *ctx)
 {
 	if (is_posix_acl_xattr(ctx->kname->name))
 		return do_set_acl(idmap, dentry, ctx->kname->name,
@@ -635,7 +635,7 @@ static int path_setxattr(const char __user *pathname,
 			 size_t size, int flags, unsigned int lookup_flags)
 {
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.cvalue   = value,
 		.kvalue   = NULL,
 		.size     = size,
@@ -687,7 +687,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 		const void __user *,value, size_t, size, int, flags)
 {
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.cvalue   = value,
 		.kvalue   = NULL,
 		.size     = size,
@@ -720,7 +720,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
  */
 ssize_t
 do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
-	struct xattr_ctx *ctx)
+	struct kernel_xattr_ctx *ctx)
 {
 	ssize_t error;
 	char *kname = ctx->kname->name;
@@ -755,7 +755,7 @@ getxattr(struct mnt_idmap *idmap, struct dentry *d,
 {
 	ssize_t error;
 	struct xattr_name kname;
-	struct xattr_ctx ctx = {
+	struct kernel_xattr_ctx ctx = {
 		.value    = value,
 		.kvalue   = NULL,
 		.size     = size,
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 967c5d8da061..f440121c3984 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -18,7 +18,7 @@
 
 struct io_xattr {
 	struct file			*file;
-	struct xattr_ctx		ctx;
+	struct kernel_xattr_ctx		ctx;
 	struct filename			*filename;
 };
 
-- 
2.39.5

[PATCH v2 07/13] new helper: import_xattr_name()

[Al Viro]

common logics for marshalling xattr names.

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  3 +++
 fs/xattr.c       | 45 +++++++++++++++++++++++----------------------
 io_uring/xattr.c |  7 ++-----
 3 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 81c7a085355c..b9f5ac4d39fc 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -288,6 +288,9 @@ ssize_t do_getxattr(struct mnt_idmap *idmap,
 int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
 int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		struct kernel_xattr_ctx *ctx);
+
+int import_xattr_name(struct xattr_name *kname, const char __user *name);
+
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
 
 #ifdef CONFIG_FS_POSIX_ACL
diff --git a/fs/xattr.c b/fs/xattr.c
index 1214ae7e71db..d8f7c766f28a 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -586,6 +586,17 @@ vfs_removexattr(struct mnt_idmap *idmap, struct dentry *dentry,
 }
 EXPORT_SYMBOL_GPL(vfs_removexattr);
 
+int import_xattr_name(struct xattr_name *kname, const char __user *name)
+{
+	int error = strncpy_from_user(kname->name, name,
+					sizeof(kname->name));
+	if (error == 0 || error == sizeof(kname->name))
+		return -ERANGE;
+	if (error < 0)
+		return error;
+	return 0;
+}
+
 /*
  * Extended attribute SET operations
  */
@@ -597,14 +608,10 @@ int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 	if (ctx->flags & ~(XATTR_CREATE|XATTR_REPLACE))
 		return -EINVAL;
 
-	error = strncpy_from_user(ctx->kname->name, name,
-				sizeof(ctx->kname->name));
-	if (error == 0 || error == sizeof(ctx->kname->name))
-		return  -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(ctx->kname, name);
+	if (error)
 		return error;
 
-	error = 0;
 	if (ctx->size) {
 		if (ctx->size > XATTR_SIZE_MAX)
 			return -E2BIG;
@@ -763,10 +770,8 @@ getxattr(struct mnt_idmap *idmap, struct dentry *d,
 		.flags    = 0,
 	};
 
-	error = strncpy_from_user(kname.name, name, sizeof(kname.name));
-	if (error == 0 || error == sizeof(kname.name))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 
 	error =  do_getxattr(idmap, d, &ctx);
@@ -906,12 +911,10 @@ static int path_removexattr(const char __user *pathname,
 {
 	struct path path;
 	int error;
-	char kname[XATTR_NAME_MAX + 1];
+	struct xattr_name kname;
 
-	error = strncpy_from_user(kname, name, sizeof(kname));
-	if (error == 0 || error == sizeof(kname))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 retry:
 	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
@@ -919,7 +922,7 @@ static int path_removexattr(const char __user *pathname,
 		return error;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname);
+		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname.name);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -945,23 +948,21 @@ SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
 	CLASS(fd, f)(fd);
-	char kname[XATTR_NAME_MAX + 1];
+	struct xattr_name kname;
 	int error;
 
 	if (fd_empty(f))
 		return -EBADF;
 	audit_file(fd_file(f));
 
-	error = strncpy_from_user(kname, name, sizeof(kname));
-	if (error == 0 || error == sizeof(kname))
-		error = -ERANGE;
-	if (error < 0)
+	error = import_xattr_name(&kname, name);
+	if (error)
 		return error;
 
 	error = mnt_want_write_file(fd_file(f));
 	if (!error) {
 		error = removexattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, kname);
+				    fd_file(f)->f_path.dentry, kname.name);
 		mnt_drop_write_file(fd_file(f));
 	}
 	return error;
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index f440121c3984..0b3b871eaa65 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -62,11 +62,8 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	if (!ix->ctx.kname)
 		return -ENOMEM;
 
-	ret = strncpy_from_user(ix->ctx.kname->name, name,
-				sizeof(ix->ctx.kname->name));
-	if (!ret || ret == sizeof(ix->ctx.kname->name))
-		ret = -ERANGE;
-	if (ret < 0) {
+	ret = import_xattr_name(ix->ctx.kname, name);
+	if (ret) {
 		kfree(ix->ctx.kname);
 		return ret;
 	}
-- 
2.39.5

[PATCH v2 08/13] replace do_setxattr() with saner helpers.

[Al Viro]

io_uring setxattr logics duplicates stuff from fs/xattr.c; provide
saner helpers (filename_setxattr() and file_setxattr() resp.) and
use them.

NB: putname(ERR_PTR()) is a no-op

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  6 ++---
 fs/xattr.c       | 69 ++++++++++++++++++++++++++++++------------------
 io_uring/xattr.c | 41 +++++-----------------------
 3 files changed, 54 insertions(+), 62 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index b9f5ac4d39fc..be7c0da3bcec 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -285,10 +285,10 @@ ssize_t do_getxattr(struct mnt_idmap *idmap,
 		    struct dentry *d,
 		    struct kernel_xattr_ctx *ctx);
 
+int file_setxattr(struct file *file, struct kernel_xattr_ctx *ctx);
+int filename_setxattr(int dfd, struct filename *filename,
+		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
 int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx);
-int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
-		struct kernel_xattr_ctx *ctx);
-
 int import_xattr_name(struct xattr_name *kname, const char __user *name);
 
 int may_write_xattr(struct mnt_idmap *idmap, struct inode *inode);
diff --git a/fs/xattr.c b/fs/xattr.c
index d8f7c766f28a..38bf8cfbd464 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -626,7 +626,7 @@ int setxattr_copy(const char __user *name, struct kernel_xattr_ctx *ctx)
 	return error;
 }
 
-int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
+static int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		struct kernel_xattr_ctx *ctx)
 {
 	if (is_posix_acl_xattr(ctx->kname->name))
@@ -637,32 +637,32 @@ int do_setxattr(struct mnt_idmap *idmap, struct dentry *dentry,
 			ctx->kvalue, ctx->size, ctx->flags);
 }
 
-static int path_setxattr(const char __user *pathname,
-			 const char __user *name, const void __user *value,
-			 size_t size, int flags, unsigned int lookup_flags)
+int file_setxattr(struct file *f, struct kernel_xattr_ctx *ctx)
+{
+	int error = mnt_want_write_file(f);
+
+	if (!error) {
+		audit_file(f);
+		error = do_setxattr(file_mnt_idmap(f), f->f_path.dentry, ctx);
+		mnt_drop_write_file(f);
+	}
+	return error;
+}
+
+/* unconditionally consumes filename */
+int filename_setxattr(int dfd, struct filename *filename,
+		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx)
 {
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
-	};
 	struct path path;
 	int error;
 
-	error = setxattr_copy(name, &ctx);
-	if (error)
-		return error;
-
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
 		goto out;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = do_setxattr(mnt_idmap(path.mnt), path.dentry, &ctx);
+		error = do_setxattr(mnt_idmap(path.mnt), path.dentry, ctx);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -672,6 +672,30 @@ static int path_setxattr(const char __user *pathname,
 	}
 
 out:
+	putname(filename);
+	return error;
+}
+
+static int path_setxattr(const char __user *pathname,
+			 const char __user *name, const void __user *value,
+			 size_t size, int flags, unsigned int lookup_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;
+
+	error = filename_setxattr(AT_FDCWD, getname(pathname), lookup_flags,
+				  &ctx);
 	kvfree(ctx.kvalue);
 	return error;
 }
@@ -707,17 +731,12 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
+
 	error = setxattr_copy(name, &ctx);
 	if (error)
 		return error;
 
-	error = mnt_want_write_file(fd_file(f));
-	if (!error) {
-		error = do_setxattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, &ctx);
-		mnt_drop_write_file(fd_file(f));
-	}
+	error = file_setxattr(fd_file(f), &ctx);
 	kvfree(ctx.kvalue);
 	return error;
 }
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 0b3b871eaa65..2671ad05d63b 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -187,12 +187,10 @@ int io_setxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
 	ix->filename = getname(path);
-	if (IS_ERR(ix->filename)) {
-		ret = PTR_ERR(ix->filename);
-		ix->filename = NULL;
-	}
+	if (IS_ERR(ix->filename))
+		return PTR_ERR(ix->filename);
 
-	return ret;
+	return 0;
 }
 
 int io_fsetxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
@@ -200,28 +198,14 @@ int io_fsetxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	return __io_setxattr_prep(req, sqe);
 }
 
-static int __io_setxattr(struct io_kiocb *req, unsigned int issue_flags,
-			const struct path *path)
-{
-	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	int ret;
-
-	ret = mnt_want_write(path->mnt);
-	if (!ret) {
-		ret = do_setxattr(mnt_idmap(path->mnt), path->dentry, &ix->ctx);
-		mnt_drop_write(path->mnt);
-	}
-
-	return ret;
-}
-
 int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
+	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-	ret = __io_setxattr(req, issue_flags, &req->file->f_path);
+	ret = file_setxattr(req->file, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
@@ -229,23 +213,12 @@ int io_fsetxattr(struct io_kiocb *req, unsigned int issue_flags)
 int io_setxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
-	struct path path;
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-retry:
-	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
-	if (!ret) {
-		ret = __io_setxattr(req, issue_flags, &path);
-		path_put(&path);
-		if (retry_estale(ret, lookup_flags)) {
-			lookup_flags |= LOOKUP_REVAL;
-			goto retry;
-		}
-	}
-
+	ret = filename_setxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
+	ix->filename = NULL;
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
-- 
2.39.5

[PATCH v2 09/13] replace do_getxattr() with saner helpers.

[Al Viro]

similar to do_setxattr() in the previous commit...

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h    |  8 ++---
 fs/xattr.c       | 88 ++++++++++++++++++++++++++++--------------------
 io_uring/xattr.c | 31 ++++-------------
 3 files changed, 61 insertions(+), 66 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index be7c0da3bcec..8001efd1f047 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -280,11 +280,9 @@ struct kernel_xattr_ctx {
 	unsigned int flags;
 };
 
-
-ssize_t do_getxattr(struct mnt_idmap *idmap,
-		    struct dentry *d,
-		    struct kernel_xattr_ctx *ctx);
-
+ssize_t file_getxattr(struct file *file, struct kernel_xattr_ctx *ctx);
+ssize_t filename_getxattr(int dfd, struct filename *filename,
+			  unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
 int file_setxattr(struct file *file, struct kernel_xattr_ctx *ctx);
 int filename_setxattr(int dfd, struct filename *filename,
 		      unsigned int lookup_flags, struct kernel_xattr_ctx *ctx);
diff --git a/fs/xattr.c b/fs/xattr.c
index 38bf8cfbd464..d55f3d1e7589 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -744,27 +744,28 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 /*
  * Extended attribute GET operations
  */
-ssize_t
+static ssize_t
 do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
 	struct kernel_xattr_ctx *ctx)
 {
 	ssize_t error;
 	char *kname = ctx->kname->name;
+	void *kvalue = NULL;
 
 	if (ctx->size) {
 		if (ctx->size > XATTR_SIZE_MAX)
 			ctx->size = XATTR_SIZE_MAX;
-		ctx->kvalue = kvzalloc(ctx->size, GFP_KERNEL);
-		if (!ctx->kvalue)
+		kvalue = kvzalloc(ctx->size, GFP_KERNEL);
+		if (!kvalue)
 			return -ENOMEM;
 	}
 
-	if (is_posix_acl_xattr(ctx->kname->name))
-		error = do_get_acl(idmap, d, kname, ctx->kvalue, ctx->size);
+	if (is_posix_acl_xattr(kname))
+		error = do_get_acl(idmap, d, kname, kvalue, ctx->size);
 	else
-		error = vfs_getxattr(idmap, d, kname, ctx->kvalue, ctx->size);
+		error = vfs_getxattr(idmap, d, kname, kvalue, ctx->size);
 	if (error > 0) {
-		if (ctx->size && copy_to_user(ctx->value, ctx->kvalue, error))
+		if (ctx->size && copy_to_user(ctx->value, kvalue, error))
 			error = -EFAULT;
 	} else if (error == -ERANGE && ctx->size >= XATTR_SIZE_MAX) {
 		/* The file system tried to returned a value bigger
@@ -772,52 +773,56 @@ do_getxattr(struct mnt_idmap *idmap, struct dentry *d,
 		error = -E2BIG;
 	}
 
+	kvfree(kvalue);
 	return error;
 }
 
-static ssize_t
-getxattr(struct mnt_idmap *idmap, struct dentry *d,
-	 const char __user *name, void __user *value, size_t size)
+ssize_t file_getxattr(struct file *f, struct kernel_xattr_ctx *ctx)
 {
-	ssize_t error;
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.value    = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = 0,
-	};
-
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-
-	error =  do_getxattr(idmap, d, &ctx);
-
-	kvfree(ctx.kvalue);
-	return error;
+	audit_file(f);
+	return do_getxattr(file_mnt_idmap(f), f->f_path.dentry, ctx);
 }
 
-static ssize_t path_getxattr(const char __user *pathname,
-			     const char __user *name, void __user *value,
-			     size_t size, unsigned int lookup_flags)
+/* unconditionally consumes filename */
+ssize_t filename_getxattr(int dfd, struct filename *filename,
+			  unsigned int lookup_flags, struct kernel_xattr_ctx *ctx)
 {
 	struct path path;
 	ssize_t error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
-	error = getxattr(mnt_idmap(path.mnt), path.dentry, name, value, size);
+		goto out;
+	error = do_getxattr(mnt_idmap(path.mnt), path.dentry, ctx);
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static ssize_t path_getxattr(const char __user *pathname,
+			     const char __user *name, void __user *value,
+			     size_t size, unsigned int lookup_flags)
+{
+	ssize_t error;
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.value    = value,
+		.size     = size,
+		.kname    = &kname,
+		.flags    = 0,
+	};
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return filename_getxattr(AT_FDCWD, getname(pathname), lookup_flags, &ctx);
+}
+
 SYSCALL_DEFINE4(getxattr, const char __user *, pathname,
 		const char __user *, name, void __user *, value, size_t, size)
 {
@@ -833,13 +838,22 @@ SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
+	ssize_t error;
+	struct xattr_name kname;
+	struct kernel_xattr_ctx ctx = {
+		.value    = value,
+		.size     = size,
+		.kname    = &kname,
+		.flags    = 0,
+	};
 	CLASS(fd, f)(fd);
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
-	return getxattr(file_mnt_idmap(fd_file(f)), fd_file(f)->f_path.dentry,
-			 name, value, size);
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return file_getxattr(fd_file(f), &ctx);
 }
 
 /*
diff --git a/io_uring/xattr.c b/io_uring/xattr.c
index 2671ad05d63b..de5064fcae8a 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -51,7 +51,7 @@ static int __io_getxattr_prep(struct io_kiocb *req,
 	ix->filename = NULL;
 	ix->ctx.kvalue = NULL;
 	name = u64_to_user_ptr(READ_ONCE(sqe->addr));
-	ix->ctx.cvalue = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+	ix->ctx.value = u64_to_user_ptr(READ_ONCE(sqe->addr2));
 	ix->ctx.size = READ_ONCE(sqe->len);
 	ix->ctx.flags = READ_ONCE(sqe->xattr_flags);
 
@@ -94,12 +94,10 @@ int io_getxattr_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	path = u64_to_user_ptr(READ_ONCE(sqe->addr3));
 
 	ix->filename = getname(path);
-	if (IS_ERR(ix->filename)) {
-		ret = PTR_ERR(ix->filename);
-		ix->filename = NULL;
-	}
+	if (IS_ERR(ix->filename))
+		return PTR_ERR(ix->filename);
 
-	return ret;
+	return 0;
 }
 
 int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
@@ -109,10 +107,7 @@ int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-	ret = do_getxattr(file_mnt_idmap(req->file),
-			req->file->f_path.dentry,
-			&ix->ctx);
-
+	ret = file_getxattr(req->file, &ix->ctx);
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
@@ -120,24 +115,12 @@ int io_fgetxattr(struct io_kiocb *req, unsigned int issue_flags)
 int io_getxattr(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_xattr *ix = io_kiocb_to_cmd(req, struct io_xattr);
-	unsigned int lookup_flags = LOOKUP_FOLLOW;
-	struct path path;
 	int ret;
 
 	WARN_ON_ONCE(issue_flags & IO_URING_F_NONBLOCK);
 
-retry:
-	ret = filename_lookup(AT_FDCWD, ix->filename, lookup_flags, &path, NULL);
-	if (!ret) {
-		ret = do_getxattr(mnt_idmap(path.mnt), path.dentry, &ix->ctx);
-
-		path_put(&path);
-		if (retry_estale(ret, lookup_flags)) {
-			lookup_flags |= LOOKUP_REVAL;
-			goto retry;
-		}
-	}
-
+	ret = filename_getxattr(AT_FDCWD, ix->filename, LOOKUP_FOLLOW, &ix->ctx);
+	ix->filename = NULL;
 	io_xattr_finish(req, ret);
 	return IOU_OK;
 }
-- 
2.39.5

[PATCH v2 10/13] new helpers: file_listxattr(), filename_listxattr()

[Al Viro]

switch path_listxattr() and flistxattr(2) to those

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index d55f3d1e7589..988ea737ae7e 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -888,24 +888,43 @@ listxattr(struct dentry *d, char __user *list, size_t size)
 	return error;
 }
 
-static ssize_t path_listxattr(const char __user *pathname, char __user *list,
-			      size_t size, unsigned int lookup_flags)
+static
+ssize_t file_listxattr(struct file *f, char __user *list, size_t size)
+{
+	audit_file(f);
+	return listxattr(f->f_path.dentry, list, size);
+}
+
+/* unconditionally consumes filename */
+static
+ssize_t filename_listxattr(int dfd, struct filename *filename,
+			   unsigned int lookup_flags,
+			   char __user *list, size_t size)
 {
 	struct path path;
 	ssize_t error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
+		goto out;
 	error = listxattr(path.dentry, list, size);
 	path_put(&path);
 	if (retry_estale(error, lookup_flags)) {
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static ssize_t path_listxattr(const char __user *pathname, char __user *list,
+			      size_t size, unsigned int lookup_flags)
+{
+	return filename_listxattr(AT_FDCWD, getname(pathname), lookup_flags,
+				  list, size);
+}
+
 SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
@@ -924,8 +943,7 @@ SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
-	return listxattr(fd_file(f)->f_path.dentry, list, size);
+	return file_listxattr(fd_file(f), list, size);
 }
 
 /*
-- 
2.39.5

[PATCH v2 11/13] new helpers: file_removexattr(), filename_removexattr()

[Al Viro]

switch path_removexattrat() and fremovexattr(2) to those

Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 53 +++++++++++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index 988ea737ae7e..b76911b23293 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -957,23 +957,33 @@ removexattr(struct mnt_idmap *idmap, struct dentry *d, const char *name)
 	return vfs_removexattr(idmap, d, name);
 }
 
-static int path_removexattr(const char __user *pathname,
-			    const char __user *name, unsigned int lookup_flags)
+static int file_removexattr(struct file *f, struct xattr_name *kname)
+{
+	int error = mnt_want_write_file(f);
+
+	if (!error) {
+		audit_file(f);
+		error = removexattr(file_mnt_idmap(f),
+				    f->f_path.dentry, kname->name);
+		mnt_drop_write_file(f);
+	}
+	return error;
+}
+
+/* unconditionally consumes filename */
+static int filename_removexattr(int dfd, struct filename *filename,
+				unsigned int lookup_flags, struct xattr_name *kname)
 {
 	struct path path;
 	int error;
-	struct xattr_name kname;
 
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
 retry:
-	error = user_path_at(AT_FDCWD, pathname, lookup_flags, &path);
+	error = filename_lookup(dfd, filename, lookup_flags, &path, NULL);
 	if (error)
-		return error;
+		goto out;
 	error = mnt_want_write(path.mnt);
 	if (!error) {
-		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname.name);
+		error = removexattr(mnt_idmap(path.mnt), path.dentry, kname->name);
 		mnt_drop_write(path.mnt);
 	}
 	path_put(&path);
@@ -981,9 +991,24 @@ static int path_removexattr(const char __user *pathname,
 		lookup_flags |= LOOKUP_REVAL;
 		goto retry;
 	}
+out:
+	putname(filename);
 	return error;
 }
 
+static int path_removexattr(const char __user *pathname,
+			    const char __user *name, unsigned int lookup_flags)
+{
+	struct xattr_name kname;
+	int error;
+
+	error = import_xattr_name(&kname, name);
+	if (error)
+		return error;
+	return filename_removexattr(AT_FDCWD, getname(pathname), lookup_flags,
+				    &kname);
+}
+
 SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
 		const char __user *, name)
 {
@@ -1004,19 +1029,11 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 
 	if (fd_empty(f))
 		return -EBADF;
-	audit_file(fd_file(f));
 
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-
-	error = mnt_want_write_file(fd_file(f));
-	if (!error) {
-		error = removexattr(file_mnt_idmap(fd_file(f)),
-				    fd_file(f)->f_path.dentry, kname.name);
-		mnt_drop_write_file(fd_file(f));
-	}
-	return error;
+	return file_removexattr(fd_file(f), &kname);
 }
 
 int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
-- 
2.39.5

[PATCH v2 12/13] fs/xattr: add *at family syscalls

[Al Viro]

From: Christian Göttsche <cgzones@googlemail.com>

Add the four syscalls setxattrat(), getxattrat(), listxattrat() and
removexattrat().  Those can be used to operate on extended attributes,
especially security related ones, either relative to a pinned directory
or on a file descriptor without read access, avoiding a
/proc/<pid>/fd/<fd> detour, requiring a mounted procfs.

One use case will be setfiles(8) setting SELinux file contexts
("security.selinux") without race conditions and without a file
descriptor opened with read access requiring SELinux read permission.

Use the do_{name}at() pattern from fs/open.c.

Pass the value of the extended attribute, its length, and for
setxattrat(2) the command (XATTR_CREATE or XATTR_REPLACE) via an added
struct xattr_args to not exceed six syscall arguments and not
merging the AT_* and XATTR_* flags.

[AV: fixes by Christian Brauner folded in, the entire thing rebased on
top of {filename,file}_...xattr() primitives, treatment of empty
pathnames regularized.  As the result, AT_EMPTY_PATH+NULL handling
is cheap, so f...(2) can use it]

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Link: https://lore.kernel.org/r/20240426162042.191916-1-cgoettsche@seltendoof.de
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Christian Brauner <brauner@kernel.org>
CC: x86@kernel.org
CC: linux-alpha@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-arm-kernel@lists.infradead.org
CC: linux-ia64@vger.kernel.org
CC: linux-m68k@lists.linux-m68k.org
CC: linux-mips@vger.kernel.org
CC: linux-parisc@vger.kernel.org
CC: linuxppc-dev@lists.ozlabs.org
CC: linux-s390@vger.kernel.org
CC: linux-sh@vger.kernel.org
CC: sparclinux@vger.kernel.org
CC: linux-fsdevel@vger.kernel.org
CC: audit@vger.kernel.org
CC: linux-arch@vger.kernel.org
CC: linux-api@vger.kernel.org
CC: linux-security-module@vger.kernel.org
CC: selinux@vger.kernel.org
[brauner: slight tweaks]
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 arch/alpha/kernel/syscalls/syscall.tbl      |   4 +
 arch/arm/tools/syscall.tbl                  |   4 +
 arch/m68k/kernel/syscalls/syscall.tbl       |   4 +
 arch/microblaze/kernel/syscalls/syscall.tbl |   4 +
 arch/mips/kernel/syscalls/syscall_n32.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_n64.tbl   |   4 +
 arch/mips/kernel/syscalls/syscall_o32.tbl   |   4 +
 arch/parisc/kernel/syscalls/syscall.tbl     |   4 +
 arch/powerpc/kernel/syscalls/syscall.tbl    |   4 +
 arch/s390/kernel/syscalls/syscall.tbl       |   4 +
 arch/sh/kernel/syscalls/syscall.tbl         |   4 +
 arch/sparc/kernel/syscalls/syscall.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_32.tbl      |   4 +
 arch/x86/entry/syscalls/syscall_64.tbl      |   4 +
 arch/xtensa/kernel/syscalls/syscall.tbl     |   4 +
 fs/xattr.c                                  | 245 +++++++++++++-------
 include/asm-generic/audit_change_attr.h     |   6 +
 include/linux/syscalls.h                    |  13 ++
 include/linux/xattr.h                       |   4 +
 include/uapi/asm-generic/unistd.h           |  11 +-
 include/uapi/linux/xattr.h                  |   7 +
 21 files changed, 260 insertions(+), 86 deletions(-)

diff --git a/arch/alpha/kernel/syscalls/syscall.tbl b/arch/alpha/kernel/syscalls/syscall.tbl
index 74720667fe09..c59d53d6d3f3 100644
--- a/arch/alpha/kernel/syscalls/syscall.tbl
+++ b/arch/alpha/kernel/syscalls/syscall.tbl
@@ -502,3 +502,7 @@
 570	common	lsm_set_self_attr		sys_lsm_set_self_attr
 571	common	lsm_list_modules		sys_lsm_list_modules
 572	common  mseal				sys_mseal
+573	common	setxattrat			sys_setxattrat
+574	common	getxattrat			sys_getxattrat
+575	common	listxattrat			sys_listxattrat
+576	common	removexattrat			sys_removexattrat
diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
index 23c98203c40f..49eeb2ad8dbd 100644
--- a/arch/arm/tools/syscall.tbl
+++ b/arch/arm/tools/syscall.tbl
@@ -477,3 +477,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/m68k/kernel/syscalls/syscall.tbl b/arch/m68k/kernel/syscalls/syscall.tbl
index 22a3cbd4c602..f5ed71f1910d 100644
--- a/arch/m68k/kernel/syscalls/syscall.tbl
+++ b/arch/m68k/kernel/syscalls/syscall.tbl
@@ -462,3 +462,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/microblaze/kernel/syscalls/syscall.tbl b/arch/microblaze/kernel/syscalls/syscall.tbl
index 2b81a6bd78b2..680f568b77f2 100644
--- a/arch/microblaze/kernel/syscalls/syscall.tbl
+++ b/arch/microblaze/kernel/syscalls/syscall.tbl
@@ -468,3 +468,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n32.tbl b/arch/mips/kernel/syscalls/syscall_n32.tbl
index 953f5b7dc723..0b9b7e25b69a 100644
--- a/arch/mips/kernel/syscalls/syscall_n32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n32.tbl
@@ -401,3 +401,7 @@
 460	n32	lsm_set_self_attr		sys_lsm_set_self_attr
 461	n32	lsm_list_modules		sys_lsm_list_modules
 462	n32	mseal				sys_mseal
+463	n32	setxattrat			sys_setxattrat
+464	n32	getxattrat			sys_getxattrat
+465	n32	listxattrat			sys_listxattrat
+466	n32	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_n64.tbl b/arch/mips/kernel/syscalls/syscall_n64.tbl
index 1464c6be6eb3..c844cd5cda62 100644
--- a/arch/mips/kernel/syscalls/syscall_n64.tbl
+++ b/arch/mips/kernel/syscalls/syscall_n64.tbl
@@ -377,3 +377,7 @@
 460	n64	lsm_set_self_attr		sys_lsm_set_self_attr
 461	n64	lsm_list_modules		sys_lsm_list_modules
 462	n64	mseal				sys_mseal
+463	n64	setxattrat			sys_setxattrat
+464	n64	getxattrat			sys_getxattrat
+465	n64	listxattrat			sys_listxattrat
+466	n64	removexattrat			sys_removexattrat
diff --git a/arch/mips/kernel/syscalls/syscall_o32.tbl b/arch/mips/kernel/syscalls/syscall_o32.tbl
index 2439a2491cff..349b8aad1159 100644
--- a/arch/mips/kernel/syscalls/syscall_o32.tbl
+++ b/arch/mips/kernel/syscalls/syscall_o32.tbl
@@ -450,3 +450,7 @@
 460	o32	lsm_set_self_attr		sys_lsm_set_self_attr
 461	o32	lsm_list_modules		sys_lsm_list_modules
 462	o32	mseal				sys_mseal
+463	o32	setxattrat			sys_setxattrat
+464	o32	getxattrat			sys_getxattrat
+465	o32	listxattrat			sys_listxattrat
+466	o32	removexattrat			sys_removexattrat
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index 66dc406b12e4..d9fc94c86965 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -461,3 +461,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/powerpc/kernel/syscalls/syscall.tbl b/arch/powerpc/kernel/syscalls/syscall.tbl
index ebae8415dfbb..d8b4ab78bef0 100644
--- a/arch/powerpc/kernel/syscalls/syscall.tbl
+++ b/arch/powerpc/kernel/syscalls/syscall.tbl
@@ -553,3 +553,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/s390/kernel/syscalls/syscall.tbl b/arch/s390/kernel/syscalls/syscall.tbl
index 01071182763e..e9115b4d8b63 100644
--- a/arch/s390/kernel/syscalls/syscall.tbl
+++ b/arch/s390/kernel/syscalls/syscall.tbl
@@ -465,3 +465,7 @@
 460  common	lsm_set_self_attr	sys_lsm_set_self_attr		sys_lsm_set_self_attr
 461  common	lsm_list_modules	sys_lsm_list_modules		sys_lsm_list_modules
 462  common	mseal			sys_mseal			sys_mseal
+463  common	setxattrat		sys_setxattrat			sys_setxattrat
+464  common	getxattrat		sys_getxattrat			sys_getxattrat
+465  common	listxattrat		sys_listxattrat			sys_listxattrat
+466  common	removexattrat		sys_removexattrat		sys_removexattrat
diff --git a/arch/sh/kernel/syscalls/syscall.tbl b/arch/sh/kernel/syscalls/syscall.tbl
index c55fd7696d40..c8cad33bf250 100644
--- a/arch/sh/kernel/syscalls/syscall.tbl
+++ b/arch/sh/kernel/syscalls/syscall.tbl
@@ -466,3 +466,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/sparc/kernel/syscalls/syscall.tbl b/arch/sparc/kernel/syscalls/syscall.tbl
index cfdfb3707c16..727f99d333b3 100644
--- a/arch/sparc/kernel/syscalls/syscall.tbl
+++ b/arch/sparc/kernel/syscalls/syscall.tbl
@@ -508,3 +508,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal 				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 534c74b14fab..4d0fb2fba7e2 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -468,3 +468,7 @@
 460	i386	lsm_set_self_attr	sys_lsm_set_self_attr
 461	i386	lsm_list_modules	sys_lsm_list_modules
 462	i386	mseal 			sys_mseal
+463	i386	setxattrat		sys_setxattrat
+464	i386	getxattrat		sys_getxattrat
+465	i386	listxattrat		sys_listxattrat
+466	i386	removexattrat		sys_removexattrat
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 7093ee21c0d1..5eb708bff1c7 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -386,6 +386,10 @@
 460	common	lsm_set_self_attr	sys_lsm_set_self_attr
 461	common	lsm_list_modules	sys_lsm_list_modules
 462 	common  mseal			sys_mseal
+463	common	setxattrat		sys_setxattrat
+464	common	getxattrat		sys_getxattrat
+465	common	listxattrat		sys_listxattrat
+466	common	removexattrat		sys_removexattrat
 
 #
 # Due to a historical design error, certain syscalls are numbered differently
diff --git a/arch/xtensa/kernel/syscalls/syscall.tbl b/arch/xtensa/kernel/syscalls/syscall.tbl
index 67083fc1b2f5..37effc1b134e 100644
--- a/arch/xtensa/kernel/syscalls/syscall.tbl
+++ b/arch/xtensa/kernel/syscalls/syscall.tbl
@@ -433,3 +433,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal 				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/fs/xattr.c b/fs/xattr.c
index b76911b23293..deb336b821c9 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -676,69 +676,90 @@ int filename_setxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static int path_setxattr(const char __user *pathname,
-			 const char __user *name, const void __user *value,
-			 size_t size, int flags, unsigned int lookup_flags)
+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)
 {
 	struct xattr_name kname;
 	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
+		.cvalue	= value,
+		.kvalue	= NULL,
+		.size	= size,
+		.kname	= &kname,
+		.flags	= flags,
 	};
+	struct filename *filename;
+	unsigned int lookup_flags = 0;
 	int error;
 
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
+	if (!(at_flags & AT_SYMLINK_NOFOLLOW))
+		lookup_flags = LOOKUP_FOLLOW;
+
 	error = setxattr_copy(name, &ctx);
 	if (error)
 		return error;
 
-	error = filename_setxattr(AT_FDCWD, getname(pathname), lookup_flags,
-				  &ctx);
+	filename = getname_maybe_null(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			error = -EBADF;
+		else
+			error = file_setxattr(fd_file(f), &ctx);
+	} else {
+		error = filename_setxattr(dfd, filename, lookup_flags, &ctx);
+	}
 	kvfree(ctx.kvalue);
 	return error;
 }
 
+SYSCALL_DEFINE6(setxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+		const char __user *, name, const struct xattr_args __user *, uargs,
+		size_t, usize)
+{
+	struct xattr_args args = {};
+	int error;
+
+	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
+	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
+
+	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
+		return -EINVAL;
+	if (usize > PAGE_SIZE)
+		return -E2BIG;
+
+	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+	if (error)
+		return error;
+
+	return path_setxattrat(dfd, pathname, at_flags, name,
+			       u64_to_user_ptr(args.value), args.size,
+			       args.flags);
+}
+
 SYSCALL_DEFINE5(setxattr, const char __user *, pathname,
 		const char __user *, name, const void __user *, value,
 		size_t, size, int, flags)
 {
-	return path_setxattr(pathname, name, value, size, flags, LOOKUP_FOLLOW);
+	return path_setxattrat(AT_FDCWD, 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_setxattr(pathname, name, value, size, flags, 0);
+	return path_setxattrat(AT_FDCWD, 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)
 {
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.cvalue   = value,
-		.kvalue   = NULL,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = flags,
-	};
-	int error;
-
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-
-	error = setxattr_copy(name, &ctx);
-	if (error)
-		return error;
-
-	error = file_setxattr(fd_file(f), &ctx);
-	kvfree(ctx.kvalue);
-	return error;
+	return path_setxattrat(fd, NULL, AT_EMPTY_PATH, name,
+			       value, size, flags);
 }
 
 /*
@@ -804,11 +825,10 @@ ssize_t filename_getxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static ssize_t path_getxattr(const char __user *pathname,
-			     const char __user *name, void __user *value,
-			     size_t size, unsigned int lookup_flags)
+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)
 {
-	ssize_t error;
 	struct xattr_name kname;
 	struct kernel_xattr_ctx ctx = {
 		.value    = value,
@@ -816,44 +836,72 @@ static ssize_t path_getxattr(const char __user *pathname,
 		.kname    = &kname,
 		.flags    = 0,
 	};
+	struct filename *filename;
+	ssize_t error;
+
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
 
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-	return filename_getxattr(AT_FDCWD, getname(pathname), lookup_flags, &ctx);
+
+	filename = getname_maybe_null(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_getxattr(fd_file(f), &ctx);
+	} else {
+		int lookup_flags = 0;
+		if (!(at_flags & AT_SYMLINK_NOFOLLOW))
+			lookup_flags = LOOKUP_FOLLOW;
+		return filename_getxattr(dfd, filename, lookup_flags, &ctx);
+	}
+}
+
+SYSCALL_DEFINE6(getxattrat, int, dfd, const char __user *, pathname, unsigned int, at_flags,
+		const char __user *, name, struct xattr_args __user *, uargs, size_t, usize)
+{
+	struct xattr_args args = {};
+	int error;
+
+	BUILD_BUG_ON(sizeof(struct xattr_args) < XATTR_ARGS_SIZE_VER0);
+	BUILD_BUG_ON(sizeof(struct xattr_args) != XATTR_ARGS_SIZE_LATEST);
+
+	if (unlikely(usize < XATTR_ARGS_SIZE_VER0))
+		return -EINVAL;
+	if (usize > PAGE_SIZE)
+		return -E2BIG;
+
+	error = copy_struct_from_user(&args, sizeof(args), uargs, usize);
+	if (error)
+		return error;
+
+	if (args.flags != 0)
+		return -EINVAL;
+
+	return path_getxattrat(dfd, 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_getxattr(pathname, name, value, size, LOOKUP_FOLLOW);
+	return path_getxattrat(AT_FDCWD, pathname, 0, name, value, size);
 }
 
 SYSCALL_DEFINE4(lgetxattr, const char __user *, pathname,
 		const char __user *, name, void __user *, value, size_t, size)
 {
-	return path_getxattr(pathname, name, value, size, 0);
+	return path_getxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name,
+			       value, size);
 }
 
 SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 		void __user *, value, size_t, size)
 {
-	ssize_t error;
-	struct xattr_name kname;
-	struct kernel_xattr_ctx ctx = {
-		.value    = value,
-		.size     = size,
-		.kname    = &kname,
-		.flags    = 0,
-	};
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-	return file_getxattr(fd_file(f), &ctx);
+	return path_getxattrat(fd, NULL, AT_EMPTY_PATH, name, value, size);
 }
 
 /*
@@ -918,32 +966,50 @@ ssize_t filename_listxattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static ssize_t path_listxattr(const char __user *pathname, char __user *list,
-			      size_t size, unsigned int lookup_flags)
+static ssize_t path_listxattrat(int dfd, const char __user *pathname,
+				unsigned int at_flags, char __user *list,
+				size_t size)
+{
+	struct filename *filename;
+	int lookup_flags;
+
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
+	filename = getname_maybe_null(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_listxattr(fd_file(f), list, size);
+	}
+
+	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	return filename_listxattr(dfd, filename, lookup_flags, list, size);
+}
+
+SYSCALL_DEFINE5(listxattrat, int, dfd, const char __user *, pathname,
+		unsigned int, at_flags,
+		char __user *, list, size_t, size)
 {
-	return filename_listxattr(AT_FDCWD, getname(pathname), lookup_flags,
-				  list, size);
+	return path_listxattrat(dfd, pathname, at_flags, list, size);
 }
 
 SYSCALL_DEFINE3(listxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
-	return path_listxattr(pathname, list, size, LOOKUP_FOLLOW);
+	return path_listxattrat(AT_FDCWD, pathname, 0, list, size);
 }
 
 SYSCALL_DEFINE3(llistxattr, const char __user *, pathname, char __user *, list,
 		size_t, size)
 {
-	return path_listxattr(pathname, list, size, 0);
+	return path_listxattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, list, size);
 }
 
 SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 {
-	CLASS(fd, f)(fd);
-
-	if (fd_empty(f))
-		return -EBADF;
-	return file_listxattr(fd_file(f), list, size);
+	return path_listxattrat(fd, NULL, AT_EMPTY_PATH, list, size);
 }
 
 /*
@@ -996,44 +1062,53 @@ static int filename_removexattr(int dfd, struct filename *filename,
 	return error;
 }
 
-static int path_removexattr(const char __user *pathname,
-			    const char __user *name, unsigned int lookup_flags)
+static int path_removexattrat(int dfd, const char __user *pathname,
+			      unsigned int at_flags, const char __user *name)
 {
 	struct xattr_name kname;
+	struct filename *filename;
+	unsigned int lookup_flags;
 	int error;
 
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
 	error = import_xattr_name(&kname, name);
 	if (error)
 		return error;
-	return filename_removexattr(AT_FDCWD, getname(pathname), lookup_flags,
-				    &kname);
+
+	filename = getname_maybe_null(pathname, at_flags);
+	if (!filename) {
+		CLASS(fd, f)(dfd);
+		if (fd_empty(f))
+			return -EBADF;
+		return file_removexattr(fd_file(f), &kname);
+	}
+	lookup_flags = (at_flags & AT_SYMLINK_NOFOLLOW) ? 0 : LOOKUP_FOLLOW;
+	return filename_removexattr(dfd, filename, lookup_flags, &kname);
+}
+
+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);
 }
 
 SYSCALL_DEFINE2(removexattr, const char __user *, pathname,
 		const char __user *, name)
 {
-	return path_removexattr(pathname, name, LOOKUP_FOLLOW);
+	return path_removexattrat(AT_FDCWD, pathname, 0, name);
 }
 
 SYSCALL_DEFINE2(lremovexattr, const char __user *, pathname,
 		const char __user *, name)
 {
-	return path_removexattr(pathname, name, 0);
+	return path_removexattrat(AT_FDCWD, pathname, AT_SYMLINK_NOFOLLOW, name);
 }
 
 SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 {
-	CLASS(fd, f)(fd);
-	struct xattr_name kname;
-	int error;
-
-	if (fd_empty(f))
-		return -EBADF;
-
-	error = import_xattr_name(&kname, name);
-	if (error)
-		return error;
-	return file_removexattr(fd_file(f), &kname);
+	return path_removexattrat(fd, NULL, AT_EMPTY_PATH, name);
 }
 
 int xattr_list_one(char **buffer, ssize_t *remaining_size, const char *name)
diff --git a/include/asm-generic/audit_change_attr.h b/include/asm-generic/audit_change_attr.h
index 331670807cf0..cc840537885f 100644
--- a/include/asm-generic/audit_change_attr.h
+++ b/include/asm-generic/audit_change_attr.h
@@ -11,9 +11,15 @@ __NR_lchown,
 __NR_fchown,
 #endif
 __NR_setxattr,
+#ifdef __NR_setxattrat
+__NR_setxattrat,
+#endif
 __NR_lsetxattr,
 __NR_fsetxattr,
 __NR_removexattr,
+#ifdef __NR_removexattrat
+__NR_removexattrat,
+#endif
 __NR_lremovexattr,
 __NR_fremovexattr,
 #ifdef __NR_fchownat
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 5758104921e6..c6333204d451 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -77,6 +77,7 @@ struct cachestat_range;
 struct cachestat;
 struct statmount;
 struct mnt_id_req;
+struct xattr_args;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -338,23 +339,35 @@ asmlinkage long sys_io_uring_register(unsigned int fd, unsigned int op,
 				void __user *arg, unsigned int nr_args);
 asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
 			     const void __user *value, size_t size, int flags);
+asmlinkage long sys_setxattrat(int dfd, const char __user *path, unsigned int at_flags,
+			       const char __user *name,
+			       const struct xattr_args __user *args, size_t size);
 asmlinkage long sys_lsetxattr(const char __user *path, const char __user *name,
 			      const void __user *value, size_t size, int flags);
 asmlinkage long sys_fsetxattr(int fd, const char __user *name,
 			      const void __user *value, size_t size, int flags);
 asmlinkage long sys_getxattr(const char __user *path, const char __user *name,
 			     void __user *value, size_t size);
+asmlinkage long sys_getxattrat(int dfd, const char __user *path, unsigned int at_flags,
+			       const char __user *name,
+			       struct xattr_args __user *args, size_t size);
 asmlinkage long sys_lgetxattr(const char __user *path, const char __user *name,
 			      void __user *value, size_t size);
 asmlinkage long sys_fgetxattr(int fd, const char __user *name,
 			      void __user *value, size_t size);
 asmlinkage long sys_listxattr(const char __user *path, char __user *list,
 			      size_t size);
+asmlinkage long sys_listxattrat(int dfd, const char __user *path,
+				unsigned int at_flags,
+				char __user *list, size_t size);
 asmlinkage long sys_llistxattr(const char __user *path, char __user *list,
 			       size_t size);
 asmlinkage long sys_flistxattr(int fd, char __user *list, size_t size);
 asmlinkage long sys_removexattr(const char __user *path,
 				const char __user *name);
+asmlinkage long sys_removexattrat(int dfd, const char __user *path,
+				  unsigned int at_flags,
+				  const char __user *name);
 asmlinkage long sys_lremovexattr(const char __user *path,
 				 const char __user *name);
 asmlinkage long sys_fremovexattr(int fd, const char __user *name);
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index d20051865800..86b0d47984a1 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -19,6 +19,10 @@
 #include <linux/user_namespace.h>
 #include <uapi/linux/xattr.h>
 
+/* List of all open_how "versions". */
+#define XATTR_ARGS_SIZE_VER0	16 /* sizeof first published struct */
+#define XATTR_ARGS_SIZE_LATEST	XATTR_ARGS_SIZE_VER0
+
 struct inode;
 struct dentry;
 
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 5bf6148cac2b..88dc393c2bca 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -841,8 +841,17 @@ __SYSCALL(__NR_lsm_list_modules, sys_lsm_list_modules)
 #define __NR_mseal 462
 __SYSCALL(__NR_mseal, sys_mseal)
 
+#define __NR_setxattrat 463
+__SYSCALL(__NR_setxattrat, sys_setxattrat)
+#define __NR_getxattrat 464
+__SYSCALL(__NR_getxattrat, sys_getxattrat)
+#define __NR_listxattrat 465
+__SYSCALL(__NR_listxattrat, sys_listxattrat)
+#define __NR_removexattrat 466
+__SYSCALL(__NR_removexattrat, sys_removexattrat)
+
 #undef __NR_syscalls
-#define __NR_syscalls 463
+#define __NR_syscalls 467
 
 /*
  * 32 bit systems traditionally used different
diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 9463db2dfa9d..9854f9cff3c6 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h
@@ -11,6 +11,7 @@
 */
 
 #include <linux/libc-compat.h>
+#include <linux/types.h>
 
 #ifndef _UAPI_LINUX_XATTR_H
 #define _UAPI_LINUX_XATTR_H
@@ -20,6 +21,12 @@
 
 #define XATTR_CREATE	0x1	/* set value, fail if attr already exists */
 #define XATTR_REPLACE	0x2	/* set value, fail if attr does not exist */
+
+struct xattr_args {
+	__aligned_u64 __user value;
+	__u32 size;
+	__u32 flags;
+};
 #endif
 
 /* Namespaces */
-- 
2.39.5

[PATCH v2 13/13] xattr: remove redundant check on variable err

[Al Viro]

From: Colin Ian King <colin.i.king@gmail.com>

Curretly in function generic_listxattr the for_each_xattr_handler loop
checks err and will return out of the function if err is non-zero.
It's impossible for err to be non-zero at the end of the function where
err is checked again for a non-zero value. The final non-zero check is
therefore redundant and can be removed. Also move the declaration of
err into the loop.

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/xattr.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xattr.c b/fs/xattr.c
index deb336b821c9..02bee149ad96 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -1142,9 +1142,10 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 {
 	const struct xattr_handler *handler, * const *handlers = dentry->d_sb->s_xattr;
 	ssize_t remaining_size = buffer_size;
-	int err = 0;
 
 	for_each_xattr_handler(handlers, handler) {
+		int err;
+
 		if (!handler->name || (handler->list && !handler->list(dentry)))
 			continue;
 		err = xattr_list_one(&buffer, &remaining_size, handler->name);
@@ -1152,7 +1153,7 @@ generic_listxattr(struct dentry *dentry, char *buffer, size_t buffer_size)
 			return err;
 	}
 
-	return err ? err : buffer_size - remaining_size;
+	return buffer_size - remaining_size;
 }
 EXPORT_SYMBOL(generic_listxattr);
 
-- 
2.39.5

Re: [RFC][PATCHES v2] xattr stuff and interactions with io_uring

[Jens Axboe]

On 11/2/24 1:28 AM, Al Viro wrote:
> 	Changes since v1:
> * moved on top of (and makes use of) getname_maybe_null() stuff
> (first two commits here, form #base.getname-fixed)
> * fixed a leak on io_uring side spotted by Jens
> * putname(ERR_PTR(-E...)) is a no-op; allows to simplify things on
> io_uring side.
> * applied reviewed-by
> * picked a generic_listxattr() cleanup from Colin Ian King
> 
> 	Help with review and testing would be welcome; if nobody objects,
> to #for-next it goes...

Tested on arm64, fwiw I get these:

<stdin>:1603:2: warning: #warning syscall setxattrat not implemented [-Wcpp]
<stdin>:1606:2: warning: #warning syscall getxattrat not implemented [-Wcpp]
<stdin>:1609:2: warning: #warning syscall listxattrat not implemented [-Wcpp]
<stdin>:1612:2: warning: #warning syscall removexattrat not implemented [-Wcpp]

when compiling. But ran it through the usual testing on the io_uring side,
and it looks fine to me. Didn't test the xattr* stuff separately, assuming
I'd just be re-running what you already did on that side.

-- 
Jens Axboe

Re: [RFC][PATCHES v2] xattr stuff and interactions with io_uring

[Al Viro]

On Sat, Nov 02, 2024 at 08:43:31AM -0600, Jens Axboe wrote:
> On 11/2/24 1:28 AM, Al Viro wrote:
> > 	Changes since v1:
> > * moved on top of (and makes use of) getname_maybe_null() stuff
> > (first two commits here, form #base.getname-fixed)
> > * fixed a leak on io_uring side spotted by Jens
> > * putname(ERR_PTR(-E...)) is a no-op; allows to simplify things on
> > io_uring side.
> > * applied reviewed-by
> > * picked a generic_listxattr() cleanup from Colin Ian King
> > 
> > 	Help with review and testing would be welcome; if nobody objects,
> > to #for-next it goes...
> 
> Tested on arm64, fwiw I get these:
> 
> <stdin>:1603:2: warning: #warning syscall setxattrat not implemented [-Wcpp]
> <stdin>:1606:2: warning: #warning syscall getxattrat not implemented [-Wcpp]
> <stdin>:1609:2: warning: #warning syscall listxattrat not implemented [-Wcpp]
> <stdin>:1612:2: warning: #warning syscall removexattrat not implemented [-Wcpp]

arch/arm64/tools/syscall*.tbl bits are missing (as well as
arch/sparc/kernel/syscall_32.tbl ones, but that's less of an
issue).

AFAICS, the following should be the right incremental.  Objections, anyone?


diff --git a/arch/arm64/tools/syscall_32.tbl b/arch/arm64/tools/syscall_32.tbl
index 9a37930d4e26..69a829912a05 100644
--- a/arch/arm64/tools/syscall_32.tbl
+++ b/arch/arm64/tools/syscall_32.tbl
@@ -474,3 +474,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat
diff --git a/scripts/syscall.tbl b/scripts/syscall.tbl
index 845e24eb372e..ebbdb3c42e9f 100644
--- a/scripts/syscall.tbl
+++ b/scripts/syscall.tbl
@@ -403,3 +403,7 @@
 460	common	lsm_set_self_attr		sys_lsm_set_self_attr
 461	common	lsm_list_modules		sys_lsm_list_modules
 462	common	mseal				sys_mseal
+463	common	setxattrat			sys_setxattrat
+464	common	getxattrat			sys_getxattrat
+465	common	listxattrat			sys_listxattrat
+466	common	removexattrat			sys_removexattrat

Re: [RFC][PATCHES v2] xattr stuff and interactions with io_uring

[Arnd Bergmann]

On Sun, Nov 3, 2024, at 07:51, Al Viro wrote:
> On Sat, Nov 02, 2024 at 08:43:31AM -0600, Jens Axboe wrote:
>> Tested on arm64, fwiw I get these:
>> 
>> <stdin>:1603:2: warning: #warning syscall setxattrat not implemented [-Wcpp]
>> <stdin>:1606:2: warning: #warning syscall getxattrat not implemented [-Wcpp]
>> <stdin>:1609:2: warning: #warning syscall listxattrat not implemented [-Wcpp]
>> <stdin>:1612:2: warning: #warning syscall removexattrat not implemented [-Wcpp]
>
> arch/arm64/tools/syscall*.tbl bits are missing (as well as
> arch/sparc/kernel/syscall_32.tbl ones, but that's less of an
> issue).
>
> AFAICS, the following should be the right incremental.  Objections, anyone?

Looks fine to me.

I have a patch to convert s390 to use the exact same format
as the others, and I should push that patch, but it slightly
conflict with this one.

We can also remove the old include/uapi/asm-generic/unistd.h
that is no longer used.

I was planning to have a patch by now to only need to chance a
single .tbl file for new entries, which is a bit behind some
other work I have planned for these files.

      Arnd

Re: [RFC][PATCHES v2] xattr stuff and interactions with io_uring

[Al Viro]

On Sun, Nov 03, 2024 at 02:57:14PM +0100, Arnd Bergmann wrote:
> On Sun, Nov 3, 2024, at 07:51, Al Viro wrote:
> > On Sat, Nov 02, 2024 at 08:43:31AM -0600, Jens Axboe wrote:
> >> Tested on arm64, fwiw I get these:
> >> 
> >> <stdin>:1603:2: warning: #warning syscall setxattrat not implemented [-Wcpp]
> >> <stdin>:1606:2: warning: #warning syscall getxattrat not implemented [-Wcpp]
> >> <stdin>:1609:2: warning: #warning syscall listxattrat not implemented [-Wcpp]
> >> <stdin>:1612:2: warning: #warning syscall removexattrat not implemented [-Wcpp]
> >
> > arch/arm64/tools/syscall*.tbl bits are missing (as well as
> > arch/sparc/kernel/syscall_32.tbl ones, but that's less of an
> > issue).
> >
> > AFAICS, the following should be the right incremental.  Objections, anyone?
> 
> Looks fine to me.
> 
> I have a patch to convert s390 to use the exact same format
> as the others, and I should push that patch, but it slightly
> conflict with this one.
> 
> We can also remove the old include/uapi/asm-generic/unistd.h
> that is no longer used.

I'd suggest starting with Documentation/process/adding-syscalls.rst, then...