* [RFC][PATCHES] xattr stuff and interactions with io_uring
@ 2024-10-02 1:10 Al Viro
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:10 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christian Brauner, io-uring, Christian Göttsche
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.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 1/9] xattr: switch to CLASS(fd)
2024-10-02 1:10 [RFC][PATCHES] xattr stuff and interactions with io_uring Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 1:22 ` [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx Al Viro
` (8 more replies)
0 siblings, 9 replies; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
Signed-off-by: Al Viro <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 5:56 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 3/9] io_[gs]etxattr_prep(): just use getname() Al Viro
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
From: Christian Göttsche <[email protected]>
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 <[email protected]>
Signed-off-by: Christian Göttsche <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Jan Kara <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/9] io_[gs]etxattr_prep(): just use getname()
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
2024-10-02 1:22 ` [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 5:57 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 4/9] new helper: import_xattr_name() Al Viro
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/9] new helper: import_xattr_name()
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
2024-10-02 1:22 ` [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx Al Viro
2024-10-02 1:22 ` [PATCH 3/9] io_[gs]etxattr_prep(): just use getname() Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 5:57 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 5/9] replace do_setxattr() with saner helpers Al Viro
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
common logics for marshalling xattr names.
Signed-off-by: Al Viro <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (2 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 4/9] new helper: import_xattr_name() Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 1:34 ` Jens Axboe
2024-10-02 1:22 ` [PATCH 6/9] replace do_getxattr() " Al Viro
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/9] replace do_getxattr() with saner helpers.
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (3 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 5/9] replace do_setxattr() with saner helpers Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 5:59 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr() Al Viro
` (3 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
similar to do_setxattr() in the previous commit...
Signed-off-by: Al Viro <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr()
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (4 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 6/9] replace do_getxattr() " Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 6:00 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr() Al Viro
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
switch path_listxattr() and flistxattr(2) to those
Signed-off-by: Al Viro <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr()
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (5 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr() Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 6:01 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 9/9] fs/xattr: add *at family syscalls Al Viro
2024-10-02 5:56 ` [PATCH 1/9] xattr: switch to CLASS(fd) Christian Brauner
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
switch path_removexattrat() and fremovexattr(2) to those
Signed-off-by: Al Viro <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 9/9] fs/xattr: add *at family syscalls
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (6 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr() Al Viro
@ 2024-10-02 1:22 ` Al Viro
2024-10-02 6:03 ` Christian Brauner
2024-10-02 5:56 ` [PATCH 1/9] xattr: switch to CLASS(fd) Christian Brauner
8 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 1:22 UTC (permalink / raw)
To: linux-fsdevel; +Cc: brauner, io-uring, cgzones
From: Christian Göttsche <[email protected]>
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 <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Reviewed-by: Arnd Bergmann <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
[brauner: slight tweaks]
Signed-off-by: Christian Brauner <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 1:22 ` [PATCH 5/9] replace do_setxattr() with saner helpers Al Viro
@ 2024-10-02 1:34 ` Jens Axboe
2024-10-02 2:08 ` Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Jens Axboe @ 2024-10-02 1:34 UTC (permalink / raw)
To: Al Viro, linux-fsdevel; +Cc: brauner, io-uring, cgzones
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 1:34 ` Jens Axboe
@ 2024-10-02 2:08 ` Al Viro
2024-10-02 18:00 ` Jens Axboe
0 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 2:08 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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 <[email protected]>
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 <[email protected]>
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;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 1/9] xattr: switch to CLASS(fd)
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
` (7 preceding siblings ...)
2024-10-02 1:22 ` [PATCH 9/9] fs/xattr: add *at family syscalls Al Viro
@ 2024-10-02 5:56 ` Christian Brauner
8 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 5:56 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
On Wed, Oct 02, 2024 at 02:22:22AM GMT, Al Viro wrote:
> Signed-off-by: Al Viro <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx
2024-10-02 1:22 ` [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx Al Viro
@ 2024-10-02 5:56 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 5:56 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
On Wed, Oct 02, 2024 at 02:22:23AM GMT, Al Viro wrote:
> From: Christian Göttsche <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Christian Göttsche <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Reviewed-by: Jan Kara <[email protected]>
> Signed-off-by: Christian Brauner <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/9] io_[gs]etxattr_prep(): just use getname()
2024-10-02 1:22 ` [PATCH 3/9] io_[gs]etxattr_prep(): just use getname() Al Viro
@ 2024-10-02 5:57 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 5:57 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
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 <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/9] new helper: import_xattr_name()
2024-10-02 1:22 ` [PATCH 4/9] new helper: import_xattr_name() Al Viro
@ 2024-10-02 5:57 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 5:57 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
On Wed, Oct 02, 2024 at 02:22:25AM GMT, Al Viro wrote:
> common logics for marshalling xattr names.
>
> Signed-off-by: Al Viro <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 6/9] replace do_getxattr() with saner helpers.
2024-10-02 1:22 ` [PATCH 6/9] replace do_getxattr() " Al Viro
@ 2024-10-02 5:59 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 5:59 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
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 <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr()
2024-10-02 1:22 ` [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr() Al Viro
@ 2024-10-02 6:00 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 6:00 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
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 <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr()
2024-10-02 1:22 ` [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr() Al Viro
@ 2024-10-02 6:01 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 6:01 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
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 <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 9/9] fs/xattr: add *at family syscalls
2024-10-02 1:22 ` [PATCH 9/9] fs/xattr: add *at family syscalls Al Viro
@ 2024-10-02 6:03 ` Christian Brauner
0 siblings, 0 replies; 25+ messages in thread
From: Christian Brauner @ 2024-10-02 6:03 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, io-uring, cgzones
On Wed, Oct 02, 2024 at 02:22:30AM GMT, Al Viro wrote:
> From: Christian Göttsche <[email protected]>
>
> 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 <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Reviewed-by: Arnd Bergmann <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> [brauner: slight tweaks]
> Signed-off-by: Christian Brauner <[email protected]>
> ---
Reviewed-by: Christian Brauner <[email protected]>
> 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
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 2:08 ` Al Viro
@ 2024-10-02 18:00 ` Jens Axboe
2024-10-02 21:19 ` Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Jens Axboe @ 2024-10-02 18:00 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 18:00 ` Jens Axboe
@ 2024-10-02 21:19 ` Al Viro
2024-10-02 22:55 ` Jens Axboe
0 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-02 21:19 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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...
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 21:19 ` Al Viro
@ 2024-10-02 22:55 ` Jens Axboe
2024-10-06 5:28 ` Al Viro
0 siblings, 1 reply; 25+ messages in thread
From: Jens Axboe @ 2024-10-02 22:55 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-02 22:55 ` Jens Axboe
@ 2024-10-06 5:28 ` Al Viro
2024-10-07 18:09 ` Jens Axboe
0 siblings, 1 reply; 25+ messages in thread
From: Al Viro @ 2024-10-06 5:28 UTC (permalink / raw)
To: Jens Axboe; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] replace do_setxattr() with saner helpers.
2024-10-06 5:28 ` Al Viro
@ 2024-10-07 18:09 ` Jens Axboe
0 siblings, 0 replies; 25+ messages in thread
From: Jens Axboe @ 2024-10-07 18:09 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, brauner, io-uring, cgzones
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
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2024-10-07 18:09 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-02 1:10 [RFC][PATCHES] xattr stuff and interactions with io_uring Al Viro
2024-10-02 1:22 ` [PATCH 1/9] xattr: switch to CLASS(fd) Al Viro
2024-10-02 1:22 ` [PATCH 2/9] fs: rename struct xattr_ctx to kernel_xattr_ctx Al Viro
2024-10-02 5:56 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 3/9] io_[gs]etxattr_prep(): just use getname() Al Viro
2024-10-02 5:57 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 4/9] new helper: import_xattr_name() Al Viro
2024-10-02 5:57 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 5/9] replace do_setxattr() with saner helpers Al Viro
2024-10-02 1:34 ` Jens Axboe
2024-10-02 2:08 ` Al Viro
2024-10-02 18:00 ` Jens Axboe
2024-10-02 21:19 ` Al Viro
2024-10-02 22:55 ` Jens Axboe
2024-10-06 5:28 ` Al Viro
2024-10-07 18:09 ` Jens Axboe
2024-10-02 1:22 ` [PATCH 6/9] replace do_getxattr() " Al Viro
2024-10-02 5:59 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 7/9] new helpers: file_listxattr(), filename_listxattr() Al Viro
2024-10-02 6:00 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 8/9] new helpers: file_removexattr(), filename_removexattr() Al Viro
2024-10-02 6:01 ` Christian Brauner
2024-10-02 1:22 ` [PATCH 9/9] fs/xattr: add *at family syscalls Al Viro
2024-10-02 6:03 ` Christian Brauner
2024-10-02 5:56 ` [PATCH 1/9] xattr: switch to CLASS(fd) Christian Brauner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox