public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [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, &params);
  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