Re: [PATCH 2/2] bpf: Implement kretprobe fallback for kprobe multi link

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Thu, 18 Dec 2025 13:33:31 +0000 (UTC)
bot+bpf-ci@kernel.org wrote:

> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 1fd07c10378f..426a1c627508 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> 
> [ ... ]
> 
> > +int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> > +{
> > +	struct bpf_kprobe_multi_link *link = NULL;
> > +	struct bpf_link_primer link_primer;
> > +	struct bpf_kprobe *kprobes = NULL;
> > +	u32 flags, cnt;
> > +	u64 __user *ucookies;
> > +	unsigned long __user *uaddrs;
> > +	unsigned long __user *usyms;
> > +	int err, i;
> 
> [ ... ]
> 
> > +	for (i = 0; i < cnt; i++) {
> > +		unsigned long addr;
> > +
> > +		if (uaddrs) {
> > +			if (__get_user(addr, uaddrs + i)) {
> > +				err = -EFAULT;
> > +				goto error;
> > +			}
> > +		} else {
> > +			unsigned long __user usymbol;
> > +			char buf[KSYM_NAME_LEN];
> > +
> > +			if (__get_user(usymbol, usyms + i)) {
> > +				err = -EFAULT;
> > +				goto error;
> > +			}
> > +			err = strncpy_from_user(buf, (const char __user *) usymbol, KSYM_NAME_LEN);
> > +			if (err == KSYM_NAME_LEN)
> > +				err = -E2BIG;
> > +			if (err < 0)
> > +				goto error;
> > +
> > +			addr = kallsyms_lookup_name(buf);
> > +			if (!addr)
> > +				goto error;
> 
> When kallsyms_lookup_name() returns 0 (symbol not found), is err being set
> to an error code before jumping to the error label? It looks like err will
> still hold the positive return value from strncpy_from_user() (the string
> length), which would be returned to the caller as a success indication.

Good catch. It should set err = -ENOENT.

Thanks,

> 
> The error label at the end of the function does:
> 
>     error:
>         kvfree(kprobes);
>         kfree(link);
>         return err;
> 
> This could cause the syscall to return a positive value to userspace instead
> of an error, and also leak a prog reference since link_create() only calls
> bpf_prog_put() when ret < 0.
> 
> > +		}
> > +		if (prog->kprobe_override && !within_error_injection_list(addr)) {
> > +			err = -EINVAL;
> > +			goto error;
> > +		}
> 
> [ ... ]
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20338242683


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>