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

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

Posted by Michael Bommarito 1 week ago

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@gmail.com>
---
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);
 	/* 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

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

Posted by Jens Axboe 6 days, 10 hours ago

On Sun, 17 May 2026 17:30:10 -0400, Michael Bommarito 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.
> 
> [...]

Applied, thanks!

[1/1] io_uring: propagate array_index_nospec opcode into req->opcode
      commit: cf18e36455603d65d4745de83e2d1743c54ada47

Best regards,
-- 
Jens Axboe

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

Posted by Caleb Sander Mateos 6 days, 15 hours ago

On Sun, May 17, 2026 at 2:30 PM Michael Bommarito
<michael.bommarito@gmail.com> 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@gmail.com>
> ---
> 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
>

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

Posted by Jens Axboe 6 days, 15 hours ago

On 5/18/26 8:42 AM, Caleb Sander Mateos wrote:
> On Sun, May 17, 2026 at 2:30?PM Michael Bommarito
> <michael.bommarito@gmail.com> 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@gmail.com>
>> ---
>> 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

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

Posted by Caleb Sander Mateos 6 days, 15 hours ago

On Mon, May 18, 2026 at 7:49 AM Jens Axboe <axboe@kernel.dk> 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@gmail.com> 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@gmail.com>
> >> ---
> >> 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

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

Posted by Michael Bommarito 6 days, 15 hours ago

On Mon, May 18, 2026 at 10:43 AM Caleb Sander Mateos
<csander@purestorage.com> wrote:
> 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()?

I'd defer to you / Jens / Keith on this, but in case you haven't seen
it, I was just following Jens's request here:
https://lore.kernel.org/all/0f7e9184-f317-40f1-b366-d8582cb97ac4@kernel.dk/

Thanks,
Mike