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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20260129091346.0744de91bb053b24ba679438@kernel.org>
Date: Thu, 29 Jan 2026 09:13:46 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: bot+bpf-ci@...nel.org
Cc: liujing.root@...il.com, ast@...nel.org, daniel@...earbox.net,
 andrii@...nel.org, martin.lau@...ux.dev, eddyz87@...il.com,
 song@...nel.org, yonghong.song@...ux.dev, john.fastabend@...il.com,
 kpsingh@...nel.org, sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org,
 bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, liujing40@...omi.com,
 martin.lau@...nel.org, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH 2/2] bpf: Implement kretprobe fallback for kprobe multi
 link

On Thu, 18 Dec 2025 13:33:31 +0000 (UTC)
bot+bpf-ci@...nel.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@...nel.org>