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

[Jens Axboe <axboe@kernel.dk>]

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

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


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

-- 
Jens Axboe