diff mbox series

[SRU,F,v2,3/3] io_uring: ensure IOPOLL locks around deferred work

Message ID 20241126041441.199041-4-chengen.du@canonical.com
State New
Headers show
Series CVE-2023-21400 | expand

Commit Message

Chengen Du Nov. 26, 2024, 4:14 a.m. UTC 
From: Jens Axboe <axboe@kernel.dk>

CVE-2023-21400

BugLink: https://bugs.launchpad.net/bugs/2078659

No direct upstream commit exists for this issue. It was fixed in
5.18 as part of a larger rework of the completion side.

io_commit_cqring() writes the CQ ring tail to make it visible, but it
also kicks off any deferred work we have. A ring setup with IOPOLL
does not need any locking around the CQ ring updates, as we're always
under the ctx uring_lock. But if we have deferred work that needs
processing, then io_queue_deferred() assumes that the completion_lock
is held, as it is for !IOPOLL.

Add a lockdep assertion to check and document this fact, and have
io_iopoll_complete() check if we have deferred work and run that
separately with the appropriate lock grabbed.

Cc: stable@vger.kernel.org # 5.10, 5.15
Reported-by: dghost david <daviduniverse18@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(backported from commit 810e401b34c4c4c244d8b93b9947ea5b3d4d49f8 linux-5.10.y)
[chengen - introduce the __io_commit_cqring_flush function to separate 
the logic in io_commit_cqring / confine the completion_lock range.]
Signed-off-by: Chengen Du <chengen.du@canonical.com>
---
 fs/io_uring.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)
diff mbox series

Patch

diff --git a/fs/io_uring.c b/fs/io_uring.c
index ce809ea55c6d..3b93cbca8048 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -567,15 +567,15 @@  static void io_kill_timeouts(struct io_ring_ctx *ctx)
 	spin_unlock_irq(&ctx->completion_lock);
 }
 
-static void io_commit_cqring(struct io_ring_ctx *ctx)
+static void __io_commit_cqring_flush(struct io_ring_ctx *ctx)
 {
 	struct io_kiocb *req;
 
+	lockdep_assert_held(&ctx->completion_lock);
+
 	while ((req = io_get_timeout_req(ctx)) != NULL)
 		io_kill_timeout(req);
 
-	__io_commit_cqring(ctx);
-
 	while ((req = io_get_deferred_req(ctx)) != NULL) {
 		if (req->flags & REQ_F_SHADOW_DRAIN) {
 			/* Just for drain, free it. */
@@ -587,6 +587,12 @@  static void io_commit_cqring(struct io_ring_ctx *ctx)
 	}
 }
 
+static void io_commit_cqring(struct io_ring_ctx *ctx)
+{
+	__io_commit_cqring_flush(ctx);
+	__io_commit_cqring(ctx);
+}
+
 static struct io_uring_cqe *io_get_cqring(struct io_ring_ctx *ctx)
 {
 	struct io_rings *rings = ctx->rings;
@@ -846,7 +852,10 @@  static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 		}
 	}
 
-	io_commit_cqring(ctx);
+	spin_lock_irq(&ctx->completion_lock);
+	__io_commit_cqring_flush(ctx);
+	spin_unlock_irq(&ctx->completion_lock);
+	__io_commit_cqring(ctx);
 	io_cqring_ev_posted_iopoll(ctx);
 	io_free_req_many(ctx, reqs, &to_free);
 }