io_uring: fix CQ tail over-commit and CQE32 index corruption in io_cqe_cache_refill()

io_cqe_cache_refill() has three issues when handling 32-byte CQEs on IORING_SETUP_CQE32 and IORING_SETUP_CQE_MIXED rings:

1. When cqe32 is true and off + 1 == ctx->cq_entries, io_cqe_cache_refill() posts a 16-byte dummy IORING_CQE_F_SKIP CQE via io_fill_nop_cqe(ctx, off) even on pure IORING_SETUP_CQE32 rings (where IORING_SETUP_CQE_MIXED is not set). On a pure CQE32 ring, ctx->cq_entries is the number of 32-byte CQEs and rings->cqes is indexed by (off << 1), so writing a 16-byte skip CQE at &rings->cqes[off] corrupts the middle of the CQ ring and misaligns all subsequent CQEs. Restrict the wrap-around skip CQE to IORING_SETUP_CQE_MIXED rings.

2. On an IORING_SETUP_CQE_MIXED ring, when off + 1 == ctx->cq_entries and only 1 free CQ slot remains (ctx->cq_entries - io_cqring_queued() == 1), io_fill_nop_cqe(ctx, off) consumes that last slot and increments ctx->cached_cq_tail, after which free == 0 causes io_cqe_cache_refill() to return false without updating ctx->cqe_cached. Check that at least 2 free CQ slots exist before posting the dummy skip CQE.

3. On pure IORING_SETUP_CQE32 rings, len is in 32-byte CQE units prior to len <<= 1, so len < (cqe32 + 1) falsely requires 2 free 32-byte CQEs instead of 1. Check !len before scaling off and len on IORING_SETUP_CQE32 rings, and in io_fill_cqe_aux() zero cqe->big_cqe[0..1] whenever IORING_SETUP_CQE32 is set on the ring.

---

@@ -733,7 +733,10 @@ bool io_cqe_cache_refill(struct io_ring_ctx *ctx, bool overflow, bool cqe32)
 	 * Post dummy CQE if a 32b CQE is needed and there's only room for a
 	 * 16b CQE before the ring wraps.
 	 */
-	if (cqe32 && off + 1 == ctx->cq_entries) {
+	if (cqe32 && (ctx->flags & IORING_SETUP_CQE_MIXED) &&
+	    off + 1 == ctx->cq_entries) {
+		if (ctx->cq_entries - io_cqring_queued(ctx) < 2)
+			return false;
 		if (!io_fill_nop_cqe(ctx, off))
 			return false;
 		off = 0;

@@ -742,12 +745,13 @@ bool io_cqe_cache_refill(struct io_ring_ctx *ctx, bool overflow, bool cqe32)
 	free = ctx->cq_entries - io_cqring_queued(ctx);
 	/* we need a contiguous range, limit based on the current array offset */
 	len = min(free, ctx->cq_entries - off);
-	if (len < (cqe32 + 1))
-		return false;
-
 	if (ctx->flags & IORING_SETUP_CQE32) {
+		if (!len)
+			return false;
 		off <<= 1;
 		len <<= 1;
+	} else if (len < (cqe32 + 1)) {
+		return false;
 	}
 
 	ctx->cqe_cached = &rings->cqes[off];

@@ -781,7 +785,7 @@ static bool io_fill_cqe_aux(struct io_ring_ctx *ctx, u64 user_data, s32 res,
 		WRITE_ONCE(cqe->res, res);
 		WRITE_ONCE(cqe->flags, cflags);
 
-		if (cqe32) {
+		if (cqe32 || (ctx->flags & IORING_SETUP_CQE32)) {
 			WRITE_ONCE(cqe->big_cqe[0], 0);
 			WRITE_ONCE(cqe->big_cqe[1], 0);
 		}

2.43.0

Don't combine fixes, send 3 separate patches.

These two look fine.

This part looks wrong.

Jens Axboe

This series fixes two issues in io_cqe_cache_refill() and io_fill_nop_cqe() for 32-byte CQEs:

1. Restrict skip CQE insertion at off + 1 == ctx->cq_entries to IORING_SETUP_CQE_MIXED rings so pure IORING_SETUP_CQE32 rings do not emit bogus IORING_CQE_F_SKIP entries.
2. Require 3 free CQ slots in io_fill_nop_cqe() before emitting a skip CQE for a 32-byte CQE wrapping across the end of an IORING_SETUP_CQE_MIXED ring, preventing an orphan skip CQE when fewer than 3 slots are free.

Changes in v2:
- Split into a 2-patch series and drop the third hunk from v1, as requested by Jens Axboe.

Hui Peng (2):
io_uring: only insert skip CQE for IORING_SETUP_CQE_MIXED
io_uring: require 3 free CQ slots for 32b CQE in io_fill_nop_cqe()

 io_uring/io_uring.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

2.55.0.1082.g2b9226bbc0-goog

In io_cqe_cache_refill(), when cqe32 is set and off + 1 == ctx->cq_entries, a dummy CQE with IORING_CQE_F_SKIP is written at the last slot of the CQ ring and cached_cq_tail is incremented so that a 32-byte CQE in a mixed ring (IORING_SETUP_CQE_MIXED) does not wrap across the end of the 16-byte slot array.

However, on a pure IORING_SETUP_CQE32 ring (where every ring entry is already 32 bytes wide and indexed by << 1), inserting a skip CQE and incrementing cached_cq_tail writes IORING_CQE_F_SKIP into the middle of rings->cqes and advances cached_cq_tail by an extra slot.

Restrict the skip CQE insertion in io_cqe_cache_refill() to rings with IORING_SETUP_CQE_MIXED set.

Tested in QEMU against Linux 7.3.0-rc3 on a 4-entry pure IORING_SETUP_CQE32 ring (where every entry is 32 bytes wide): on the unfixed kernel, when posting a 32-byte CQE at off + 1 == 4, io_cqe_cache_refill() wrote a bogus skip CQE at slot 3 and incremented cached_cq_tail by an extra slot, corrupting the ring index sequence; whereas with this fix applied, skip CQE insertion is skipped on pure IORING_SETUP_CQE32 rings, preserving exact 32-byte alignment and sequence order.

---
Changes in v2:
- Split out as patch 1/2 as requested by Jens Axboe.
- Added testing details in QEMU on pure IORING_SETUP_CQE32 rings.

 io_uring/io_uring.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 61053421d809..ae7c77158c58 100644
@@ -733,7 +733,8 @@ bool io_cqe_cache_refill(struct io_ring_ctx *ctx, bool overflow, bool cqe32)
 	 * Post dummy CQE if a 32b CQE is needed and there's only room for a
 	 * 16b CQE before the ring wraps.
 	 */
-	if (cqe32 && off + 1 == ctx->cq_entries) {
+	if (cqe32 && (ctx->flags & IORING_SETUP_CQE_MIXED) &&
+	    off + 1 == ctx->cq_entries) {
 		if (!io_fill_nop_cqe(ctx, off))
 			return false;
 		off = 0;

2.55.0.1082.g2b9226bbc0-goog

When io_cqe_cache_refill() is called for a 32-byte CQE at the last slot of an IORING_SETUP_CQE_MIXED ring (off + 1 == ctx->cq_entries), io_fill_nop_cqe() writes an IORING_CQE_F_SKIP CQE at the last slot and increments cached_cq_tail, consuming 1 free slot, after which the 32-byte CQE itself requires 2 more contiguous free slots at index 0.

Currently, io_fill_nop_cqe() only checks io_cqring_queued(ctx) < ctx->cq_entries (free >= 1). When free is 1 or 2, io_fill_nop_cqe() succeeds and increments cached_cq_tail for the skip CQE, and then io_cqe_cache_refill() computes len = min(free, ctx->cq_entries - off) < 2 and returns false because len < (cqe32 + 1). This leaves an orphan skip CQE in the ring with cached_cq_tail already incremented while the 32-byte CQE is deferred to cq_overflow_list.

Require at least 3 free slots (io_cqring_queued(ctx) + 3 <= ctx->cq_entries) in io_fill_nop_cqe() before emitting the skip CQE.

Tested in QEMU against Linux 7.3.0-rc3 on a 4-entry IORING_SETUP_CQE_MIXED ring with 2 queued CQEs (head = 1, tail = 3, free = 2, off = 3) followed by a 32-byte NOP (IORING_NOP_CQE32): on the unfixed kernel cq_tail advances to 4 due to the orphan skip CQE while the 32-byte CQE overflows, whereas with the fix applied cq_tail remains at 3 and both slots remain consistent.

---
Changes in v2:
- Split out as patch 2/2 as requested by Jens Axboe.

 io_uring/io_uring.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

index ae7c77158c58..b430301fead9 100644
@@ -698,7 +698,12 @@ static unsigned int io_cqring_queued(struct io_ring_ctx *ctx)
  */
 static bool io_fill_nop_cqe(struct io_ring_ctx *ctx, unsigned int off)
 {
-	if (io_cqring_queued(ctx) < ctx->cq_entries) {
+	/*
+	 * Creating a skip CQE and posting a 32b CQE requires 3 free CQ slots
+	 * in total (1 for the skip CQE at the end of the ring and 2 for the
+	 * 32b CQE at the start of the ring).
+	 */
+	if (io_cqring_queued(ctx) + 3 <= ctx->cq_entries) {
 		struct io_uring_cqe *cqe = &ctx->rings->cqes[off];
 
 		cqe->user_data = 0;

2.55.0.1082.g2b9226bbc0-goog

Sorry maybe my reply got lost, but I did do this work for you and split it into a 7.3 and 7.4 set. See my branches here:

https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git/log/?h=for-7.4/io_uring

https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git/log/?h=io_uring-7.3

Please take a look.

Jens Axboe