Re: [PATCH v2] io_uring: propagate array_index_nospec opcode into req->opcode

[Jens Axboe]

On 5/18/26 8:42 AM, Caleb Sander Mateos wrote:
> On Sun, May 17, 2026 at 2:30?PM Michael Bommarito
> <michael.bommarito@xxxxxxxxx> wrote:
>>
>> Commit 1e988c3fe126 ("io_uring: prevent opcode speculation") added
>> array_index_nospec() to io_init_req(), but applied it only to a local
>> opcode variable. req->opcode is initialized from sqe->opcode before the
>> bounds check and remains the raw value.
>>
>> Keep req->opcode as the canonical opcode in io_init_req(): reject
>> out-of-range values architecturally, then write the array_index_nospec()
>> result back to req->opcode before any table lookup. This keeps downstream
>> users of req->opcode from observing the raw user byte on a mispredicted
>> path.
>>
>> No functional change: array_index_nospec() is a no-op for opcodes in
>> [0, IORING_OP_LAST), and out-of-range opcodes are still rejected at the
>> bounds check above the assignment. Boot-tested under UML (x86_64
>> defconfig) by building stock and patched kernels and running a 54-test
>> subset of liburing against each; pass/fail results were identical.
>>
>> Fixes: 1e988c3fe126 ("io_uring: prevent opcode speculation")
>>
>> Assisted-by: Claude:claude-opus-4-7
>> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
>> ---
>> v2:
>> - Fold the clamped value into req->opcode and use req->opcode for
>>   the io_issue_defs[] lookup, rather than keeping a second local
>>   opcode variable. Suggested by Jens.
>> - Keep the hardening-only framing; no functional behavior change.
>>
>>  io_uring/io_uring.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index 4ed998d60c09c..84e16c3ad3f47 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -1721,10 +1721,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
>>         const struct io_issue_def *def;
>>         unsigned int sqe_flags;
>>         int personality;
>> -       u8 opcode;
>>
>>         req->ctx = ctx;
>> -       req->opcode = opcode = READ_ONCE(sqe->opcode);
>> +       req->opcode = READ_ONCE(sqe->opcode);
> 
> The local variable should improve performance, I'm not sure removing
> it is a good idea. Due to the intervening stores, the compiler can't
> tell that req->opcode is unchanged between this assignment and the
> later loads, so it will have to reload it from memory. Can you just
> assign to the local variable opcode here and wait to assign to
> req->opcode until after updating opcode with array_index_nospec()?

It generated the same code on my end, using gcc and arm64. If that's not
the case for you, yeah then retaining the local variable would be fine
too, like v1 did.

-- 
Jens Axboe