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
