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

[sashiko-bot@kernel.org]

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611114230.950379-2-jolsa@kernel.org?part=1