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

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

[Jiri Olsa]

As reported by sashiko we use __get_user without prior access_ok call on the
user space pointer. Adding the missing call for the whole pointer array.

Plus removing the err check in the error path, because it's not needed and
also we can return -ENOMEM directly from the first kvmalloc_array fail path.

Cc: stable@vger.kernel.org
[1] https://lore.kernel.org/bpf/20260611115503.AC16D1F00893@smtp.kernel.org/
Fixes: 0236fec57a15 ("bpf: Resolve symbols with ftrace_lookup_symbols for kprobe multi link")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/bpf/20260611115503.AC16D1F00893@smtp.kernel.org/
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 82f8feea6931..75495a5c3507 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2376,9 +2376,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)
@@ -2403,10 +2406,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;
 }
 
-- 
2.54.0

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

[sashiko-bot]

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`.
--

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

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

[Jiri Olsa]

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