diff mbox series

aio: Fix null ptr deref in aio_complete() wakeup

Message ID 20240409195607.759234-1-mitchell.augustin@canonical.com
State New
Headers show
Series aio: Fix null ptr deref in aio_complete() wakeup | expand

Commit Message

mitchell.augustin@canonical.com April 9, 2024, 7:56 p.m. UTC
From: Kent Overstreet <kent.overstreet@linux.dev>

BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2058557

list_del_init_careful() needs to be the last access to the wait queue
entry - it effectively unlocks access.

Previously, finish_wait() would see the empty list head and skip taking
the lock, and then we'd return - but the completion path would still
attempt to do the wakeup after the task_struct pointer had been
overwritten.

Fixes: 71eb6b6b0ba9 ("fs/aio: obey min_nr when doing wakeups")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/linux-fsdevel/CAHTA-ubfwwB51A5Wg5M6H_rPEQK9pNf8FkAGH=vr=FEkyRrtqw@mail.gmail.com/
Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
Link: https://lore.kernel.org/stable/20240331215212.522544-1-kent.overstreet%40linux.dev
Link: https://lore.kernel.org/r/20240331215212.522544-1-kent.overstreet@linux.dev
Signed-off-by: Christian Brauner <brauner@kernel.org>
(cherry picked from commit caeb4b0a11b3393e43f7fa8e0a5a18462acc66bd)
Signed-off-by: Mitchell Augustin <mitchell.augustin@canonical.com>
---
 fs/aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stefan Bader April 10, 2024, 7:40 a.m. UTC | #1
On 09.04.24 21:56, mitchell.augustin@canonical.com wrote:
> From: Kent Overstreet <kent.overstreet@linux.dev>
> 
> BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2058557
> 
> list_del_init_careful() needs to be the last access to the wait queue
> entry - it effectively unlocks access.
> 
> Previously, finish_wait() would see the empty list head and skip taking
> the lock, and then we'd return - but the completion path would still
> attempt to do the wakeup after the task_struct pointer had been
> overwritten.
> 
> Fixes: 71eb6b6b0ba9 ("fs/aio: obey min_nr when doing wakeups")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/linux-fsdevel/CAHTA-ubfwwB51A5Wg5M6H_rPEQK9pNf8FkAGH=vr=FEkyRrtqw@mail.gmail.com/
> Signed-off-by: Kent Overstreet <kent.overstreet@linux.dev>
> Link: https://lore.kernel.org/stable/20240331215212.522544-1-kent.overstreet%40linux.dev
> Link: https://lore.kernel.org/r/20240331215212.522544-1-kent.overstreet@linux.dev
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> (cherry picked from commit caeb4b0a11b3393e43f7fa8e0a5a18462acc66bd)
> Signed-off-by: Mitchell Augustin <mitchell.augustin@canonical.com>
> ---

Rejected for the following reasons:
- another submission on mailing list which has Noble as target. This 
submission has no
   proper target. Assuming the new submission to be a replacement of this.

-Stefan

>   fs/aio.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index 9cdaa2faa536..0f4f531c9780 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -1202,8 +1202,8 @@ static void aio_complete(struct aio_kiocb *iocb)
>   		spin_lock_irqsave(&ctx->wait.lock, flags);
>   		list_for_each_entry_safe(curr, next, &ctx->wait.head, w.entry)
>   			if (avail >= curr->min_nr) {
> -				list_del_init_careful(&curr->w.entry);
>   				wake_up_process(curr->w.private);
> +				list_del_init_careful(&curr->w.entry);
>   			}
>   		spin_unlock_irqrestore(&ctx->wait.lock, flags);
>   	}
diff mbox series

Patch

diff --git a/fs/aio.c b/fs/aio.c
index 9cdaa2faa536..0f4f531c9780 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1202,8 +1202,8 @@  static void aio_complete(struct aio_kiocb *iocb)
 		spin_lock_irqsave(&ctx->wait.lock, flags);
 		list_for_each_entry_safe(curr, next, &ctx->wait.head, w.entry)
 			if (avail >= curr->min_nr) {
-				list_del_init_careful(&curr->w.entry);
 				wake_up_process(curr->w.private);
+				list_del_init_careful(&curr->w.entry);
 			}
 		spin_unlock_irqrestore(&ctx->wait.lock, flags);
 	}