Re: [PATCH 06/10] io_uring: add support for futex wake and wait

Jens Axboe
2023-07-21T15:29:23Z

On 7/21/23 8:43?AM, Jens Axboe wrote:
> On 7/21/23 5:37?AM, Peter Zijlstra wrote:
>> On Fri, Jul 21, 2023 at 01:30:31PM +0200, Peter Zijlstra wrote:
>>
>> Sorry, I was too quick..
>>
>> iof->uaddr = sqe->addr;
>> iof->val = sqe->futex_val;
>> iof->mask = sqe->futex_mask;
>> flags = sqe->futex_flags;
>>
>> if (flags & ~FUTEX2_MASK)
>> return -EINVAL;
>>
>> iof->flags = futex2_to_flags(flags);
>> if (!futex_flags_valid(iof->flags))
>> return -EINVAL;
>>
>> if (!futex_validate_input(iof->flags, iof->val) ||
>> !futex_validate_input(iof->flags, iof->mask))
>> return -EINVAL
>
> Something like that should work, with some variable names fixed up. I
> just went with 'addr' for the futex address, addr2 for the value, and
> addr3 for the mask.
>
> Rebased on top of your first 4 updated patches, and added a single patch
> that moves FUTEX2_MASK, will run some testing to validate it's all still
> sane.

FWIW, here's the io_uring incremental after that rebase. Update the
liburing futex branch as well, updating the prep helpers to take 64 bit
values for mask/val and also add the flags argument that was missing as
well. Only other addition was adding those 4 new patches instead of the
old 3 ones, and adding single patch that just moves FUTEX2_MASK to
futex.h.

All checks out fine, tests pass and it works.

diff --git a/io_uring/futex.c b/io_uring/futex.c
index 93df54dffaa0..4c9f2c841b98 100644
--- a/io_uring/futex.c
+++ b/io_uring/futex.c
@@ -18,11 +18,11 @@ struct io_futex {
u32 __user *uaddr;
struct futex_waitv __user *uwaitv;
};
- unsigned int futex_val;
- unsigned int futex_flags;
- unsigned int futex_mask;
- unsigned int futex_nr;
+ unsigned long futex_val;
+ unsigned long futex_mask;
unsigned long futexv_owned;
+ u32 futex_flags;
+ unsigned int futex_nr;
};

struct io_futex_data {
@@ -171,15 +171,28 @@ bool io_futex_remove_all(struct io_ring_ctx *ctx, struct task_struct *task,
int io_futex_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_futex *iof = io_kiocb_to_cmd(req, struct io_futex);
+ u32 flags;

- if (unlikely(sqe->fd || sqe->buf_index || sqe->addr3))
+ if (unlikely(sqe->fd || sqe->buf_index || sqe->file_index))
return -EINVAL;

iof->uaddr = u64_to_user_ptr(READ_ONCE(sqe->addr));
- iof->futex_val = READ_ONCE(sqe->len);
- iof->futex_mask = READ_ONCE(sqe->file_index);
- iof->futex_flags = READ_ONCE(sqe->futex_flags);
- if (iof->futex_flags & FUTEX_CMD_MASK)
+ iof->futex_val = READ_ONCE(sqe->addr2);
+ iof->futex_mask = READ_ONCE(sqe->addr3);
+ iof->futex_nr = READ_ONCE(sqe->len);
+ if (iof->futex_nr && req->opcode != IORING_OP_FUTEX_WAITV)
+ return -EINVAL;
+
+ flags = READ_ONCE(sqe->futex_flags);
+ if (flags & ~FUTEX2_MASK)
+ return -EINVAL;
+
+ iof->futex_flags = futex2_to_flags(flags);
+ if (!futex_flags_valid(iof->futex_flags))
+ return -EINVAL;
+
+ if (!futex_validate_input(iof->futex_flags, iof->futex_val) ||
+ !futex_validate_input(iof->futex_flags, iof->futex_mask))
return -EINVAL;

iof->futexv_owned = 0;
@@ -211,7 +224,6 @@ int io_futexv_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
if (ret)
return ret;

- iof->futex_nr = READ_ONCE(sqe->off);
if (!iof->futex_nr || iof->futex_nr > FUTEX_WAITV_MAX)
return -EINVAL;

--
Jens Axboe