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

[Caleb Sander Mateos]

On Mon, May 18, 2026 at 7:49 AM Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> 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.

Oh, I missed that the only stores in between are to other fields of
*req. Yeah, the compiler should be able to tell that those don't alias
req->opcode. Removing the local variable sounds good.

Thanks,
Caleb