On Fri, Feb 20, 2026 at 10:57:53AM +0000, [email protected] 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
