BPF List
 help / color / mirror / Atom feed
* [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
@ 2026-05-15 14:58 Michael Bommarito
  2026-05-15 15:45 ` Keith Busch
  2026-05-15 15:46 ` Jens Axboe
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-05-15 14:58 UTC (permalink / raw)
  To: Jens Axboe, io-uring; +Cc: Pavel Begunkov, Li Zetao, bpf, linux-kernel

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
  2026-05-15 14:58 [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode Michael Bommarito
@ 2026-05-15 15:45 ` Keith Busch
  2026-05-15 15:47   ` Jens Axboe
  2026-05-15 15:46 ` Jens Axboe
  1 sibling, 1 reply; 4+ messages in thread
From: Keith Busch @ 2026-05-15 15:45 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Jens Axboe, io-uring, Pavel Begunkov, Li Zetao, bpf, linux-kernel

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?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
  2026-05-15 14:58 [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode Michael Bommarito
  2026-05-15 15:45 ` Keith Busch
@ 2026-05-15 15:46 ` Jens Axboe
  1 sibling, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2026-05-15 15:46 UTC (permalink / raw)
  To: Michael Bommarito, io-uring; +Cc: Pavel Begunkov, Li Zetao, bpf, linux-kernel

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode
  2026-05-15 15:45 ` Keith Busch
@ 2026-05-15 15:47   ` Jens Axboe
  0 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2026-05-15 15:47 UTC (permalink / raw)
  To: Keith Busch, Michael Bommarito
  Cc: io-uring, Pavel Begunkov, Li Zetao, bpf, linux-kernel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-15 15:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-15 14:58 [PATCH] io_uring: propagate array_index_nospec opcode into req->opcode Michael Bommarito
2026-05-15 15:45 ` Keith Busch
2026-05-15 15:47   ` Jens Axboe
2026-05-15 15:46 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox