On Wed, Sep 25, 2024 at 12:01:01AM -0600, Jens Axboe wrote:
> The normal policy is that anything that is read-only should remain
> stable after ->prep() has been called, so that ->issue() can use it.
> That means the application can keep it on-stack as long as it's valid
> until io_uring_submit() returns. For structs/buffers that are copied to
> after IO, those the application obviously need to keep around until they
> see a completion for that request. So yes, for the xattr cases where the
> struct is copied to at completion time, those do not need to be stable
> after ->prep(), could be handled purely on the ->issue() side.
Looks like io_fsetxattr() was missing audit_file()... Anyway, in a local
branch I've added two helpers -
int file_setxattr(struct file *file, struct xattr_ctx *ctx);
int filename_setxattr(int dfd, struct filename *filename,
struct xattr_ctx *ctx, unsigned int lookup_flags);
and converted both fs/xattr.c and io_uring/xattr.c to those.
Completely untested delta follows; it's _not_ in the final form,
it misses getxattr side, etc.
BTW, I think fs/internal.h is a very wrong place for that, as well as
for do_mkdirat() et.al. include/linux/marshalled_syscalls.h, perhaps?
diff --git a/fs/internal.h b/fs/internal.h
index 8cf42b327e5e..e39f80201ff8 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -285,8 +285,9 @@ ssize_t do_getxattr(struct mnt_idmap *idmap,
struct xattr_ctx *ctx);
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);
+int file_setxattr(struct file *file, struct xattr_ctx *ctx);
+int filename_setxattr(int dfd, struct filename *filename,
+ struct xattr_ctx *ctx, unsigned int lookup_flags);
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..fc6409181c46 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -619,7 +619,7 @@ int setxattr_copy(const char __user *name, struct 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 xattr_ctx *ctx)
{
if (is_posix_acl_xattr(ctx->kname->name))
@@ -630,32 +630,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 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,
+ struct xattr_ctx *ctx, unsigned int lookup_flags)
{
- struct xattr_name kname;
- struct 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);
@@ -665,6 +664,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 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),
+ &ctx, lookup_flags);
kvfree(ctx.kvalue);
return error;
}
@@ -700,17 +723,11 @@ 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 13e8d7d2cdc2..702d5981fd63 100644
--- a/io_uring/xattr.c
+++ b/io_uring/xattr.c
@@ -203,28 +203,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;
}
@@ -232,22 +218,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, &ix->ctx, LOOKUP_FOLLOW);
io_xattr_finish(req, ret);
return IOU_OK;
