On 1/6/25 4:24 PM, Andrii Nakryiko wrote:
On Wed, Dec 18, 2024 at 2:53 PM Yonghong Song <yonghong.song@xxxxxxxxx> wrote:Jordan reported an issue in Meta production environment where func try_to_wake_up() is renamed to try_to_wake_up.llvm.<hash>() by clang compiler at lto mode. The original 'kprobe/try_to_wake_up' does not work any more since try_to_wake_up() does not match the actual func name in /proc/kallsyms. There are a couple of ways to resolve this issue. For example, in attach_kprobe(), we could do lookup in /proc/kallsyms so try_to_wake_up() can be replaced by try_to_wake_up.llvm.<hach>(). Or we can force users to use bpf_program__attach_kprobe() where they need to lookup /proc/kallsyms to find out try_to_wake_up.llvm.<hach>(). But these two approaches requires extra work by either libbpf or user. Luckily, suggested by Andrii, multi kprobe already supports wildcard ('*') for symbol matching. In the above example, 'try_to_wake_up*' can match to try_to_wake_up() or try_to_wake_up.llvm.<hash>() and this allows bpf prog works for different kernels as some kernels may have try_to_wake_up() and some others may have try_to_wake_up.llvm.<hash>(). The original intention is to kprobe try_to_wake_up() only, so an optional field unique_match is added to struct bpf_kprobe_multi_opts. If the field is set to true, the number of matched functions must be one. Otherwise, the attachment will fail. In the above case, multi kprobe with 'try_to_wake_up*' and unique_match preserves user functionality. Reported-by: Jordan Rome <linux@xxxxxxxxxxxxxx> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Signed-off-by: Yonghong Song <yonghong.song@xxxxxxxxx> --- tools/lib/bpf/libbpf.c | 10 +++++++++- tools/lib/bpf/libbpf.h | 4 +++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index 66173ddb5a2d..649c6e92972a 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -11522,7 +11522,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, struct bpf_link *link = NULL; const unsigned long *addrs; int err, link_fd, prog_fd; - bool retprobe, session; + bool retprobe, session, unique_match; const __u64 *cookies; const char **syms; size_t cnt; @@ -11558,6 +11558,14 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, err = libbpf_available_kallsyms_parse(&res); if (err) goto error; + + unique_match = OPTS_GET(opts, unique_match, false); + if (unique_match && res.cnt != 1) { + pr_warn("prog '%s': failed to find unique match: cnt %lu\n", + prog->name, res.cnt); + return libbpf_err_ptr(-EINVAL);goto error, leaking resources here
Ack. Will fix.
we should also think about interaction of unique_match interaction for !pattern case, and either reject it (if it makes no sense), or enforce it (if it does, I haven't really thought about which case do we have)
The unique_match only makes sense for pattern case. So I suggest to reject the case unique_match && !pattern. WDYT?
pw-bot: cr+ } + addrs = res.addrs; cnt = res.cnt; } diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h index d45807103565..3020ee45303a 100644 --- a/tools/lib/bpf/libbpf.h +++ b/tools/lib/bpf/libbpf.h @@ -552,10 +552,12 @@ struct bpf_kprobe_multi_opts { bool retprobe; /* create session kprobes */ bool session; + /* enforce unique match */ + bool unique_match; size_t :0; }; -#define bpf_kprobe_multi_opts__last_field session +#define bpf_kprobe_multi_opts__last_field unique_match LIBBPF_API struct bpf_link * bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, -- 2.43.5
