Re: [PATCH bpf-next v2 14/21] bpf: Add tracing_multi link info support for bpf progs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpftool: buffer over-read of uninitialized heap memory in sym_name_trim_prefix()
- [Medium] bpf: strict fids check in bpf_tracing_multi_link_fill_link_info() breaks UAPI
--

commit 69edd76aa1ef54260b1403707834fa758ec91952
Author: Leon Hwang <leon.hwang@linux.dev>
bpf: Add tracing_multi link info support for bpf progs

This commit adds support for showing BPF program information in tracing_multi 
links via bpftool.

> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 8b1a52350cfda..39c911917ae24 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
[ ... ]
> @@ -3725,9 +3726,13 @@ static int bpf_tracing_multi_link_fill_link_info(const struct bpf_link *link,
>  	info->tracing_multi.attach_type = tr_link->link.attach_type;
>  	info->tracing_multi.count = tr_link->nodes_cnt;
>  	info->tracing_multi.btf_obj_id = btf_obj_id(tr_link->link.prog->aux->attach_btf);
> +	has_progs = !!tr_link->tgt_progs;
> +	info->tracing_multi.flags = has_progs ? BPF_F_TRACING_MULTI_TGT_PROGS : 0;
>  
> -	if (!uids && !ucookies && !uaddrs)
> +	if (!uids && !ucookies && !uaddrs && !fids)
>  		return 0;
> +	if (has_progs != !!fids)
> +		return -EINVAL;

[Severity: Medium]
Does this strict equality check break forward compatibility for generic 
userspace tools?

If a tool unconditionally pre-allocates and provides a func_btf_ids array 
while querying an older, non-BPF-targeted tracing_multi link (where has_progs 
is false), bpf_obj_get_info_by_fd() will fail with -EINVAL.

Would it be safer to gracefully ignore or zero-fill the fids buffer if the 
target does not have programs, rather than rejecting the query entirely?

> diff --git a/tools/bpf/bpftool/link.c b/tools/bpf/bpftool/link.c
> index 5cffa2d283b47..da7e430b14c8a 100644
> --- a/tools/bpf/bpftool/link.c
> +++ b/tools/bpf/bpftool/link.c
[ ... ]
> @@ -422,12 +422,31 @@ show_uprobe_multi_json(struct bpf_link_info *info, json_writer_t *wtr)
>  	jsonw_end_array(json_wtr);
>  }
>  
> +#define BPF_PROG_PREFIX		"bpf_prog_"
> +#define BPF_PROG_PREFIX_LEN	(sizeof(BPF_PROG_PREFIX) - 1)
> +
> +static const char *
> +sym_name_trim_prefix(struct kernel_sym *sym, bool is_prog)
> +{
> +	int prefix_len;
> +
> +	if (!is_prog)
> +		return sym->name;
> +
> +	/* Ref kernel/bpf/core.c::bpf_prog_ksym_set_name() */
> +	prefix_len  = BPF_PROG_PREFIX_LEN;
> +	prefix_len += BPF_TAG_SIZE * 2;
> +	if (sym->name[prefix_len] != '\0')

[Severity: Medium]
Can this result in an out-of-bounds read if the kernel symbol name is shorter 
than the prefix length?

In bpftool, sym->name is allocated using libbpf_reallocarray() and populated 
via sscanf(), which null-terminates the string but leaves the remainder of 
the buffer uninitialized.

If a target address resolves to a short kernel symbol (for example, due to a 
TOCTOU race where a BPF program is unloaded and its vmalloc address is 
reused), sym->name[prefix_len] evaluates uninitialized heap memory. If that 
byte happens to be non-zero, it skips it and returns a pointer into the 
uninitialized region, potentially corrupting CLI or JSON output.

Should we verify that strlen(sym->name) is sufficiently large before checking 
the character at prefix_len?

> +		prefix_len += 1;		/* skip the '_' */
> +	return sym->name + prefix_len;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012154024.29423-1-leon.hwang@linux.dev?part=14