public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: io-uring <[email protected]>
Subject: [PATCH] io_uring: prevent eventfd recursion on poll
Date: Thu, 30 Jan 2020 22:24:56 -0700	[thread overview]
Message-ID: <[email protected]> (raw)

If we register an eventfd with io_uring for completion notification,
and then subsequently does a poll for that very descriptor, then we
can trigger a deadlock scenario. Once a request completes and signals
the eventfd context, that will in turn trigger the poll command to
complete. When that poll request completes, it'll try trigger another
event on the eventfd, but this time off the path led us to complete
the poll in the first place. The result is a deadlock in eventfd,
as it tries to ctx->wqh.lock in a nested fashion.

Check if the file in question for the poll request is our eventfd
context, and if it is, don't trigger a nested event for the poll
completion.

Cc: [email protected]
Signed-off-by: Jens Axboe <[email protected]>

---

We're holding the file here, so checking ->private_data is fine.
It's a bit iffy in that it implies knowledge that this is where
eventfd stores the ctx. We could potentially make that helper in
eventfd, if needed.

diff --git a/fs/io_uring.c b/fs/io_uring.c
index ac5340fdcdfe..5788c2139c72 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1025,16 +1025,21 @@ static inline bool io_should_trigger_evfd(struct io_ring_ctx *ctx)
 	return io_wq_current_is_worker() || in_interrupt();
 }
 
-static void io_cqring_ev_posted(struct io_ring_ctx *ctx)
+static void __io_cqring_ev_posted(struct io_ring_ctx *ctx, bool no_evfd)
 {
 	if (waitqueue_active(&ctx->wait))
 		wake_up(&ctx->wait);
 	if (waitqueue_active(&ctx->sqo_wait))
 		wake_up(&ctx->sqo_wait);
-	if (ctx->cq_ev_fd && io_should_trigger_evfd(ctx))
+	if (!no_evfd && ctx->cq_ev_fd && io_should_trigger_evfd(ctx))
 		eventfd_signal(ctx->cq_ev_fd, 1);
 }
 
+static void io_cqring_ev_posted(struct io_ring_ctx *ctx)
+{
+	__io_cqring_ev_posted(ctx, false);
+}
+
 /* Returns true if there are no backlogged entries after the flush */
 static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force)
 {
@@ -3586,13 +3591,27 @@ static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
 
 		if (llist_empty(&ctx->poll_llist) &&
 		    spin_trylock_irqsave(&ctx->completion_lock, flags)) {
+			bool no_evfd = false;
+
 			hash_del(&req->hash_node);
 			io_poll_complete(req, mask, 0);
 			req->flags |= REQ_F_COMP_LOCKED;
+
+			/*
+			 * If the file triggering the poll is our eventfd
+			 * context, then don't trigger another event for this
+			 * request. If we do, we can recurse, as this poll
+			 * request may be triggered by a completion that
+			 * signaled our eventfd.
+			 */
+			if (ctx->cq_ev_fd &&
+			    req->file->private_data == ctx->cq_ev_fd)
+				no_evfd = true;
+
 			io_put_req(req);
 			spin_unlock_irqrestore(&ctx->completion_lock, flags);
 
-			io_cqring_ev_posted(ctx);
+			__io_cqring_ev_posted(ctx, no_evfd);
 			req = NULL;
 		} else {
 			req->result = mask;

-- 
Jens Axboe


             reply	other threads:[~2020-01-31  5:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-31  5:24 Jens Axboe [this message]
2020-02-04  9:38 ` [PATCH] io_uring: prevent eventfd recursion on poll Daurnimator
2020-02-04 18:57   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox