On Tue, Jun 6, 2023 at 11:01 PM Jackie Liu <liu.yun@xxxxxxxxx> wrote: > > Hi Andrii. > > 在 2023/6/3 01:27, Andrii Nakryiko 写道: > > On Thu, May 25, 2023 at 6:38 PM Jackie Liu <liu.yun@xxxxxxxxx> wrote: > >> > >> Hi Andrii. > >> > >> 在 2023/5/26 04:43, Andrii Nakryiko 写道: > >>> On Thu, May 25, 2023 at 3:28 AM Jackie Liu <liu.yun@xxxxxxxxx> wrote: > >>>> > >>>> From: Jackie Liu <liuyun01@xxxxxxxxxx> > >>>> > >>>> 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@xxxxxxxxx> > >>>> Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx> > >>>> --- > >>>> 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 hm... I'm pretty sure there are plenty: $ sudo cat /sys/kernel/debug/tracing/available_filter_functions | grep -v __ftrace_invalid_address | sort | uniq -c | sort -nr | head -n10 14 type_show 12 init_once 11 modalias_show 8 event_show 7 name_show 6 enabled_show 5 version_show 5 size_show 5 offset_show 5 numa_node_show > > >> > >> 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. It is, because currently when we parse kallsyms we remember function addresses, which are unique. We don't rely on kernel string -> addr resolution. > > > > > 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; > >>>> }; > >>>> > > > > [...]
