On 12/15/21 22:17, Stefan Roesch wrote:
This adds support to io_uring for the fsetxattr and setxattr API.
Apart from potentially putname comment,
Reviewed-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
Signed-off-by: Stefan Roesch <shr@xxxxxx>
---
fs/io_uring.c | 171 ++++++++++++++++++++++++++++++++++
include/uapi/linux/io_uring.h | 6 +-
2 files changed, 176 insertions(+), 1 deletion(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 5092dfe56da6..fc2239635342 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
[...]> +static int __io_setxattr_prep(struct io_kiocb *req,
+ const struct io_uring_sqe *sqe,
+ struct user_namespace *user_ns)
+{
+ struct io_xattr *ix = &req->xattr;
+ const char __user *name;
+ void *ret;
+
+ if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
+ return -EINVAL;
+ if (unlikely(sqe->ioprio))
+ return -EINVAL;
+ if (unlikely(req->flags & REQ_F_FIXED_FILE))
+ return -EBADF;
+
+ ix->filename = NULL;
+ name = u64_to_user_ptr(READ_ONCE(sqe->addr));
+ 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);
+
+ ix->ctx.kname = kmalloc(XATTR_NAME_MAX + 1, GFP_KERNEL);
+ if (!ix->ctx.kname)
+ return -ENOMEM;> + ix->ctx.kname_sz = XATTR_NAME_MAX + 1;
Might make sense to let the userspace specify kname size, but
depends what is the average name length and how much of free
space we have in io_xattr
[...]
+static int io_setxattr(struct io_kiocb *req, unsigned int issue_flags)
+{
+ struct io_xattr *ix = &req->xattr;
+ unsigned int lookup_flags = LOOKUP_FOLLOW;
+ struct path path;
+ int ret;
+
+ if (issue_flags & IO_URING_F_NONBLOCK)
+ return -EAGAIN;
+
+retry:
+ ret = do_user_path_at_empty(AT_FDCWD, ix->filename, lookup_flags, &path);
+ putname(ix->filename);
It putname() multiple times on retry, how does it work?
+ if (!ret) {
+ ret = __io_setxattr(req, issue_flags, &path);
+ path_put(&path);
+ if (retry_estale(ret, lookup_flags)) {
+ lookup_flags |= LOOKUP_REVAL;
+ goto retry;
+ }
+ }
+
+ req->flags &= ~REQ_F_NEED_CLEANUP;
+ kfree(ix->ctx.kname);
+
+ if (ix->value)
+ kvfree(ix->value);
+ if (ret < 0)
+ req_set_fail(req);
+
+ io_req_complete(req, ret);
+ return 0;
+}
+
[...]
printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
@@ -6715,6 +6870,15 @@ static void io_clean_op(struct io_kiocb *req)
putname(req->hardlink.oldpath);
putname(req->hardlink.newpath);
break;
+ case IORING_OP_SETXATTR:
+ if (req->xattr.filename)
+ putname(req->xattr.filename);
+ fallthrough;
+ case IORING_OP_FSETXATTR:
+ kfree(req->xattr.ctx.kname);
+ if (req->xattr.value)
+ kvfree(req->xattr.value);
nit: it's slow path and kvfree() handles NULLs, so we don't
really need NULL checks here.
--
Pavel Begunkov
