Re: [PATCH bpf-next v3 2/7] 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:
- [Low] The `dst` arguments in `bpf_copy_from_user_mm` and `bpf_copy_from_user_mm_str` lack the `__uninit` suffix, causing the verifier to reject uninitialized stack memory and forcing unnecessary zero-initialization.
--

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

bpf: Add user memory access kfuncs for mm_struct

This commit adds bpf_copy_from_user_mm() and bpf_copy_from_user_mm_str()
kfuncs to allow BPF programs to access trusted address spaces not associated
with a task_struct. It also updates existing task-based user memory access
helpers to use these new functions.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b3cc5c8fc8756..19c01aa734ae1 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c

[ ... ]

> +__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)
> +{

[Severity: Low]
Should the dst parameter be named dst__uninit here?

Without the __uninit suffix, the BPF verifier natively treats void pointer
kfunc parameters as ARG_PTR_TO_MEM. This forces BPF programs to redundantly
zero-initialize their buffers before calling these sleepable kfuncs, degrading
usability and performance compared to the older bpf_copy_from_user_task helper.

> +	int ret;
> +
> +	if (unlikely(flags))
> +		return -EINVAL;

[Severity: Low]
If the __uninit suffix is added, the verifier will assume the buffer is
initialized upon return. Without clearing the buffer on this early path,
could this allow a BPF program to read uninitialized kernel stack memory?

[ ... ]

> +__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)
>  {
>  	int ret;

[Severity: Low]
Similar to bpf_copy_from_user_mm above, does this dst parameter also need the
__uninit suffix to avoid forcing BPF programs to pre-initialize the memory?

If it does, the early flags check in this function would also need to zero
the buffer before returning -EINVAL to avoid leaking uninitialized memory.

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