Re: [PATCH v4 4/5] io_uring: add fsetxattr and setxattr support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux