| Message ID | 20240409200900.783069-1-mitchell.augustin@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [noble] aio: Fix null ptr deref in aio_complete() wakeup | expand |
Acked-by: Thibault Ferrante <thibault.ferrante@canonical.com> Please add v2 to the mail header when sending a patch a second time to the mailing list with a small text explaining the difference with the previous version. On 09-04-2024 22:09, 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> > --- > 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); > }
On 24/04/09 03:09PM, 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> > --- > 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); > } Acked-by: Andrei Gherzan <andrei.gherzan@canonical.com>
On 09/04/2024 22:09, 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> > --- > 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); > } Applied to noble master-next branch. Thanks!
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); }