diff mbox series

[v6,RESEND] libbpf: kprobe.multi: Filter with available_filter_functions

Message ID 20230526155026.1419390-1-liu.yun@linux.dev (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [v6,RESEND] libbpf: kprobe.multi: Filter with available_filter_functions | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for s390x 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-6 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on s390x 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-12 success Logs for test_progs on s390x 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-16 success Logs for test_progs_no_alu32 on s390x 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-24 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on s390x 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
netdev/tree_selection success Not a local patch

Commit Message

Jackie Liu May 26, 2023, 3:50 p.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.

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 | 100 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 93 insertions(+), 7 deletions(-)

Comments

Jiri Olsa May 29, 2023, 1:23 p.m. UTC | #1 
On Fri, May 26, 2023 at 11:50:26PM +0800, 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.
> 
> 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 | 100 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 93 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index ad1ec893b41b..0914b7e98e30 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10106,6 +10106,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)
>  {
> @@ -10417,13 +10423,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;
> @@ -10440,6 +10447,77 @@ 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_kprobe_cb_t)(const char *sym_name, void *ctx);
> +
> +static int
> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
> +{
> +	char sym_name[256];
> +	FILE *f;
> +	int ret, err = 0;
> +	const char *available_path = tracefs_available_filter_functions();
> +
> +	f = fopen(available_path, "r");
> +	if (!f) {
> +		err = -errno;
> +		pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
> +			available_path);
> +		return err;
> +	}
> +
> +	while (true) {
> +		ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
> +		if (ret == EOF && feof(f))
> +			break;
> +		if (ret != 1) {
> +			pr_warn("failed to read available kprobe entry: %d\n",
> +				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)
> +{
> +	while (res->syms && res->cnt)
> +		free((char *)res->syms[--res->cnt]);
> +
> +	free(res->syms);
> +	free(res->addrs);

I think we also need to zero the res->syms pointer, so the final
kprobe_multi_resolve_free won't try to release it again
perhaps use zfree for both syms and addrs

other than this it looks ok to me:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> +	/* reset cap to zero, when fallback */
> +	res->cap = 0;
> +}
> +
>  struct bpf_link *
>  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>  				      const char *pattern,
> @@ -10476,13 +10554,21 @@ 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_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
> +						     &res);
> +		if (err) {
> +			/* fallback to kallsyms */
> +			kprobe_multi_resolve_free(&res);
> +			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 +10597,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
>
Jackie Liu May 29, 2023, 4:03 p.m. UTC | #2 
Hi Jiri.


May 29, 2023 9:23 PM, "Jiri Olsa" <olsajiri@gmail.com> 写到:

> On Fri, May 26, 2023 at 11:50:26PM +0800, 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.
>> 
>> 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 | 100 ++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 93 insertions(+), 7 deletions(-)
>> 
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index ad1ec893b41b..0914b7e98e30 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -10106,6 +10106,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)
>> {
>> @@ -10417,13 +10423,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;
>> @@ -10440,6 +10447,77 @@ 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_kprobe_cb_t)(const char *sym_name, void *ctx);
>> +
>> +static int
>> +libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
>> +{
>> + char sym_name[256];
>> + FILE *f;
>> + int ret, err = 0;
>> + const char *available_path = tracefs_available_filter_functions();
>> +
>> + f = fopen(available_path, "r");
>> + if (!f) {
>> + err = -errno;
>> + pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
>> + available_path);
>> + return err;
>> + }
>> +
>> + while (true) {
>> + ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
>> + if (ret == EOF && feof(f))
>> + break;
>> + if (ret != 1) {
>> + pr_warn("failed to read available kprobe entry: %d\n",
>> + 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)
>> +{
>> + while (res->syms && res->cnt)
>> + free((char *)res->syms[--res->cnt]);
>> +
>> + free(res->syms);
>> + free(res->addrs);
> 
> I think we also need to zero the res->syms pointer, so the final
> kprobe_multi_resolve_free won't try to release it again
> perhaps use zfree for both syms and addrs
> 
> other than this it looks ok to me:
> 
> Acked-by: Jiri Olsa <jolsa@kernel.org>

Thank you for your patient guidance and best wishes to you.

You are right, I finally decided to initialize all related variables to 0. We may indeed guarantee 
that res->cnt will become 0 now, but we will change the logic in the future to miss this.

Thanks again.
diff mbox series

Patch

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ad1ec893b41b..0914b7e98e30 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10106,6 +10106,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)
 {
@@ -10417,13 +10423,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;
@@ -10440,6 +10447,77 @@  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_kprobe_cb_t)(const char *sym_name, void *ctx);
+
+static int
+libbpf_available_kprobes_parse(available_kprobe_cb_t cb, void *ctx)
+{
+	char sym_name[256];
+	FILE *f;
+	int ret, err = 0;
+	const char *available_path = tracefs_available_filter_functions();
+
+	f = fopen(available_path, "r");
+	if (!f) {
+		err = -errno;
+		pr_warn("failed to open %s, fallback to /proc/kallsyms.\n",
+			available_path);
+		return err;
+	}
+
+	while (true) {
+		ret = fscanf(f, "%255s%*[^\n]\n", sym_name);
+		if (ret == EOF && feof(f))
+			break;
+		if (ret != 1) {
+			pr_warn("failed to read available kprobe entry: %d\n",
+				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)
+{
+	while (res->syms && res->cnt)
+		free((char *)res->syms[--res->cnt]);
+
+	free(res->syms);
+	free(res->addrs);
+	/* reset cap to zero, when fallback */
+	res->cap = 0;
+}
+
 struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 				      const char *pattern,
@@ -10476,13 +10554,21 @@  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_available_kprobes_parse(ftrace_resolve_kprobe_multi_cb,
+						     &res);
+		if (err) {
+			/* fallback to kallsyms */
+			kprobe_multi_resolve_free(&res);
+			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 +10597,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);
 }