Message ID | 20230525102747.68708-1-liu.yun@linux.dev (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | [v4] libbpf: kprobe.multi: Filter with available_filter_functions | expand |
Context | Check | Description |
---|---|---|
bpf/vmtest-bpf-next-PR | success | PR summary |
netdev/tree_selection | success | Not a local patch |
bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for ShellCheck |
bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for set-matrix |
bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for build for aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for build for x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-5 | success | Logs for build for x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for build for s390x with gcc |
bpf/vmtest-bpf-next-VM_Test-7 | success | Logs for test_maps on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-25 | success | Logs for test_verifier on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-9 | success | Logs for test_maps on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-10 | success | Logs for test_maps on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-11 | success | Logs for test_progs on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-13 | success | Logs for test_progs on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-14 | success | Logs for test_progs on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-15 | success | Logs for test_progs_no_alu32 on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-17 | success | Logs for test_progs_no_alu32 on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-19 | success | Logs for test_progs_no_alu32_parallel on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-20 | success | Logs for test_progs_no_alu32_parallel on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-21 | success | Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-22 | success | Logs for test_progs_parallel on aarch64 with gcc |
bpf/vmtest-bpf-next-VM_Test-23 | success | Logs for test_progs_parallel on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-24 | success | Logs for test_progs_parallel on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-27 | success | Logs for test_verifier on x86_64 with gcc |
bpf/vmtest-bpf-next-VM_Test-28 | success | Logs for test_verifier on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-29 | success | Logs for veristat |
bpf/vmtest-bpf-next-VM_Test-18 | fail | Logs for test_progs_no_alu32 on x86_64 with llvm-16 |
bpf/vmtest-bpf-next-VM_Test-16 | success | Logs for test_progs_no_alu32 on s390x with gcc |
bpf/vmtest-bpf-next-VM_Test-26 | success | Logs for test_verifier on s390x with gcc |
bpf/vmtest-bpf-next-VM_Test-12 | success | Logs for test_progs on s390x with gcc |
bpf/vmtest-bpf-next-VM_Test-8 | success | Logs for test_maps on s390x with gcc |
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? > 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; > }; > > static int > -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > - const char *sym_name, void *ctx) > +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > + const char *sym_name, void *ctx) > { > struct kprobe_multi_resolve *res = ctx; > int err; > @@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > if (!glob_match(sym_name, res->pattern)) > return 0; > > - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long), > - res->cnt + 1); > + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, > + sizeof(unsigned long), res->cnt + 1); > if (err) > return err; > > @@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > return 0; > } > > +static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx) > +{ > + struct kprobe_multi_resolve *res = ctx; > + int err; > + char *name; > + > + if (!glob_match(sym_name, res->pattern)) > + return 0; > + > + err = libbpf_ensure_mem((void **) &res->syms, &res->cap, > + sizeof(const char *), res->cnt + 1); > + if (err) > + return err; > + > + name = strdup(sym_name); > + if (!name) > + return errno; -errno > + > + res->syms[res->cnt++] = name; > + return 0; > +} > + > +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx); quite mouthful, maybe just "available_kprobe_cb_t"? "filters" terminology isn't common within libbpf and BPF tracing in general > + > +static int > +libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx) let's call it "libbpf_available_kprobes_parse" ? > +{ > + char sym_name[256]; > + FILE *f; > + int ret, err = 0; > + > + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r"); we need to check between DEBUGFS and TRACEFS, let's do something like tracefs_kprobe_events() > + if (!f) { > + pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n"); > + return -EINVAL; preserve errno, just like libbpf_kallsyms_parse > + } > + > + while (true) { > + ret = fscanf(f, "%s%*[^\n]\n", sym_name); %255s, similar to libbpf_kallsyms_probe. You have precedent code that does parsing like this, please stick to the same approaches > + if (ret == EOF && feof(f)) > + break; > + if (ret != 1) { > + pr_warn("failed to read available_filter_functions entry: %d\n", s/available_filter_functions/kprobe/ > + ret); err = -EINVAL > + break; > + } > + > + err = cb(sym_name, ctx); > + if (err) > + break; > + } > + > + fclose(f); > + return err; > +} > + > +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res) > +{ > + if (res->syms) { > + while (res->cnt) > + free((char *)res->syms[--res->cnt]); > + free(res->syms); > + } else { > + free(res->addrs); there is no need to assume that res->addrs will be null, let's free it unconditionally. free() handles NULL just fine > + } > +} > + > struct bpf_link * > bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, > const char *pattern, > @@ -10476,13 +10544,19 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, > return libbpf_err_ptr(-EINVAL); > > if (pattern) { > - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res); > - if (err) > - goto error; > + err = libbpf_ftrace_parse(ftrace_resolve_kprobe_multi_cb, &res); > + if (err) { > + /* fallback to kallsyms */ > + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, > + &res); > + if (err) > + goto error; > + } > if (!res.cnt) { > err = -ENOENT; > goto error; > } > + syms = res.syms; > addrs = res.addrs; > cnt = res.cnt; > } > @@ -10511,12 +10585,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, > goto error; > } > link->fd = link_fd; > - free(res.addrs); > + kprobe_multi_resolve_free(&res); > return link; > > error: > free(link); > - free(res.addrs); > + kprobe_multi_resolve_free(&res); > return libbpf_err_ptr(err); > } > > -- > 2.25.1 > >
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? 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. 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. > >> 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; >> }; >> >> static int >> -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, >> - const char *sym_name, void *ctx) >> +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, >> + const char *sym_name, void *ctx) >> { >> struct kprobe_multi_resolve *res = ctx; >> int err; >> @@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, >> if (!glob_match(sym_name, res->pattern)) >> return 0; >> >> - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long), >> - res->cnt + 1); >> + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, >> + sizeof(unsigned long), res->cnt + 1); >> if (err) >> return err; >> >> @@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, >> return 0; >> } >> >> +static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx) >> +{ >> + struct kprobe_multi_resolve *res = ctx; >> + int err; >> + char *name; >> + >> + if (!glob_match(sym_name, res->pattern)) >> + return 0; >> + >> + err = libbpf_ensure_mem((void **) &res->syms, &res->cap, >> + sizeof(const char *), res->cnt + 1); >> + if (err) >> + return err; >> + >> + name = strdup(sym_name); >> + if (!name) >> + return errno; > > -errno > >> + >> + res->syms[res->cnt++] = name; >> + return 0; >> +} >> + >> +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx); > > quite mouthful, maybe just "available_kprobe_cb_t"? "filters" > terminology isn't common within libbpf and BPF tracing in general > >> + >> +static int >> +libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx) > > let's call it "libbpf_available_kprobes_parse" ? > >> +{ >> + char sym_name[256]; >> + FILE *f; >> + int ret, err = 0; >> + >> + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r"); > > we need to check between DEBUGFS and TRACEFS, let's do something like > tracefs_kprobe_events() Got. > >> + if (!f) { >> + pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n"); >> + return -EINVAL; > > preserve errno, just like libbpf_kallsyms_parse > >> + } >> + >> + while (true) { >> + ret = fscanf(f, "%s%*[^\n]\n", sym_name); > > %255s, similar to libbpf_kallsyms_probe. You have precedent code that > does parsing like this, please stick to the same approaches > >> + if (ret == EOF && feof(f)) >> + break; >> + if (ret != 1) { >> + pr_warn("failed to read available_filter_functions entry: %d\n", > > s/available_filter_functions/kprobe/ > >> + ret); > > err = -EINVAL > >> + break; >> + } >> + >> + err = cb(sym_name, ctx); >> + if (err) >> + break; >> + } >> + >> + fclose(f); >> + return err; >> +} >> + >> +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res) >> +{ >> + if (res->syms) { >> + while (res->cnt) >> + free((char *)res->syms[--res->cnt]); >> + free(res->syms); >> + } else { >> + free(res->addrs); > > there is no need to assume that res->addrs will be null, let's free it > unconditionally. free() handles NULL just fine Yes.
On Fri, May 26, 2023 at 09:38:15AM +0800, Jackie Liu 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? > > 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. it won't get through the kprobe_multi symbols resolve code, because we check that the number of resolved addresses matches the number of provided symbols also found test bug (hunk#2) when checking on that (hunk#1) ;-) jirka --- diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c index 2173c4bb555e..e78362354bd3 100644 --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c @@ -124,7 +124,7 @@ static void test_link_api_syms(void) "bpf_fentry_test5", "bpf_fentry_test6", "bpf_fentry_test7", - "bpf_fentry_test8", + "bpf_fentry_test7", }; opts.kprobe_multi.syms = syms; @@ -477,9 +477,9 @@ void test_kprobe_multi_test(void) if (test__start_subtest("skel_api")) test_skel_api(); if (test__start_subtest("link_api_addrs")) - test_link_api_syms(); - if (test__start_subtest("link_api_syms")) test_link_api_addrs(); + if (test__start_subtest("link_api_syms")) + test_link_api_syms(); if (test__start_subtest("attach_api_pattern")) test_attach_api_pattern(); if (test__start_subtest("attach_api_addrs"))
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? > > 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. So, it worries me that we'll switch from kallsyms to available_filters by default, because that introduces new failure modes. Either way, let's add a selftest that uses a duplicate function name and see what happens? > 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). So it's a regression to switch to that without taking any other precautions. > > > > >> 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; > >> }; > >> [...]
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.
On Tue, Jun 6, 2023 at 11:01 PM Jackie Liu <liu.yun@linux.dev> wrote: > > 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 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; > >>>> }; > >>>> > > > > [...]
On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote: > 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? > > > > 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. right, as I wrote in here [1] it will fail [1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/ > > So, it worries me that we'll switch from kallsyms to available_filters > by default, because that introduces new failure modes. we did not care about duplicate with kallsyms because we used addresses, and I think with duplicate addresss the kprobe_multi link will probably attach (need to check) while with duplicate symbols it won't.. perhaps we could make sure we don't pass duplicate symbols? we do the kprobe_multi bench with symbol names read from available_filter_functions and we filter out duplicates jirka > > Either way, let's add a selftest that uses a duplicate function name > and see what happens? > > > 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). > > So it's a regression to switch to that without taking any other precautions. > > > > > > > > >> 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; > > >> }; > > >> > > [...]
On Wed, Jun 7, 2023 at 4:22 PM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote: > > 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? > > > > > > 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. > > right, as I wrote in here [1] it will fail > > [1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/ > > > > > So, it worries me that we'll switch from kallsyms to available_filters > > by default, because that introduces new failure modes. > > we did not care about duplicate with kallsyms because we used addresses, > and I think with duplicate addresss the kprobe_multi link will probably > attach (need to check) while with duplicate symbols it won't.. > > perhaps we could make sure we don't pass duplicate symbols? I think we have to stick to kallsyms and addresses. What if I actually want to attach to all instances of type_show? We should take into account available_filter_functions, but still use addresses from kallsyms. I'd also advocate working on having an available_filter_functions version reporting not just function names, but also its associated address. That would actually eliminate the need for kallsyms. I chatted with Steven Rostedt about this at the last LSF/MM/BPF conference, and I think we both agreed that we both a) have all the information in the kernel to implement this and b) it's a good idea to expose all that to user space. For backwards compat reasons it will have to be a separate file, but it's generated on the fly, so it's not a big deal in terms of resource usage. > > we do the kprobe_multi bench with symbol names read from available_filter_functions > and we filter out duplicates > > jirka > > > > > Either way, let's add a selftest that uses a duplicate function name > > and see what happens? > > > > > 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). > > > > So it's a regression to switch to that without taking any other precautions. > > > > > > > > > > > > >> 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; > > > >> }; > > > >> > > > > [...]
在 2023/6/8 08:00, Andrii Nakryiko 写道: > On Wed, Jun 7, 2023 at 4:22 PM Jiri Olsa <olsajiri@gmail.com> wrote: >> >> On Fri, Jun 02, 2023 at 10:27:31AM -0700, Andrii Nakryiko wrote: >>> 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? >>>> >>>> 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. >> >> right, as I wrote in here [1] it will fail >> >> [1] https://lore.kernel.org/bpf/ZHB0xNEbjmwHv18d@krava/ >> >>> >>> So, it worries me that we'll switch from kallsyms to available_filters >>> by default, because that introduces new failure modes. >> >> we did not care about duplicate with kallsyms because we used addresses, >> and I think with duplicate addresss the kprobe_multi link will probably >> attach (need to check) while with duplicate symbols it won't.. >> >> perhaps we could make sure we don't pass duplicate symbols? > > I think we have to stick to kallsyms and addresses. What if I actually > want to attach to all instances of type_show? We should take into > account available_filter_functions, but still use addresses from > kallsyms. > > I'd also advocate working on having an available_filter_functions > version reporting not just function names, but also its associated > address. That would actually eliminate the need for kallsyms. > > I chatted with Steven Rostedt about this at the last LSF/MM/BPF > conference, and I think we both agreed that we both a) have all the > information in the kernel to implement this and b) it's a good idea to > expose all that to user space. For backwards compat reasons it will > have to be a separate file, but it's generated on the fly, so it's not > a big deal in terms of resource usage. Yes, I noticed that the latest version of the kernel has added touched_functions and enabled_functions, are they? I'm not sure. Perhaps we can wait for such an interface to appear before directly switching to that interface, and then submit this patch again.
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; }; static int -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, - const char *sym_name, void *ctx) +kallsyms_resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, + const char *sym_name, void *ctx) { struct kprobe_multi_resolve *res = ctx; int err; @@ -10431,8 +10432,8 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, if (!glob_match(sym_name, res->pattern)) return 0; - err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long), - res->cnt + 1); + err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, + sizeof(unsigned long), res->cnt + 1); if (err) return err; @@ -10440,6 +10441,73 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, return 0; } +static int ftrace_resolve_kprobe_multi_cb(const char *sym_name, void *ctx) +{ + struct kprobe_multi_resolve *res = ctx; + int err; + char *name; + + if (!glob_match(sym_name, res->pattern)) + return 0; + + err = libbpf_ensure_mem((void **) &res->syms, &res->cap, + sizeof(const char *), res->cnt + 1); + if (err) + return err; + + name = strdup(sym_name); + if (!name) + return errno; + + res->syms[res->cnt++] = name; + return 0; +} + +typedef int (*available_filter_functions_cb_t)(const char *sym_name, void *ctx); + +static int +libbpf_ftrace_parse(available_filter_functions_cb_t cb, void *ctx) +{ + char sym_name[256]; + FILE *f; + int ret, err = 0; + + f = fopen("/sys/kernel/debug/tracing/available_filter_functions", "r"); + if (!f) { + pr_warn("failed to open available_filter_functions, fallback to /proc/kallsyms.\n"); + return -EINVAL; + } + + while (true) { + ret = fscanf(f, "%s%*[^\n]\n", sym_name); + if (ret == EOF && feof(f)) + break; + if (ret != 1) { + pr_warn("failed to read available_filter_functions entry: %d\n", + ret); + break; + } + + err = cb(sym_name, ctx); + if (err) + break; + } + + fclose(f); + return err; +} + +static void kprobe_multi_resolve_free(struct kprobe_multi_resolve *res) +{ + if (res->syms) { + while (res->cnt) + free((char *)res->syms[--res->cnt]); + free(res->syms); + } else { + free(res->addrs); + } +} + struct bpf_link * bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, const char *pattern, @@ -10476,13 +10544,19 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, return libbpf_err_ptr(-EINVAL); if (pattern) { - err = libbpf_kallsyms_parse(resolve_kprobe_multi_cb, &res); - if (err) - goto error; + err = libbpf_ftrace_parse(ftrace_resolve_kprobe_multi_cb, &res); + if (err) { + /* fallback to kallsyms */ + err = libbpf_kallsyms_parse(kallsyms_resolve_kprobe_multi_cb, + &res); + if (err) + goto error; + } if (!res.cnt) { err = -ENOENT; goto error; } + syms = res.syms; addrs = res.addrs; cnt = res.cnt; } @@ -10511,12 +10585,12 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, goto error; } link->fd = link_fd; - free(res.addrs); + kprobe_multi_resolve_free(&res); return link; error: free(link); - free(res.addrs); + kprobe_multi_resolve_free(&res); return libbpf_err_ptr(err); }