* [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