Re: [PATCH bpf-next 1/2] libbpf: Add unique_match option for multi kprobe

[Yonghong Song <yonghong.song@linux.dev>]

On 1/6/25 4:24 PM, Andrii Nakryiko wrote:
> On Wed, Dec 18, 2024 at 2:53 PM Yonghong Song <yonghong.song@linux.dev> 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@jordanrome.com>
>> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   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
>>