Skip to contentSkip to conversation

io_uring: OOB read in SQE_MIXED mode via sq_array physical index bypass

PATCH Applied Tom Ryan 4 participants 8 messages Mar 9 – Mar 10 io_uring

lore
Share & tools
lore

Tom Ryan

author
#0 lore raw

Hi All,

Patch attached.

Thanks,
Tom

From 4ba67b00d176e9f0ddff8fc80d5c28051d580f8b Mon Sep 17 00:00:00 2001
From: Tom Ryan <[email protected]>
Date: Mon, 9 Mar 2026 09:14:59 -0700
Subject: [PATCH] io_uring: validate physical SQE index for SQE_MIXED 128-byte
ops

When IORING_SETUP_SQE_MIXED is used with sq_array (the default, without IORING_SETUP_NO_SQARRAY), the boundary check for 128-byte SQE operations in io_init_req() validates the logical SQ head position but not the physical index obtained from sq_array.

Since sq_array allows user-controlled remapping of logical to physical SQE indices, an unprivileged user can set sq_array[N] = sq_entries - 1, placing a 128-byte operation at the last physical SQE slot. The subsequent 128-byte memcpy in io_uring_cmd_sqe_copy() then reads 64 bytes past the end of the SQE array.

Fix this by checking that the physical SQE index (derived from the sqe pointer) has room for the full 128-byte read, i.e., is not the last entry in the array.

Fixes 1cba30bf9fdd ("io_uring: add support for IORING_SETUP_SQE_MIXED")
Signed-off-by Tom Ryan <[email protected]>
---
 io_uring/io_uring.c | 3 +++
 1 file changed, 3 insertions(+)
io_uring/io_uring.c +3 -0
index aa9570316..2fa72d5e5 100644
@@ -1747,6 +1747,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 		if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
 		    !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
 			return io_init_fail_req(req, -EINVAL);
+		/* Validate physical SQE index has room for 128-byte read */
+		if ((unsigned)(sqe - ctx->sq_sqes) >= ctx->sq_entries - 1)
+			return io_init_fail_req(req, -EINVAL);
 		/*
 		 * A 128b operation on a mixed SQ uses two entries, so we have
 		 * to increment the head and cached refs, and decrement what's
Signature · 1 line
2.50.1 (Apple Git-155)
#1 lore raw
Quoted 1 line · Tom Ryan

Patch attached.

You can just submit the patch as text in the mail message.

io_uring/io_uring.c static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, · line 1752

Quoted 7 lines
io_uring/io_uring.c +3 -0
@@ -1747,6 +1747,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 		if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
 		    !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
 			return io_init_fail_req(req, -EINVAL);
+		/* Validate physical SQE index has room for 128-byte read */
+		if ((unsigned)(sqe - ctx->sq_sqes) >= ctx->sq_entries - 1)
+			return io_init_fail_req(req, -EINVAL);

Isn't this new check redundant with the "left < 2" check preceding it?

#2 lore raw
Quoted 6 lines · Keith Busch
Quoted 1 line · Tom Ryan

Patch attached.

You can just submit the patch as text in the mail message.

Quoted 9 lines

io_uring/io_uring.c static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, · line 1752

Quoted 7 lines
io_uring/io_uring.c +3 -0
@@ -1747,6 +1747,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
              if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
                  !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
                      return io_init_fail_req(req, -EINVAL);
+             /* Validate physical SQE index has room for 128-byte read */
+             if ((unsigned)(sqe - ctx->sq_sqes) >= ctx->sq_entries - 1)
+                     return io_init_fail_req(req, -EINVAL);

Isn't this new check redundant with the "left < 2" check preceding it?

I think it's orthogonal with *left < 2. How many SQEs are remaining to submit is unrelated to the index of each SQE. It is, however, redundant with !(ctx->cached_sq_head & (ctx->sq_entries - 1)), but only in the IORING_SETUP_NO_SQARRAY case. For non-IORING_SETUP_NO_SQARRAY rings, the SQ indirection array entry can point to the last entry of the SQE array, causing the big SQE to extend past the end. Probably, this added condition can replace !(ctx->cached_sq_head & (ctx->sq_entries - 1)). That checks whether this is the last entry *in the SQ indirection array*, but it should be checking the SQE array.

Best,
Caleb

#3 lore raw
Quoted 17 lines · Caleb Sander Mateos
Quoted 6 lines · Keith Busch
Quoted 1 line · Tom Ryan

Patch attached.

You can just submit the patch as text in the mail message.

Quoted 9 lines

io_uring/io_uring.c static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, · line 1752

Quoted 7 lines
io_uring/io_uring.c +3 -0
@@ -1747,6 +1747,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
              if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
                  !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
                      return io_init_fail_req(req, -EINVAL);
+             /* Validate physical SQE index has room for 128-byte read */
+             if ((unsigned)(sqe - ctx->sq_sqes) >= ctx->sq_entries - 1)
+                     return io_init_fail_req(req, -EINVAL);

Isn't this new check redundant with the "left < 2" check preceding it?

Quoted 10 lines

I think it's orthogonal with *left < 2. How many SQEs are remaining to submit is unrelated to the index of each SQE. It is, however, redundant with !(ctx->cached_sq_head & (ctx->sq_entries - 1)), but only in the IORING_SETUP_NO_SQARRAY case. For non-IORING_SETUP_NO_SQARRAY rings, the SQ indirection array entry can point to the last entry of the SQE array, causing the big SQE to extend past the end. Probably, this added condition can replace !(ctx->cached_sq_head & (ctx->sq_entries - 1)). That checks whether this is the last entry *in the SQ indirection array*, but it should be checking the SQE array.

Oh, right. The left < 2 was to confirm we have contiguous entries for a big sqe, but you could index to an unaligned end with the sqarray.

Folding this into the previous 'if' sounds good. And please consider an addition to liburing tests.

author
#4 lore raw

PATCH v2 1/1 io_uring: fix physical SQE bounds check for SQE_MIXED 128-byte ops

When IORING_SETUP_SQE_MIXED is used without IORING_SETUP_NO_SQARRAY, the boundary check for 128-byte SQE operations in io_init_req() validated the logical SQ head position rather than the physical SQE index.

The existing check:

!(ctx->cached_sq_head & (ctx->sq_entries - 1))

ensures the logical position isn't at the end of the ring, which is correct for NO_SQARRAY rings where physical == logical. However, when sq_array is present, an unprivileged user can remap any logical position to an arbitrary physical index via sq_array. Setting sq_array[N] = sq_entries - 1 places a 128-byte operation at the last physical SQE slot, causing the 128-byte memcpy in io_uring_cmd_sqe_copy() to read 64 bytes past the end of the SQE array.

Replace the cached_sq_head alignment check with a direct validation of the physical SQE index, which correctly handles both sq_array and NO_SQARRAY cases.

Fixes 1cba30bf9fdd ("io_uring: add support for IORING_SETUP_SQE_MIXED")
Signed-off-by Tom Ryan <[email protected]>

---
v1 -> v2:
- Replace the cached_sq_head alignment check rather than adding a separate check, per Caleb Sander Mateos' observation that the new physical index validation subsumes the old logical check for both sq_array and NO_SQARRAY cases
- Fold into existing conditional per Keith Busch
- liburing test sent separately

 io_uring/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
io_uring/io_uring.c +1 -1
index aa9570316..d9a307384 100644
@@ -1745,7 +1745,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req,
 		 * well as 2 contiguous entries.
 		 */
 		if (!(ctx->flags & IORING_SETUP_SQE_MIXED) || *left < 2 ||
-		    !(ctx->cached_sq_head & (ctx->sq_entries - 1)))
+		    (unsigned)(sqe - ctx->sq_sqes) >= ctx->sq_entries - 1)
 			return io_init_fail_req(req, -EINVAL);
 		/*
 		 * A 128b operation on a mixed SQ uses two entries, so we have
Signature · 1 line
2.50.1 (Apple Git-155)
author
#5 lore raw

PATCH 1/1 test/sqe-mixed-boundary: validate physical SQE index for 128-byte ops

Add a test for the kernel fix that replaces the cached_sq_head alignment check with physical SQE index validation in io_init_req() for SQE_MIXED 128-byte operations.

test_valid_position: verifies that a NOP128 at a valid physical slot (identity-mapped via sq_array) succeeds.

test_oob_boundary: verifies that a NOP128 remapped via sq_array to the last physical SQE slot is rejected with -EINVAL, preventing a 64-byte OOB read past the SQE array.

Signed-off-by Tom Ryan <[email protected]>
---
 test/Makefile             |   1 +
 test/sqe-mixed-boundary.c | 182 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)
 create mode 100644 test/sqe-mixed-boundary.c
test/Makefile +1 -0
index 7b94a1f..a10d44c 100644
@@ -253,6 +253,7 @@ test_srcs := \
 	sq-poll-share.c \
 	sqpoll-sleep.c \
 	sq-space_left.c \
+	sqe-mixed-boundary.c \
 	sqe-mixed-nop.c \
 	sqe-mixed-bad-wrap.c \
 	sqe-mixed-uring_cmd.c \
test/sqe-mixed-boundary.c new file +182 -0

182 lines not on this page · raw message

Signature · 1 line
2.50.1 (Apple Git-155)
maintainer
#6 lore raw

Re: [PATCH liburing] test/sqe-mixed-boundary: validate physical SQE index for 128-byte ops

Quoted 10 lines · Tom Ryan
test/sqe-mixed-boundary.c new file +10 -0
+/*
+ * Negative test: NOP128 at the last physical SQE slot via sq_array remap
+ * must be rejected. Without the kernel fix, this triggers a 64-byte OOB
+ * read in io_uring_cmd_sqe_copy().
+ */
+static int test_oob_boundary(void)
+{
+	struct io_uring ring;
+	struct io_uring_cqe *cqe;
+	struct io_uring_sqe *sqe;

test/sqe-mixed-boundary.c line 100

Quoted 10 lines
test/sqe-mixed-boundary.c new file +10 -0
+	unsigned mask;
+	int ret, i, found;
+
+	ret = io_uring_queue_init(NENTRIES, &ring, IORING_SETUP_SQE_MIXED);
+	if (ret) {
+		if (ret == -EINVAL)
+			return T_EXIT_SKIP;
+		fprintf(stderr, "ring init: %d\n", ret);
+		return T_EXIT_FAIL;
+	}

I don't think this will work, because this function requires the sqe redirection array and liburing will wrap the above in SETUP_NO_SQARRAY. Is this some llm written test case, or conversion of a raw use case? Did you actually try and run the test case?

You can certainly make it work, you'd have to use __io_uring_queue_init_params() to accomplish the setting up of the ring without IORING_SETUP_NO_SQARRAY.

Quoted 7 lines
test/sqe-mixed-boundary.c new file +7 -0
+	found = 0;
+	for (i = 0; i < 3; i++) {
+		ret = io_uring_wait_cqe(&ring, &cqe);
+		if (ret)
+			break;
+		if (cqe->user_data == 2) {
+			if (cqe->res != -EINVAL) {

test/sqe-mixed-boundary.c line 146

Quoted 10 lines
test/sqe-mixed-boundary.c new file +10 -0
+				fprintf(stderr,
+					"NOP128 at last slot: expected -EINVAL, got %d\n",
+					cqe->res);
+				io_uring_cqe_seen(&ring, cqe);
+				goto fail;
+			}
+			found = 1;
+		}
+		io_uring_cqe_seen(&ring, cqe);
+	}

This one puzzles me too - you submit 2 SQEs, yet you wait for 3. This will just sit forever until killed by the test suite timeout.

Signature · 1 line
Jens Axboe
maintainer Applied
#7 lore raw

Re: [PATCH v2] io_uring: fix physical SQE bounds check for SQE_MIXED 128-byte ops

In reply to Tom Ryan
Quoted 8 lines · Tom Ryan

When IORING_SETUP_SQE_MIXED is used without IORING_SETUP_NO_SQARRAY, the boundary check for 128-byte SQE operations in io_init_req() validated the logical SQ head position rather than the physical SQE index.

The existing check:

[...]

Applied, thanks!

[1/1] io_uring: fix physical SQE bounds check for SQE_MIXED 128-byte ops

      commit: c76e0f1d77f87e258193c2628253782d5ff414c7

Best regards,

Signature · 1 line
Jens Axboe