Re: [PATCHv3 bpf-next 1/7] bpf: Guard __get_user acesss with access_ok for uprobe_multi data

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Jun 11, 2026 at 11:55:02AM +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() check for the usyms array in bpf_kprobe_multi_link_attach() allows arbitrary kernel memory read.
> --
> 
> commit e5440667240e003d50bb838ad4b19e526ef6c703
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Guard __get_user acesss with access_ok for uprobe_multi data
> 
> This patch adds access_ok checks for uprobe_multi arrays before calling
> __get_user. It ensures that the user space data bounds are safe.
> 
> Link: https://lore.kernel.org/bpf/20260610145235.CB1441F00893@smtp.kernel.org/
> 
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> 
> [ ... ]
> 
> > @@ -3261,6 +3262,16 @@ int bpf_uprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >  	uref_ctr_offsets = u64_to_user_ptr(attr->link_create.uprobe_multi.ref_ctr_offsets);
> >  	ucookies = u64_to_user_ptr(attr->link_create.uprobe_multi.cookies);
> >  
> > +	/*
> > +	 * All uoffsets/uref_ctr_offsets/ucookies arrays have the same value
> > +	 * size, we need to check their address range is safe for __get_user
> > +	 * calls.
> > +	 */
> > +	size = sizeof(*uoffsets) * cnt;
> > +	if (!access_ok(uoffsets, size) || !access_ok(uref_ctr_offsets, size) ||
> > +	    !access_ok(ucookies, size))
> > +		return -EFAULT;
> 
> [Severity: High]
> This is a pre-existing issue, but while you are adding these access checks
> for uprobes, does bpf_kprobe_multi_link_attach() have an identical missing
> check for the usyms array?
> 
> Looking at bpf_kprobe_multi_link_attach(), it calls copy_user_syms() with
> the unchecked user pointer usyms:
> 
> kernel/trace/bpf_trace.c:bpf_kprobe_multi_link_attach() {
>     ...
> 		struct user_syms us;
> 
> 		err = copy_user_syms(&us, usyms, cnt);
> 		if (err)
> 			goto error;
>     ...
> }
> 
> And copy_user_syms() iteratively calls __get_user(usymbol, usyms + i)
> without any prior access checks on the array:
> 
> kernel/trace/bpf_trace.c:copy_user_syms() {
>     ...
> 	for (p = buf, i = 0; i < cnt; i++) {
> 		if (__get_user(usymbol, usyms + i)) {
> 			err = -EFAULT;
> 			goto error;
> 		}
>     ...
> }
> 
> Since __get_user() bypasses bounds checking, couldn't an attacker provide
> a kernel address for usyms and read arbitrary kernel memory into usymbol?
> 
> A subsequent strncpy_from_user() call would use usymbol as its source
> pointer, potentially acting as a side-channel oracle to leak kernel
> information or bypass kernel address layout randomization.
> 
> Should copy_user_syms() or bpf_kprobe_multi_link_attach() also include
> an access_ok() check for the usyms array?

I'm not sure it's possible to exploit it, but the contract for __get_user is
clear, so I think we should call access_ok also in here, I'll send the fix

jirka