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

[Caleb Sander Mateos]

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()?

Best,
Caleb

>         /* same numerical values with corresponding REQ_F_*, safe to copy */
>         sqe_flags = READ_ONCE(sqe->flags);
>         req->flags = (__force io_req_flags_t) sqe_flags;
> @@ -1734,13 +1733,13 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
>         req->cancel_seq_set = false;
>         req->async_data = NULL;
>
> -       if (unlikely(opcode >= IORING_OP_LAST)) {
> +       if (unlikely(req->opcode >= IORING_OP_LAST)) {
>                 req->opcode = 0;
>                 return io_init_fail_req(req, -EINVAL);
>         }
> -       opcode = array_index_nospec(opcode, IORING_OP_LAST);
> +       req->opcode = array_index_nospec(req->opcode, IORING_OP_LAST);
>
> -       def = &io_issue_defs[opcode];
> +       def = &io_issue_defs[req->opcode];
>         if (def->is_128 && !(ctx->flags & IORING_SETUP_SQE128)) {
>                 /*
>                  * A 128b op on a non-128b SQ requires mixed SQE support as
> --
> 2.53.0
>