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

[Kumar Kartikeya Dwivedi]

On Wed Jul 8, 2026 at 7:28 AM CEST, Sanghyun Park wrote:
> Sure,
>

Please avoid top-posting.

> I’ll take a look at that issue once the current patch is sorted out.
>

We had to drop this from the tree because of build-time warnings [0], since
mmput_needed will not be used in case CONFIG_MMU is unset. It should be easy
to address by adding __maybe_unused to the mmput_needed's declaration. Could
you make that fix and respin?

  [0]: https://lore.kernel.org/bpf/202607080951.rMUtVX8X-lkp@xxxxxxxxx

> 2026년 7월 7일 (화) 오후 4:44, Daniel Borkmann <daniel@xxxxxxxxxxxxx>님이 작성:
>>
>> On 7/6/26 1:28 PM, Puranjay Mohan wrote:
>> > 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>
>>
>> Applied, thanks! Could either you or Sanghyun look into the pre-existing
>> sashiko-reported issue as well?