Re: [PATCH v6 1/2] libbpf: kprobe.multi: cross filter using available_filter_functions and kallsyms

[Jackie Liu <liu.yun@xxxxxxxxx>]

在 2023/7/5 17:01, Daniel Borkmann 写道:
> On 7/5/23 5:34 AM, Jackie Liu 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>
> > ---
> >   v5->v6: fix crash by not init "const char *syms"
> >   v4->v5: simplified code
> >
> >   tools/lib/bpf/libbpf.c | 106 +++++++++++++++++++++++++++++++++++++----
> >   1 file changed, 96 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 214f828ece6b..3b5a12ca47bf 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,78 @@ 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;

> All your jumps to cleanup here and below leak f.

Yes, thank you. I miss that when simplifying the code.

--
Jackie

> > +        }
> > +
> > +        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;
> > +    }
> > +    fclose(f);
> > +
> > +    /* not found entry, return direct */
> > +    if (!cnt)
> > +        return -ENOENT;
> > +
> > +    /* 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);
> > +
> > +    return err;
> > +}
> > +
> >   struct bpf_link *
> >   bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >                         const char *pattern,
> > @@ -10594,13 +10684,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;
> >       }