Skip to contentSkip to conversation

io_uring: fix potential deadlock in io_poll_wake()

PATCH In review Jens Axboe 1 participant 2 messages Nov 12, 2019

Subsystems (2)
lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

PATCH 1/1 io_uring: fix potential deadlock in io_poll_wake()

We attempt to run the poll completion inline, but we're using trylock to do so. This avoids a deadlock since we're grabbing the locks in reverse order at this point, we already hold the poll wq lock and we're trying to grab the completion lock, while the normal rules are the reverse of that order.

IO completion for a timeout link will need to grab the completion lock, so it's not safe to use from this particular context. If the poll request has a timeout link, don't attempt complete inline.

---

fs/io_uring.c +8 -1
index 3c573f0578a8..e15c2f8e0840 100644
@@ -2064,7 +2064,14 @@ static int io_poll_wake(struct wait_queue_entry *wait, unsigned mode, int sync,
 
 	list_del_init(&poll->wait.entry);
 
-	if (mask && spin_trylock_irqsave(&ctx->completion_lock, flags)) {
+	/*
+	 * Run completion inline if we can. We're using trylock here because
+	 * we are violating the completion_lock -> poll wq lock ordering.
+	 * If we have a link timeout we're going to need the completion_lock
+	 * for finalizing the request, don't allow inline completion for that.
+	 */
+	if (!(req->flags & REQ_F_LINK_TIMEOUT) &&
+	    mask && spin_trylock_irqsave(&ctx->completion_lock, flags)) {
 		list_del(&req->list);
 		io_poll_complete(req, mask);
 		spin_unlock_irqrestore(&ctx->completion_lock, flags);
Signature · 1 line
Jens Axboe
author maintainer
#1 lore raw
Quoted 9 lines · Jens Axboe

We attempt to run the poll completion inline, but we're using trylock to do so. This avoids a deadlock since we're grabbing the locks in reverse order at this point, we already hold the poll wq lock and we're trying to grab the completion lock, while the normal rules are the reverse of that order.

IO completion for a timeout link will need to grab the completion lock, so it's not safe to use from this particular context. If the poll request has a timeout link, don't attempt complete inline.

It's a shame to lose the inline run with a linked timer, I'm going to send a v2 that retains this.

Signature · 1 line
Jens Axboe