diff mbox series

[SRU,F,v2,2/3] io_uring: dont kill fasync under completion_lock

Message ID 20241126041441.199041-3-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: Pavel Begunkov <asml.silence@gmail.com>

CVE-2023-21400

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

      CPU0                    CPU1
       ----                    ----
  lock(&new->fa_lock);
                               local_irq_disable();
                               lock(&ctx->completion_lock);
                               lock(&new->fa_lock);
  <Interrupt>
    lock(&ctx->completion_lock);

 *** DEADLOCK ***

Move kill_fasync() out of io_commit_cqring() to io_cqring_ev_posted(),
so it doesn't hold completion_lock while doing it. That saves from the
reported deadlock, and it's just nice to shorten the locking time and
untangle nested locks (compl_lock -> wq_head::lock).

Reported-by: syzbot+91ca3f25bd7f795f019c@syzkaller.appspotmail.com
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
(backported from commit 4aa84f2ffa81f71e15e5cffc2cc6090dbee78f8e)
[chengen - introduce the io_cqring_ev_posted_iopoll function to handle
the kill_fasync logic in the io_iopoll_complete function.]
Signed-off-by: Chengen Du <chengen.du@canonical.com>
---
 fs/io_uring.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/fs/io_uring.c b/fs/io_uring.c
index f90b159bfc3a..ce809ea55c6d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -514,11 +514,6 @@  static void __io_commit_cqring(struct io_ring_ctx *ctx)
 
 	/* order cqe stores with ring update */
 	smp_store_release(&rings->cq.tail, ctx->cached_cq_tail);
-
-	if (wq_has_sleeper(&ctx->cq_wait)) {
-		wake_up_interruptible(&ctx->cq_wait);
-		kill_fasync(&ctx->cq_fasync, SIGIO, POLL_IN);
-	}
 }
 
 static inline void io_queue_async_work(struct io_ring_ctx *ctx,
@@ -639,6 +634,18 @@  static void io_cqring_ev_posted(struct io_ring_ctx *ctx)
 		wake_up(&ctx->sqo_wait);
 	if (ctx->cq_ev_fd)
 		eventfd_signal(ctx->cq_ev_fd, 1);
+	if (wq_has_sleeper(&ctx->cq_wait)) {
+		wake_up_interruptible(&ctx->cq_wait);
+		kill_fasync(&ctx->cq_fasync, SIGIO, POLL_IN);
+	}
+}
+
+static void io_cqring_ev_posted_iopoll(struct io_ring_ctx *ctx)
+{
+	if (wq_has_sleeper(&ctx->cq_wait)) {
+		wake_up_interruptible(&ctx->cq_wait);
+		kill_fasync(&ctx->cq_fasync, SIGIO, POLL_IN);
+	}
 }
 
 static void io_cqring_add_event(struct io_ring_ctx *ctx, u64 user_data,
@@ -840,6 +847,7 @@  static void io_iopoll_complete(struct io_ring_ctx *ctx, unsigned int *nr_events,
 	}
 
 	io_commit_cqring(ctx);
+	io_cqring_ev_posted_iopoll(ctx);
 	io_free_req_many(ctx, reqs, &to_free);
 }