Re: [PATCH bpf-next 11/17] libbpf: Add support to create tracing multi link

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Feb 20, 2026 at 10:57:53AM +0000, bot+bpf-ci@kernel.org wrote:
> > The provided <pattern> is used as 'pattern' argument in
> > bpf_program__attach_kprobe_multi_opts function.
> 
> This isn't a bug, but the commit message references
> bpf_program__attach_kprobe_multi_opts when the code actually calls
> bpf_program__attach_tracing_multi. Looks like a leftover from
> the kprobe_multi code this was modeled after.

copy&paste error :-\ thnx

> 
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> 
> [ ... ]
> 
> > +struct bpf_link *
> > +bpf_program__attach_tracing_multi(const struct bpf_program *prog, const char *pattern,
> > +				  const struct bpf_tracing_multi_opts *opts)
> > +{
> > +	LIBBPF_OPTS(bpf_link_create_opts, lopts);
> > +	__u32 *ids, cnt, *free_ids = NULL;
> 
> cnt is declared as __u32 here, but collect_btf_func_ids_by_glob()
> returns int. When that function returns a negative error such as
> -ENOMEM, the value wraps to a large unsigned number.

yes, will fix

> 
> > +	__u64 *cookies;
> > +	int prog_fd, link_fd, err;
> > +	struct bpf_link *link;
> 
> [ ... ]
> 
> > +	if (pattern) {
> > +		err = bpf_object__load_vmlinux_btf(prog->obj, true);
> > +		if (err)
> > +			return libbpf_err_ptr(err);
> > +
> > +		cnt = collect_btf_func_ids_by_glob(prog->obj->btf_vmlinux, pattern, &ids);
> > +		if (cnt < 0)
> > +			return libbpf_err_ptr(cnt);
> 
> Since cnt is __u32, this comparison is always false. On an allocation
> failure inside collect_btf_func_ids_by_glob(), the error is silently
> ignored. That function also does free(*ids) before returning -ENOMEM,
> so ids becomes a dangling pointer. The code then continues with a
> freed ids pointer and a garbage cnt, and eventually double-frees ids
> through the error label via free_ids.
> 
> Should cnt be declared as int instead of __u32?

it should, will fix

> 
> > +		if (cnt == 0)
> > +			return libbpf_err_ptr(-EINVAL);
> > +		free_ids = ids;
> > +	}
> 
> [ ... ]
> 
> > +error:
> > +	free(link);
> > +	free(free_ids);
> > +	return libbpf_err_ptr(err);
> > +}
> > +
> > +static int attach_tracing_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > +{
> > +	bool is_fexit, is_fsession;
> > +	const char *spec;
> > +	char *pattern;
> > +	int n;
> > +
> > +	/* Do not allow auto attach if there's no function pattern. */
> > +	if (strcmp(prog->sec_name, "fentry.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fexit.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fsession.multi") == 0 ||
> > +	    strcmp(prog->sec_name, "fentry.multi.s") == 0 ||
> > +	    strcmp(prog->sec_name, "fexit.multi.s") == 0 ||
> > +	    strcmp(prog->sec_name, "fsession.multi.s") == 0)
> > +		return 0;
> > +
> > +	is_fexit = str_has_pfx(prog->sec_name, "fexit.multi/");
> > +	is_fsession = str_has_pfx(prog->sec_name, "fsession.multi/");
> > +
> > +	if (is_fsession)
> > +		spec = prog->sec_name + sizeof("fsession.multi/") - 1;
> > +	else if (is_fexit)
> > +		spec = prog->sec_name + sizeof("fexit.multi/") - 1;
> > +	else
> > +		spec = prog->sec_name + sizeof("fentry.multi/") - 1;
> 
> The sleepable variants with patterns don't seem to be handled here.
> For a section name like "fentry.multi.s/my_func", none of the
> str_has_pfx checks match because "fexit.multi/" and "fsession.multi/"
> are not prefixes of "fentry.multi.s/..." (the character at position
> 11 or 14 is '.' not '/').
> 
> The else branch then computes spec using sizeof("fentry.multi/") - 1
> which gives an offset of 13, pointing into "s/my_func" instead of
> "my_func". The sscanf then extracts "s" as the pattern, and the
> program attaches to any function matching the glob "s" rather than
> "my_func".
> 
> Similarly for "fexit.multi.s/X" and "fsession.multi.s/X", the
> offsets are wrong and produce either error returns or incorrect
> patterns.
> 
> The SEC_DEF entries register all six sleepable variants, and
> sec_def_matches() correctly distinguishes "fentry.multi.s+" from
> "fentry.multi+" using the '/' separator check, so the matching
> works but the pattern extraction here does not.
> 
> Would it work to also check for the ".s/" variants, for example:
> 
>     is_fexit = str_has_pfx(prog->sec_name, "fexit.multi/") ||
>                str_has_pfx(prog->sec_name, "fexit.multi.s/");
> 
> with the corresponding sizeof adjustments for the spec offset?

yes, I mentioned in the cover letter that I did not add tests for
sleepable functions attachments.. I should have ;-) will fix

thanks,
jirka