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

Michael Bommarito posted 1 patch 1 week, 2 days ago
There is a newer version of this series
io_uring/io_uring.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Michael Bommarito 1 week, 2 days ago
Commit 1e988c3fe126 ("io_uring: prevent opcode speculation") added
array_index_nospec() to the local opcode in io_init_req(), but the
sanitised value is not written back to req->opcode.  The
unconditional write at the top of io_init_req() stores the raw byte
into the persistent field; the success path of the bounds check
leaves it unchanged, and downstream consumers read the raw value.

In io_uring/io_uring.c those consumers are io_issue_sqe(),
__io_issue_sqe(), io_wq_submit_work() and io_prep_async_work() (all
indexing io_issue_defs[]); io_clean_op(), io_req_defer_failed() and
io_req_sqe_copy() (io_cold_defs[]); io_check_restriction() via
test_bit() on ctx->restrictions.sqe_op; and the audit hook at the
io_issue_sqe entry.  io_uring/bpf_filter.c added in v7.0 extends
this set with io_uring_populate_bpf_ctx() and
__io_uring_run_bpf_filters(), indexing a heap-resident per-filter
pointer array sized at allocation to IORING_OP_LAST.

The kernel's spectre_v1 protection is per-site array_index_nospec()
annotation, so a site missing the annotation is unprotected
regardless of CPU vendor, microarchitecture, or microcode revision.
A per-site array_index_nospec() was applied to the same class of
gap in io_uring/fdinfo.c recently.  Propagating the clamped opcode
to req->opcode once, immediately after the existing
array_index_nospec(), closes the remaining sites at the source
without per-site clamps.

The compiled change is one instruction (a single mov of the clamped
byte to req->opcode); the cmp/sbb/and clamp triplet is unchanged.
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 this assignment.  Boot-tested under UML
(x86_64 defconfig) by building stock and patched kernels and running
a 54-test subset of liburing's test suite against each; pass/fail
identical on both.

Fixes: 1e988c3fe126 ("io_uring: prevent opcode speculation")
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 io_uring/io_uring.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 4ed998d60c09c..7b257a03ef84c 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1739,6 +1739,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 		return io_init_fail_req(req, -EINVAL);
 	}
 	opcode = array_index_nospec(opcode, IORING_OP_LAST);
+	req->opcode = opcode;
 
 	def = &io_issue_defs[opcode];
 	if (def->is_128 && !(ctx->flags & IORING_SETUP_SQE128)) {
-- 
2.53.0
Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Jens Axboe 1 week, 1 day ago
On Fri, 15 May 2026 10:58:11 -0400, Michael Bommarito wrote:
> Commit 1e988c3fe126 ("io_uring: prevent opcode speculation") added
> array_index_nospec() to the local opcode in io_init_req(), but the
> sanitised value is not written back to req->opcode.  The
> unconditional write at the top of io_init_req() stores the raw byte
> into the persistent field; the success path of the bounds check
> leaves it unchanged, and downstream consumers read the raw value.
> 
> [...]

Applied, thanks!

[1/1] io_uring: propagate array_index_nospec opcode into req->opcode
      (no commit info)

Best regards,
-- 
Jens Axboe
Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Jens Axboe 1 week, 1 day ago
On 5/16/26 1:05 PM, Jens Axboe wrote:
> 
> On Fri, 15 May 2026 10:58:11 -0400, Michael Bommarito wrote:
>> Commit 1e988c3fe126 ("io_uring: prevent opcode speculation") added
>> array_index_nospec() to the local opcode in io_init_req(), but the
>> sanitised value is not written back to req->opcode.  The
>> unconditional write at the top of io_init_req() stores the raw byte
>> into the persistent field; the success path of the bounds check
>> leaves it unchanged, and downstream consumers read the raw value.
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/1] io_uring: propagate array_index_nospec opcode into req->opcode
>       (no commit info)

Oops, was just applied for review, nothing has been applied. Awaiting
a v2 based on the feedback.

-- 
Jens Axboe
Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Keith Busch 1 week, 2 days ago
On Fri, May 15, 2026 at 10:58:11AM -0400, Michael Bommarito wrote:
> The compiled change is one instruction (a single mov of the clamped
> byte to req->opcode); the cmp/sbb/and clamp triplet is unchanged.
> 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 this assignment.  

Since the bounds check above already catches an invalid opcode, why does
it need to be re-initialized to the clamped value? Surely it's already
the same value if we've taken this path, no?
Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Jens Axboe 1 week, 2 days ago
On 5/15/26 9:45 AM, Keith Busch wrote:
> On Fri, May 15, 2026 at 10:58:11AM -0400, Michael Bommarito wrote:
>> The compiled change is one instruction (a single mov of the clamped
>> byte to req->opcode); the cmp/sbb/and clamp triplet is unchanged.
>> 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 this assignment.  
> 
> Since the bounds check above already catches an invalid opcode, why does
> it need to be re-initialized to the clamped value? Surely it's already
> the same value if we've taken this path, no?

It's to avoid speculation values being used. If the ->opcode store is
the last one, then it doesn't exist.

It's pretty narrow and mostly theoretical, but does make sense.

-- 
Jens Axboe
Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
Posted by Jens Axboe 1 week, 2 days ago
On 5/15/26 8:58 AM, Michael Bommarito wrote:
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 4ed998d60c09c..7b257a03ef84c 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -1739,6 +1739,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
>  		return io_init_fail_req(req, -EINVAL);
>  	}
>  	opcode = array_index_nospec(opcode, IORING_OP_LAST);
> +	req->opcode = opcode;
>  
>  	def = &io_issue_defs[opcode];
>  	if (def->is_128 && !(ctx->flags & IORING_SETUP_SQE128)) {

We could just kill 'opcode' while at it, and just use req->opcode for
this. I think that'd end up generating the same code, and avoid having
two versions of opcode.

-- 
Jens Axboe