* [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot
@ 2026-09-11 15:34 Jann Horn
2026-09-11 15:37 ` Jann Horn
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jann Horn @ 2026-09-11 15:34 UTC (permalink / raw)
To: Jens Axboe
Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel,
Jann Horn
A cqe32 entry spans two CQ array slots, so the last CQ array slot can't
contain a cqe32 entry. If the CQ tail points at the last CQ array slot and
the kernel wants to write a cqe32 entry, it uses io_fill_nop_cqe() to pad
the last CQ array slot with a dummy entry and make the tail wrap around.
However, malicious userspace can directly set IORING_CQE_F_32 on the last
CQ array slot, causing __io_uring_show_fdinfo() to read the second cqe32
half from beyond the CQ array. Change __io_uring_show_fdinfo() to
explicitly ignore the IORING_CQE_F_32 flag in this case.
This is not a real bugfix, just tightening the code a bit, because:
1. the number of CQE slots is always a power of 2, see io_uring_fill_params
2. the ring_region region consists of:
- a 64-byte header
- pow(2, N) CQE slots (each 0x10 bytes)
- optionally, the SQ array
3. the ring_region size must be page-aligned because it is shared memory
Together, these properties imply that the last CQE slot can't be close
before the end of a page, so the "out-of-bounds" data is
Reported-by: Dominik Maier <dmnk+artist@google.com>
Fixes: 82ceb7fcc5ff ("io_uring/fdinfo: handle mixed sized CQEs")
Cc: stable+noautosel@kernel.org # no impact due to memory layout
Signed-off-by: Jann Horn <jannh@google.com>
---
io_uring/fdinfo.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c
index 3ae4804765b9..0b656a6c8428 100644
--- a/io_uring/fdinfo.c
+++ b/io_uring/fdinfo.c
@@ -155,9 +155,11 @@ static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m)
for (i = 0; i < cq_entries; i++) {
struct io_uring_cqe *cqe;
bool cqe32 = false;
+ bool is_last_cqarray_slot = (cq_head == cq_mask);
cqe = &r->cqes[(cq_head & cq_mask)];
- if (cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32)
+ if ((cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32) &&
+ !is_last_cqarray_slot)
cqe32 = true;
seq_printf(m, "%5u: user_data:%llu, res:%d, flags:%x",
cq_head & cq_mask, cqe->user_data, cqe->res,
---
base-commit: 50d05c7c76c96b90462f24debacca971d2e86713
change-id: 20260910-uring-fdinfo-tighten-1b1c18945305
Best regards,
--
Jann Horn <jannh@google.com>
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:34 [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot Jann Horn @ 2026-09-11 15:37 ` Jann Horn 2026-09-11 15:44 ` Jann Horn 2026-09-11 16:33 ` Gabriel Krisman Bertazi 2 siblings, 0 replies; 7+ messages in thread From: Jann Horn @ 2026-09-11 15:37 UTC (permalink / raw) To: Jens Axboe; +Cc: io-uring, linux-kernel, Dominik Maier On Fri, Sep 11, 2026 at 5:35 PM Jann Horn <jannh@google.com> wrote: > A cqe32 entry spans two CQ array slots, so the last CQ array slot can't > contain a cqe32 entry. If the CQ tail points at the last CQ array slot and > the kernel wants to write a cqe32 entry, it uses io_fill_nop_cqe() to pad > the last CQ array slot with a dummy entry and make the tail wrap around. > > However, malicious userspace can directly set IORING_CQE_F_32 on the last > CQ array slot, causing __io_uring_show_fdinfo() to read the second cqe32 > half from beyond the CQ array. Change __io_uring_show_fdinfo() to > explicitly ignore the IORING_CQE_F_32 flag in this case. Here is the testcase I used to experiment with this: #define _GNU_SOURCE #include <stdio.h> #include <stdlib.h> #include <string.h> #include <unistd.h> #include <sys/mman.h> #include <sys/syscall.h> #include <linux/io_uring.h> int main(void) { const int cq_entries = 16; struct io_uring_params params = { .cq_entries = cq_entries, .flags = IORING_SETUP_NO_SQARRAY|IORING_SETUP_CQSIZE }; int uring_fd = syscall(__NR_io_uring_setup, /*entries=*/1, ¶ms); char *mmap_region = mmap(NULL, 0x1000, PROT_READ|PROT_WRITE, MAP_SHARED, uring_fd, IORING_OFF_SQ_RING); *(unsigned int *)(mmap_region + params.cq_off.tail) = cq_entries; struct io_uring_cqe *cqes = (struct io_uring_cqe *)(mmap_region + params.cq_off.cqes); for (int i=1; i<cq_entries; i+=2) cqes[i].flags = IORING_CQE_F_32; char cmd[1000]; sprintf(cmd, "cat /proc/$PPID/fdinfo/%d", uring_fd); system(cmd); } With the fix applied, it prints the CQEs as follows (note that the last line has flags 0x8000 but no extra1/extra2): 0: user_data:0, res:0, flags:0 1: user_data:0, res:0, flags:8000, extra1:0, extra2:0 3: user_data:0, res:0, flags:8000, extra1:0, extra2:0 5: user_data:0, res:0, flags:8000, extra1:0, extra2:0 7: user_data:0, res:0, flags:8000, extra1:0, extra2:0 9: user_data:0, res:0, flags:8000, extra1:0, extra2:0 11: user_data:0, res:0, flags:8000, extra1:0, extra2:0 13: user_data:0, res:0, flags:8000, extra1:0, extra2:0 15: user_data:0, res:0, flags:8000 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:34 [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot Jann Horn 2026-09-11 15:37 ` Jann Horn @ 2026-09-11 15:44 ` Jann Horn 2026-09-11 15:50 ` Jens Axboe 2026-09-11 16:33 ` Gabriel Krisman Bertazi 2 siblings, 1 reply; 7+ messages in thread From: Jann Horn @ 2026-09-11 15:44 UTC (permalink / raw) To: Jens Axboe; +Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel On Fri, Sep 11, 2026 at 5:35 PM Jann Horn <jannh@google.com> wrote: > A cqe32 entry spans two CQ array slots, so the last CQ array slot can't > contain a cqe32 entry. If the CQ tail points at the last CQ array slot and > the kernel wants to write a cqe32 entry, it uses io_fill_nop_cqe() to pad > the last CQ array slot with a dummy entry and make the tail wrap around. > > However, malicious userspace can directly set IORING_CQE_F_32 on the last > CQ array slot, causing __io_uring_show_fdinfo() to read the second cqe32 > half from beyond the CQ array. Change __io_uring_show_fdinfo() to > explicitly ignore the IORING_CQE_F_32 flag in this case. > > This is not a real bugfix, just tightening the code a bit, because: > > 1. the number of CQE slots is always a power of 2, see io_uring_fill_params > 2. the ring_region region consists of: > - a 64-byte header > - pow(2, N) CQE slots (each 0x10 bytes) > - optionally, the SQ array > 3. the ring_region size must be page-aligned because it is shared memory > > Together, these properties imply that the last CQE slot can't be close > before the end of a page, so the "out-of-bounds" data is Oops, sorry, somehow I forgot to complete that sentence, that was supposed to be: Together, these properties imply that the last CQE slot can't be close before the end of a page, so the "out-of-bounds" data is in memory that is anyway accessible to userspace. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:44 ` Jann Horn @ 2026-09-11 15:50 ` Jens Axboe 2026-09-11 15:52 ` Jann Horn 0 siblings, 1 reply; 7+ messages in thread From: Jens Axboe @ 2026-09-11 15:50 UTC (permalink / raw) To: Jann Horn; +Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel On 9/11/26 9:44 AM, Jann Horn wrote: > On Fri, Sep 11, 2026 at 5:35?PM Jann Horn <jannh@google.com> wrote: >> A cqe32 entry spans two CQ array slots, so the last CQ array slot can't >> contain a cqe32 entry. If the CQ tail points at the last CQ array slot and >> the kernel wants to write a cqe32 entry, it uses io_fill_nop_cqe() to pad >> the last CQ array slot with a dummy entry and make the tail wrap around. >> >> However, malicious userspace can directly set IORING_CQE_F_32 on the last >> CQ array slot, causing __io_uring_show_fdinfo() to read the second cqe32 >> half from beyond the CQ array. Change __io_uring_show_fdinfo() to >> explicitly ignore the IORING_CQE_F_32 flag in this case. >> >> This is not a real bugfix, just tightening the code a bit, because: >> >> 1. the number of CQE slots is always a power of 2, see io_uring_fill_params >> 2. the ring_region region consists of: >> - a 64-byte header >> - pow(2, N) CQE slots (each 0x10 bytes) >> - optionally, the SQ array >> 3. the ring_region size must be page-aligned because it is shared memory >> >> Together, these properties imply that the last CQE slot can't be close >> before the end of a page, so the "out-of-bounds" data is > > Oops, sorry, somehow I forgot to complete that sentence, that was > supposed to be: > > Together, these properties imply that the last CQE slot can't be close > before the end of a page, so the "out-of-bounds" data is in memory > that is anyway accessible to userspace. I did spot that as well, thanks for finishing it. Your fdinfo idea keeps on giving, at least this one doesn't really matter :-) -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:50 ` Jens Axboe @ 2026-09-11 15:52 ` Jann Horn 2026-09-11 15:55 ` Jens Axboe 0 siblings, 1 reply; 7+ messages in thread From: Jann Horn @ 2026-09-11 15:52 UTC (permalink / raw) To: Jens Axboe; +Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel On Fri, Sep 11, 2026 at 5:50 PM Jens Axboe <axboe@kernel.dk> wrote: > I did spot that as well, thanks for finishing it. Your fdinfo idea keeps > on giving, at least this one doesn't really matter :-) Ugh, yes. I hope it is at least somewhat useful for debugging, and doesn't _just_ cause lots of bugs? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:52 ` Jann Horn @ 2026-09-11 15:55 ` Jens Axboe 0 siblings, 0 replies; 7+ messages in thread From: Jens Axboe @ 2026-09-11 15:55 UTC (permalink / raw) To: Jann Horn; +Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel On 9/11/26 9:52 AM, Jann Horn wrote: > On Fri, Sep 11, 2026 at 5:50?PM Jens Axboe <axboe@kernel.dk> wrote: >> I did spot that as well, thanks for finishing it. Your fdinfo idea keeps >> on giving, at least this one doesn't really matter :-) > > Ugh, yes. I hope it is at least somewhat useful for debugging, and > doesn't _just_ cause lots of bugs? Lots of bugs was earlier, and to be fair this one is caused by the more recent addition of the mixed size CQEs. Everything is under the main lock these days anyway, so it's more logic bugs like this one rather than races, etc. I just like blaming you (a little bit), but in practice it's not your fault. -- Jens Axboe ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot 2026-09-11 15:34 [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot Jann Horn 2026-09-11 15:37 ` Jann Horn 2026-09-11 15:44 ` Jann Horn @ 2026-09-11 16:33 ` Gabriel Krisman Bertazi 2 siblings, 0 replies; 7+ messages in thread From: Gabriel Krisman Bertazi @ 2026-09-11 16:33 UTC (permalink / raw) To: Jann Horn, Jens Axboe Cc: io-uring, linux-kernel, Dominik Maier, stable+noautosel, Jann Horn Jann Horn <jannh@google.com> writes: > > diff --git a/io_uring/fdinfo.c b/io_uring/fdinfo.c > index 3ae4804765b9..0b656a6c8428 100644 > --- a/io_uring/fdinfo.c > +++ b/io_uring/fdinfo.c > @@ -155,9 +155,11 @@ static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m) > for (i = 0; i < cq_entries; i++) { > struct io_uring_cqe *cqe; > bool cqe32 = false; > + bool is_last_cqarray_slot = (cq_head == cq_mask); > > cqe = &r->cqes[(cq_head & cq_mask)]; > - if (cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32) > + if ((cqe->flags & IORING_CQE_F_32 || ctx->flags & IORING_SETUP_CQE32) && > + !is_last_cqarray_slot) Could we have a small comment here explaining we can't just trust IORING_CQE_F_32 for the last slot? it is very non-obvious to me from the code. Something as simple as (if I got it right): /* userspace can manipulate IORING_CQE_F_32 which is usually harmless, * except in the last CQ slot. Ignore the flag only for that slot to avoid * reading memory out-of-bounds. */ Either way, Reviewed-by: Gabriel Krisman Bertazi <krisman@suse.de> Thanks, > cqe32 = true; > seq_printf(m, "%5u: user_data:%llu, res:%d, flags:%x", > cq_head & cq_mask, cqe->user_data, cqe->res, > > --- > base-commit: 50d05c7c76c96b90462f24debacca971d2e86713 > change-id: 20260910-uring-fdinfo-tighten-1b1c18945305 > > Best regards, > -- > Jann Horn <jannh@google.com> > -- Gabriel Krisman Bertazi ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-09-11 16:33 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-11 15:34 [PATCH] io_uring/fdinfo: ignore IORING_CQE_F_32 in last CQ array slot Jann Horn 2026-09-11 15:37 ` Jann Horn 2026-09-11 15:44 ` Jann Horn 2026-09-11 15:50 ` Jens Axboe 2026-09-11 15:52 ` Jann Horn 2026-09-11 15:55 ` Jens Axboe 2026-09-11 16:33 ` Gabriel Krisman Bertazi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox