Re: [PATCH bpf] bpf: Add missing access_ok call to copy_user_syms

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Jun 16, 2026 at 08:42:03AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing `access_ok()` checks for user pointers `uoffsets`, `uref_ctr_offsets`, and `ucookies` before using `__get_user()` in `bpf_uprobe_multi_link_attach`.
> --

it's fixed in separate post in here:
  https://lore.kernel.org/bpf/20260611114230.950379-2-jolsa@kernel.org/

jirka

> 
> commit cd20e04dceb618df71198c96a9e801e033dcb0a0
> Author: Jiri Olsa <jolsa@kernel.org>
> bpf: Add missing access_ok call to copy_user_syms
> 
> The patch adds a missing access_ok() check for the user pointer array usyms
> in copy_user_syms() before accessing it via __get_user(). It also cleans
> up the error path by returning -ENOMEM directly upon memory allocation failure
> instead of using a goto.
> 
> Link: https://lore.kernel.org/bpf/20260611115503.AC16D1F00893@smtp.kernel.org/
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index a02bd258677ee..c274109d020f3 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2331,9 +2331,12 @@ static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32
> >  	int err = -ENOMEM;
> >  	unsigned int i;
> >  
> > +	if (!access_ok(usyms, cnt * sizeof(*usyms)))
> > +		return -EFAULT;
> > +
> >  	syms = kvmalloc_array(cnt, sizeof(*syms), GFP_KERNEL);
> >  	if (!syms)
> > -		goto error;
> > +		return -ENOMEM;
> >  
> >  	buf = kvmalloc_array(cnt, KSYM_NAME_LEN, GFP_KERNEL);
> >  	if (!buf)
> > @@ -2358,10 +2361,8 @@ static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32
> >  	return 0;
> >  
> >  error:
> > -	if (err) {
> > -		kvfree(syms);
> > -		kvfree(buf);
> > -	}
> > +	kvfree(syms);
> > +	kvfree(buf);
> >  	return err;
> >  }
> 
> [Severity: High]
> This is a pre-existing issue, but while reviewing this patch I noticed a
> similar pattern in bpf_uprobe_multi_link_attach() within the same file.
> 
> kernel/trace/bpf_trace.c:bpf_uprobe_multi_link_attach() {
>     ...
> 	for (i = 0; i < cnt; i++) {
> 		if (__get_user(uprobes[i].offset, uoffsets + i)) {
> 			err = -EFAULT;
> 			goto error_free;
> 		}
> 		if (uprobes[i].offset < 0) {
> 			err = -EINVAL;
> 			goto error_free;
> 		}
> 		if (uref_ctr_offsets && __get_user(uprobes[i].ref_ctr_offset, uref_ctr_offsets + i)) {
> 			err = -EFAULT;
> 			goto error_free;
> 		}
> 		if (ucookies && __get_user(uprobes[i].cookie, ucookies + i)) {
> 			err = -EFAULT;
> 			goto error_free;
> 		}
>     ...
> }
> 
> Are the user-provided array pointers uoffsets, uref_ctr_offsets, and ucookies
> missing overarching access_ok() validations before being accessed in this loop?
> 
> Since __get_user() explicitly bypasses the access_ok() validation performed by
> get_user(), reading these pointers without a prior check could potentially allow
> an attacker to pass arbitrary kernel memory addresses and trigger unauthorized
> reads or crashes.
> 
> Does this function need the same fix as copy_user_syms()?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260616083056.405652-1-jolsa@kernel.org?part=1