Re: [PATCH bpf-next v6] bpf: Fix use-after-free on mm_struct in bpf_find_vma()

[Puranjay Mohan]

On Wed, Jul 1, 2026 at 8:48 PM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Jun 29, 2026 at 8:25 PM Sanghyun Park
> <sanghyun.park.cnu@xxxxxxxxx> wrote:
> >
> > bpf_find_vma() reads task->mm and calls mmap_read_trylock(mm) without
> > holding a reference on the mm. On a foreign task, a concurrent exit_mm()
> > can free the mm_struct between the lockless read and the trylock,
> > resulting in a use-after-free. mm_struct is not SLAB_TYPESAFE_BY_RCU.
> >
> > For the current task, task->mm is stable. For a foreign task, pin the mm
> > under task->alloc_lock and release it with mmput_async(), mirroring commit
> > d8e27d2d22b6 ("bpf: fix mm lifecycle in open-coded task_vma iterator").
> > Use spin_trylock() instead of get_task_mm() so BPF context does not block
> > on alloc_lock. Reject irqs-disabled contexts and !CONFIG_MMU on the
> > foreign-task path because dropping the mm reference is not safe there.
> >
> > Race:
> >
> >   CPU0 (BPF program)                  CPU1 (exiting task)
> >   ============================        ==========================
> >   bpf_find_vma(foreign_task):
> >     mm = task->mm
> >                                       exit_mm():
> >                                         task->mm = NULL
> >                                         mmput(mm) -> frees mm_struct
> >     mmap_read_trylock(mm)
> >         // UAF on mm
> >
> > Fixes: 7c7e3d31e785 ("bpf: Introduce helper bpf_find_vma")
> > Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> > Signed-off-by: Sanghyun Park <sanghyun.park.cnu@xxxxxxxxx>
> > ---
>
> Puranjay, PTAL, I think you've been fixing similar issue in
> bpf_iter_task_vma_new(), would be nice for you to check this fix as
> well. Thanks!
>
> > v6:
> >  - Restore bpf_iter_mmput_async() because bpf_iter_task_vma_destroy()
> >    still uses it.
> >  - Keep the direct mmput_async() call in bpf_find_vma(), but guard it with
> >    CONFIG_MMU because mmput_async() is declared only for CONFIG_MMU builds.
> > v5: https://lore.kernel.org/bpf/20260630023443.3026627-2-sanghyun.park.cnu@xxxxxxxxx/
> >  - Replace bpf_iter_mmput_async() with mmput_async() directly as suggested
> >    by Yonghong.
> >  - Add Yonghong's Acked-by.
> > v4: https://lore.kernel.org/bpf/20260610024637.343364-1-sanghyun.park.cnu@xxxxxxxxx/
> >  - Use [PATCH bpf-next] subject as requested by Alexei.
> >  - Add the missing BPF maintainers/reviewers to Cc.
> > v3: https://lore.kernel.org/bpf/20260609105216.3536839-1-sanghyun.park.cnu@xxxxxxxxx/
> >  - Drop get_task_mm()+mmput(); mirror d8e27d2d22b6 with alloc_lock
> >    trylock + mmput_async(). (Yonghong Song)
> >  - Reject irqs-disabled contexts on the foreign-task path.
> >  - Reject foreign-task path when !CONFIG_MMU: bpf_iter_mmput_async()
> >    falls back to mmput() which may sleep, and bpf_find_vma() can run
> >    in non-sleepable context.
> >  - Shorten the foreign-task rationale comment and trim the changelog body.
> >  - Fix the v2's whitespace damage.
> > v2: https://lore.kernel.org/bpf/CAOrxSK5_7e4114VyfEU9htGi+UneuNt88fGVKOAa3_ZenPOFkA@xxxxxxxxxxxxxx/
> >
> >  kernel/bpf/task_iter.c | 36 +++++++++++++++++++++++++++++++++---
> >  1 file changed, 33 insertions(+), 3 deletions(-)
> >
> > diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
> > index fc5f463ca529a..1d56193caf5e4 100644
> > --- a/kernel/bpf/task_iter.c
> > +++ b/kernel/bpf/task_iter.c
> > @@ -760,6 +760,7 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
> >         struct mmap_unlock_irq_work *work = NULL;
> >         struct vm_area_struct *vma;
> >         bool irq_work_busy = false;
> > +       bool mmput_needed = false;
> >         struct mm_struct *mm;
> >         int ret = -ENOENT;
> >
> > @@ -769,14 +770,38 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
> >         if (!task)
> >                 return -ENOENT;
> >
> > -       mm = task->mm;
> > +       if (task == current) {
> > +               mm = task->mm;
> > +       } else {
> > +               /*
> > +                * Foreign task: pin task->mm against a concurrent exit_mm().
> > +                * Use trylock on alloc_lock instead of get_task_mm()'s
> > +                * blocking task_lock() to avoid deadlocking the target task.
> > +                */
> > +               if (!IS_ENABLED(CONFIG_MMU))
> > +                       return -EOPNOTSUPP;
> > +               if (irqs_disabled())
> > +                       return -EBUSY;
> > +               if (!spin_trylock(&task->alloc_lock))
> > +                       return -EBUSY;
> > +               mm = task->mm;
> > +               if (mm && !(task->flags & PF_KTHREAD)) {
> > +                       mmget(mm);
> > +                       mmput_needed = true;
> > +               } else {
> > +                       mm = NULL;
> > +               }
> > +               spin_unlock(&task->alloc_lock);
> > +       }
> >         if (!mm)
> >                 return -ENOENT;
> >
> >         irq_work_busy = bpf_mmap_unlock_get_irq_work(&work);
> >
> > -       if (irq_work_busy || !mmap_read_trylock(mm))
> > -               return -EBUSY;
> > +       if (irq_work_busy || !mmap_read_trylock(mm)) {
> > +               ret = -EBUSY;
> > +               goto out;
> > +       }
> >
> >         vma = find_vma(mm, start);
> >
> > @@ -786,6 +811,11 @@ BPF_CALL_5(bpf_find_vma, struct task_struct *, task, u64, start,
> >                 ret = 0;
> >         }
> >         bpf_mmap_unlock_mm(work, mm);
> > +out:
> > +#ifdef CONFIG_MMU
> > +       if (mmput_needed)
> > +               mmput_async(mm);
> > +#endif
> >         return ret;
> >  }
> >
> > --
> > 2.48.1

Thanks for this, it looks good to me.

Reviewed-by: Puranjay Mohan <puranjay@xxxxxxxxxx>