diff mbox series

[v7,1/2] libbpf: kprobe.multi: cross filter using available_filter_functions and kallsyms

Message ID 20230705091209.3803873-1-liu.yun@linux.dev (mailing list archive)
State Accepted
Commit e5852934b64720cc594b2874e6c10c9d2a136477
Delegated to: BPF
Headers show
Series [v7,1/2] libbpf: kprobe.multi: cross filter using available_filter_functions and kallsyms | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-30 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-31 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-32 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-33 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-34 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-35 success Logs for veristat
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-24 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps 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-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-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-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-18 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
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-25 success Logs for test_verifier on aarch64 with gcc
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-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-3 pending Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-4 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-5 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-6 success Logs for set-matrix

Commit Message

Jackie Liu July 5, 2023, 9:12 a.m. UTC 
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>
---
 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(-)

Comments

Andrii Nakryiko July 6, 2023, 9:45 p.m. UTC | #1 
On Wed, Jul 5, 2023 at 2:12 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.
>
> 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>
> ---
>  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
>
>
patchwork-bot+netdevbpf@kernel.org July 6, 2023, 10 p.m. UTC | #2 
Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed,  5 Jul 2023 17:12:08 +0800 you 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.
> 
> [...]

Here is the summary with links:
  - [v7,1/2] libbpf: kprobe.multi: cross filter using available_filter_functions and kallsyms
    https://git.kernel.org/bpf/bpf-next/c/e5852934b647
  - [v7,2/2] libbpf: kprobe.multi: Filter with available_filter_functions_addrs
    https://git.kernel.org/bpf/bpf-next/c/0d28e1abb801

You are awesome, thank you!
diff mbox series

Patch

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;
+	}
+
+	/* 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;
 	}