[PATCH] io_uring: fix list corruption race in io_pollfree_wake()

[PATCH] io_uring: fix list corruption race in io_pollfree_wake()

[Soham Kute]

io_pollfree_wake() removes the poll wait entry without holding
the waitqueue head lock. Other removal paths take the head lock,
so this can race and lead to list corruption detected by list_debug.

Acquire the waitqueue lock before calling io_poll_remove_waitq(),
matching the locking used in io_poll_remove_entry().

Reported-by: syzbot+ab12f0c08dd7ab8d057c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab12f0c08dd7ab8d057c
Signed-off-by: Soham Kute <officialsohamkute@gmail.com>
---
 io_uring/poll.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index aac4b3b88..006154355 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -383,10 +383,17 @@ static void io_poll_cancel_req(struct io_kiocb *req)
 
 static __cold int io_pollfree_wake(struct io_kiocb *req, struct io_poll *poll)
 {
+	struct wait_queue_head *head;
 	io_poll_mark_cancelled(req);
 	/* we have to kick tw in case it's not already */
 	io_poll_execute(req, 0);
-	io_poll_remove_waitq(poll);
+	/* Pairs with smp_store_release() in io_poll_remove_waitq() */
+	head = smp_load_acquire(&poll->head);
+	if (head) {
+		spin_lock_irq(&head->lock);
+		io_poll_remove_waitq(poll);
+		spin_unlock_irq(&head->lock);
+	}
 	return 1;
 }
 
-- 
2.34.1

Re: [PATCH] io_uring: fix list corruption race in io_pollfree_wake()

[Jens Axboe]

On 2/12/26 4:54 AM, Soham Kute wrote:
> io_pollfree_wake() removes the poll wait entry without holding
> the waitqueue head lock. Other removal paths take the head lock,
> so this can race and lead to list corruption detected by list_debug.
> 
> Acquire the waitqueue lock before calling io_poll_remove_waitq(),
> matching the locking used in io_poll_remove_entry().
> 
> Reported-by: syzbot+ab12f0c08dd7ab8d057c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab12f0c08dd7ab8d057c
> Signed-off-by: Soham Kute <officialsohamkute@gmail.com>
> ---
>  io_uring/poll.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index aac4b3b88..006154355 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -383,10 +383,17 @@ static void io_poll_cancel_req(struct io_kiocb *req)
>  
>  static __cold int io_pollfree_wake(struct io_kiocb *req, struct io_poll *poll)
>  {
> +	struct wait_queue_head *head;
>  	io_poll_mark_cancelled(req);
>  	/* we have to kick tw in case it's not already */
>  	io_poll_execute(req, 0);
> -	io_poll_remove_waitq(poll);
> +	/* Pairs with smp_store_release() in io_poll_remove_waitq() */
> +	head = smp_load_acquire(&poll->head);
> +	if (head) {
> +		spin_lock_irq(&head->lock);
> +		io_poll_remove_waitq(poll);
> +		spin_unlock_irq(&head->lock);
> +	}
>  	return 1;
>  }

The callpath is io_poll_wake() -> io_pollfree_wake(), where the former
where much holds &head->lock already. How did you come to this
conclusion? Your patch would instantly deadlock.

Additionally, if you follow the replies off your syzbot link:

Closes: https://syzkaller.appspot.com/bug?extid=ab12f0c08dd7ab8d057c

you'll see I already debugged this issue, it's a media bug, and there's
no POLLFREE involved at all. And it looks like I'm not even the first
one to debug it, I subsequently found that someone else sent in a
different patch for this last year, but solving the same root cause in
media.

-- 
Jens Axboe