On Wed, Jul 5, 2023 at 2:12 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. > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > Closes: https://lore.kernel.org/oe-kbuild-all/202307030355.TdXOHklM-lkp@xxxxxxxxx/ > Suggested-by: Jiri Olsa <jolsa@xxxxxxxxxx> > Suggested-by: Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> > Signed-off-by: Jackie Liu <liuyun01@xxxxxxxxxx> > --- > v6->v7: fix leak "FILE *f" > v5->v6: fix crash by not init "const char *syms" > v4->v5: simplified code > > tools/lib/bpf/libbpf.c | 107 +++++++++++++++++++++++++++++++++++++---- > 1 file changed, 97 insertions(+), 10 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 214f828ece6b..af7e224ba4ca 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -10224,6 +10224,12 @@ static const char *tracefs_uprobe_events(void) > return use_debugfs() ? DEBUGFS"/uprobe_events" : TRACEFS"/uprobe_events"; > } > > +static const char *tracefs_available_filter_functions(void) > +{ > + return use_debugfs() ? DEBUGFS"/available_filter_functions" : > + TRACEFS"/available_filter_functions"; > +} > + > static void gen_kprobe_legacy_event_name(char *buf, size_t buf_sz, > const char *kfunc_name, size_t offset) > { > @@ -10539,14 +10545,26 @@ struct kprobe_multi_resolve { > size_t cnt; > }; > > -static int > -resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > - const char *sym_name, void *ctx) > +static int avail_compare_function(const void *a, const void *b) > +{ > + return strcmp(*(const char **)a, *(const char **)b); > +} > + > +struct avail_kallsyms_data { > + const char **syms; > + size_t cnt; > + struct kprobe_multi_resolve *res; > +}; > + > +static int avail_kallsyms_cb(unsigned long long sym_addr, char sym_type, > + const char *sym_name, void *ctx) > { > - struct kprobe_multi_resolve *res = ctx; > + struct avail_kallsyms_data *data = ctx; > + struct kprobe_multi_resolve *res = data->res; > int err; > > - if (!glob_match(sym_name, res->pattern)) > + if (!bsearch(&sym_name, data->syms, data->cnt, sizeof(void *), > + avail_compare_function)) > return 0; > > err = libbpf_ensure_mem((void **) &res->addrs, &res->cap, sizeof(unsigned long), > @@ -10558,6 +10576,79 @@ resolve_kprobe_multi_cb(unsigned long long sym_addr, char sym_type, > return 0; > } > > +static int libbpf_available_kallsyms_parse(struct kprobe_multi_resolve *res) > +{ > + struct avail_kallsyms_data data; > + char sym_name[500]; > + const char *available_functions_file = tracefs_available_filter_functions(); > + FILE *f; > + int err = 0, ret, i; > + const char **syms = NULL; > + size_t cap = 0, cnt = 0; > + > + f = fopen(available_functions_file, "r"); > + if (!f) { > + err = -errno; > + pr_warn("failed to open %s\n", available_functions_file); > + return err; > + } > + > + while (true) { > + char *name; > + > + ret = fscanf(f, "%499s%*[^\n]\n", sym_name); > + if (ret == EOF && feof(f)) > + break; > + > + if (ret != 1) { > + pr_warn("failed to read available function file entry: %d\n", > + ret); > + err = -EINVAL; > + goto cleanup; > + } > + > + if (!glob_match(sym_name, res->pattern)) > + continue; > + > + err = libbpf_ensure_mem((void **)&syms, &cap, sizeof(void *), > + cnt + 1); > + if (err) > + goto cleanup; > + > + name = strdup(sym_name); > + if (!name) { > + err = -errno; > + goto cleanup; > + } > + > + syms[cnt++] = name; > + } > + /* not found entry, return direct */ > + if (!cnt) { > + fclose(f); > + return -ENOENT; this leaks syms, you should have done `err = -ENOENT; goto cleanup;` here to keep it simple and consistent. I fixed this up, did a few more small cosmetic changes, and applied to bpf-next, thanks. > + } > + > + /* sort available functions */ > + qsort(syms, cnt, sizeof(void *), avail_compare_function); > + > + data.syms = syms; > + data.res = res; > + data.cnt = cnt; > + libbpf_kallsyms_parse(avail_kallsyms_cb, &data); > + > + if (!res->cnt) > + err = -ENOENT; > + > +cleanup: > + for (i = 0; i < cnt; i++) > + free((char *)syms[i]); > + free(syms); > + > + fclose(f); > + return err; > +} > + > struct bpf_link * > bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog, > const char *pattern, > @@ -10594,13 +10685,9 @@ 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); > + err = libbpf_available_kallsyms_parse(&res); > if (err) > goto error; > - if (!res.cnt) { > - err = -ENOENT; > - goto error; > - } > addrs = res.addrs; > cnt = res.cnt; > } > -- > 2.25.1 > >
