io_uring/io_uring.c | 1 + 1 file changed, 1 insertion(+)
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
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
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
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?
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
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
© 2016 - 2026 Red Hat, Inc.