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

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

在 2023/7/5 17:01, Daniel Borkmann 写道:
> On 7/5/23 5:34 AM, Jackie Liu 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.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: 
>> https://lore.kernel.org/oe-kbuild-all/202307030355.TdXOHklM-lkp@intel.com/
>> Suggested-by: Jiri Olsa <jolsa@kernel.org>
>> Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>> ---
>>   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;
>>       }
>>
>