> 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.
> 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.
> + __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?
> + 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?
> +
> + n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
> + if (n < 1) {
> + pr_warn("tracing multi pattern is invalid: %s\n", spec);
> + return -EINVAL;
> + }
> +
> + *link = bpf_program__attach_tracing_multi(prog, pattern, NULL);
> + free(pattern);
> + return libbpf_get_error(*link);
> +}
---
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/22220470181
AI-authorship-score: low
AI-authorship-explanation: Written by a well-known BPF maintainer with long
contribution history; bugs are typical human copy-paste errors from modeling
after existing kprobe_multi code, and the patch evolved through multiple RFC
revisions on LKML.
issues-found: 3
issue-severity-score: medium
issue-severity-explanation: The __u32 cnt type bug creates dead error handling
that can lead to a double-free under memory pressure, and the sleepable section
parsing bug causes wrong functions to be attached silently; both affect
correctness of a new public API.
