Re: [PATCH v4] libbpf: kprobe.multi: Filter with available_filter_functions

[Jackie Liu <liu.yun@linux.dev>]

Hi Andrii.

在 2023/6/3 01:27, Andrii Nakryiko 写道:
> On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@linux.dev> wrote:
>>
>> Hi Andrii.
>>
>> 在 2023/5/26 04:43, Andrii Nakryiko 写道:
>>> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@linux.dev> wrote:
>>>>
>>>> From: Jackie Liu <liuyun01@kylinos.cn>
>>>>
>>>> When using regular expression matching with "kprobe multi", it scans all
>>>> the functions under "/proc/kallsyms" that can be matched. However, not all
>>>> of them can be traced by kprobe.multi. If any one of the functions fails
>>>> to be traced, it will result in the failure of all functions. The best
>>>> approach is to filter out the functions that cannot be traced to ensure
>>>> proper tracking of the functions.
>>>>
>>>> Use available_filter_functions check first, if failed, fallback to
>>>> kallsyms.
>>>>
>>>> Here is the test eBPF program [1].
>>>> [1] https://github.com/JackieLiu1/ketones/commit/a9e76d1ba57390e533b8b3eadde97f7a4535e867
>>>>
>>>> Suggested-by: Jiri Olsa <olsajiri@gmail.com>
>>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>>> ---
>>>>    tools/lib/bpf/libbpf.c | 92 +++++++++++++++++++++++++++++++++++++-----
>>>>    1 file changed, 83 insertions(+), 9 deletions(-)
>>>>
>>>
>>> Question to you and Jiri: what happens when multi-kprobe's syms has
>>> duplicates? Will the program be attached multiple times? If yes, then
>>> it sounds like a problem? Both available_filters and kallsyms can have
>>> duplicate function names in them, right?

I don't have any idea, I tested it on my own device, and they don't have
duplicate functions.

╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk 
-F' ' '{print $1}' | wc -l 

81882
╭─jackieliu@jackieliu-PC ~/gitee/ketones/src
╰─➤ sudo cat /sys/kernel/debug/tracing/available_filter_functions | awk 
-F' ' '{print $1}' | uniq | wc -l 

81882

>>
>> If I understand correctly, there should be no problem with repeated
>> function registration, because the bottom layer is done through fprobe
>> registration addrs, kprobe.multi itself does not do this work, but
>> fprobe is based on ftrace, it will register addr by makes a hash,
>> that is, if it is the same address, it should be filtered out.
>>
> 
> Looking at kernel code, it seems kernel will actually return error if
> user specifies multiple duplicated names. Because kernel will
> bsearch() to the first instance, and never resolve the second
> duplicated instance. And then will assume that not all symbols are
> resolved.

I wrote a test program myself, but it cannot be attached normally, and
an error will be reported.

const char *sysms[] = {
     "vfs_read",
     "vfs_write",
     "vfs_read",
};

when attach_kprobe_multi, -3 returned.

> 
> So, it worries me that we'll switch from kallsyms to available_filters
> by default, because that introduces new failure modes.

In fact, this is not a new problem introduced by switching from kallsyms
to available_filters. If kallsyms also has duplicate functions, then
this problem will also exist before.

> 
> Either way, let's add a selftest that uses a duplicate function name
> and see what happens?

Hi Jiri, Do you mind write a self-test program for duplicate function? I
saw that it has been written before.
for some reason, I failed to compile kselftest/bpf successfully on
fedora38 and Ubuntu2004. :<


> 
>> The main problem here is not the problem of repeated registration of
>> functions, but some functions are not allowed to hook. For example, when
>> I track vfs_*, vfs_set_acl_prepare_kgid and vfs_set_acl_prepare_kuid are
>> not allowed to hook. These exist under kallsyms, but
>> available_filter_functions does not, I have observed for a while,
>> matching through available_filter_functions can effectively prevent this
>> from happening.
> 
> Yeah, I understand that. My point above is that a)
> available_filter_functions contains duplicates and b) doesn't contain
> addresses. So we are forced to rely on kernel string -> addr
> resolution, which doesn't seem to handle duplicate entries well (let's
> test).

Yes, the test for repeated functions reports errors. If there is an
interface similar to available_filter_functions, which contains the
function name and function address, and ensures that it is not 
duplicate, then it is a good speedup for eBPF program, because using
'strdup' to record the function name consumes a certain amount of
startup time.

> 
> So it's a regression to switch to that without taking any other precautions.
> 

Yes, agree.

-- 
BR, Jackie Liu
>>
>>>
>>>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>>>> index ad1ec893b41b..3dd72d69cdf7 100644
>>>> --- a/tools/lib/bpf/libbpf.c
>>>> +++ b/tools/lib/bpf/libbpf.c
>>>> @@ -10417,13 +10417,14 @@ static bool glob_match(const char *str, const char *pat)
>>>>    struct kprobe_multi_resolve {
>>>>           const char *pattern;
>>>>           unsigned long *addrs;
>>>> +       const char **syms;
>>>>           size_t cap;
>>>>           size_t cnt;
>>>>    };
>>>>
> 
> [...]