Re: [PATCH bpf-next v8 3/5] bpf: Add user memory access kfuncs for mm_struct

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: UAF in bpf_copy_from_user_mm due to unreferenced mm_struct
--

commit cbb90d2c797aaefaca575d8d6f41d087c38c6414
Author: Anastasios Papagiannis <tasos.papagiannnis@gmail.com>

bpf: Add user memory access kfuncs for mm_struct

This commit adds new bpf_copy_from_user_mm() and bpf_copy_from_user_mm_str()
kfuncs that allow BPF programs to access an address space directly using an
mm_struct pointer. It refactors task-based helpers to share their internal
implementations with the new mm-based kfuncs.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 051b6654e57c6..f6b3eee6098a0 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
[ ... ]
> @@ -3659,6 +3697,68 @@ __bpf_kfunc int bpf_copy_from_user_str(void *dst, u32 dst__sz, const void __user
[ ... ]
> +__bpf_kfunc int bpf_copy_from_user_mm(void *dst, u32 dst__sz,
> +				      const void __user *unsafe_ptr__ign,
> +				      struct mm_struct *mm, u64 flags)
> +{
> +	if (unlikely(flags))
> +		return -EINVAL;
> +
> +	if (unlikely(!dst__sz))
> +		return 0;
> +
> +	return __bpf_copy_from_user_mm(dst, dst__sz, unsafe_ptr__ign, mm);

[Severity: High]
Can this result in a Use-After-Free during concurrent task exit?

When a task exits, mmput() drops mm_users to 0 and calls __mmput(), which
invokes exit_mmap(). exit_mmap() releases the mmap_write_lock and then calls
free_pgtables() without holding any locks.

If a BPF program passes a trusted mm_struct like task->mm to this kfunc, it
eventually calls access_remote_vm(mm) without validating or incrementing
mm_users (for example, by using mmget_not_zero()).

Since access_remote_vm() acquires mmap_read_lock(), if this occurs after
exit_mmap() has dropped the write lock but before it destroys the VMA tree,
vma_lookup() will succeed. This means __get_user_pages() will walk page
tables that are being concurrently freed locklessly by free_pgtables(),
resulting in a Use-After-Free.

Does this need to acquire a reference to mm_users before proceeding?

> +}
[ ... ]
> +__bpf_kfunc int bpf_copy_from_user_mm_str(void *dst, u32 dst__sz,
> +					  const void __user *unsafe_ptr__ign,
> +					  struct mm_struct *mm, u64 flags)
> +{
> +	if (unlikely(flags & ~BPF_F_PAD_ZEROS))
> +		return -EINVAL;
> +
> +	if (unlikely(dst__sz == 0))
> +		return 0;
> +
> +	return __bpf_copy_from_user_mm_str(dst, dst__sz, unsafe_ptr__ign,
> +					   mm, flags);

[Severity: High]
Similarly, does this also suffer from the same unreferenced mm_struct
lifetime issue described above?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918091058.297740-1-tasos.papagiannnis@gmail.com?part=3