Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function
@ 2021-07-21 21:58 Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe Jiri Olsa
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-07-21 21:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Masami Hiramatsu, Alan Maguire

hi,
making bpf_program__attach_kprobe_opts function exported,
and other fixes suggested by Andrii in recent review [1][2].

thanks,
jirka


[1] https://lore.kernel.org/bpf/CAEf4BzYELMgTv_RhW7qWNgOYc_mCyh8-VX0FUYabi_TU3OiGKw@mail.gmail.com/
[2] https://lore.kernel.org/bpf/CAEf4Bzbk-nyenpc86jtEShset_ZSkapvpy3fG2gYKZEOY7uAQg@mail.gmail.com/

---
Jiri Olsa (3):
      libbpf: Fix func leak in attach_kprobe
      libbpf: Allow decimal offset for kprobes
      libbpf: Export bpf_program__attach_kprobe_opts function

 tools/lib/bpf/libbpf.c                                    | 34 +++++++++++++++++++---------------
 tools/lib/bpf/libbpf.h                                    | 14 ++++++++++++++
 tools/lib/bpf/libbpf.map                                  |  1 +
 tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c |  2 ++
 tools/testing/selftests/bpf/progs/get_func_ip_test.c      | 11 +++++++++++
 5 files changed, 47 insertions(+), 15 deletions(-)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe
  2021-07-21 21:58 [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
@ 2021-07-21 21:58 ` Jiri Olsa
  2021-07-22  7:15   ` Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 2/3] libbpf: Allow decimal offset for kprobes Jiri Olsa
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2021-07-21 21:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Andrii Nakryiko, netdev, bpf, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Masami Hiramatsu,
	Alan Maguire

Adding missing free for func pointer in attach_kprobe function.

Reported-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 4c153c379989..d46c2dd37be2 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10431,6 +10431,7 @@ static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec,
 		return libbpf_err_ptr(err);
 	}
 	if (opts.retprobe && offset != 0) {
+		free(func);
 		err = -EINVAL;
 		pr_warn("kretprobes do not support offset specification\n");
 		return libbpf_err_ptr(err);
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf-next 2/3] libbpf: Allow decimal offset for kprobes
  2021-07-21 21:58 [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe Jiri Olsa
@ 2021-07-21 21:58 ` Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
  2021-07-22 12:45 ` [PATCH bpf-next 0/3] " Alan Maguire
  3 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-07-21 21:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Andrii Nakryiko, netdev, bpf, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Masami Hiramatsu,
	Alan Maguire

Allow to specify decimal offset in SEC macro, like:
  SEC("kprobe/bpf_fentry_test7+5")

Adding selftest for that.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c                                |  2 +-
 .../selftests/bpf/prog_tests/get_func_ip_test.c       |  2 ++
 tools/testing/selftests/bpf/progs/get_func_ip_test.c  | 11 +++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index d46c2dd37be2..52f4f1d4f495 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10424,7 +10424,7 @@ static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec,
 	func_name = prog->sec_name + sec->len;
 	opts.retprobe = strcmp(sec->sec, "kretprobe/") == 0;
 
-	n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%lx", &func, &offset);
+	n = sscanf(func_name, "%m[a-zA-Z0-9_.]+%li", &func, &offset);
 	if (n < 1) {
 		err = -EINVAL;
 		pr_warn("kprobe name is invalid: %s\n", func_name);
diff --git a/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c b/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
index 088b3653610d..02a465f36d59 100644
--- a/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c
@@ -17,6 +17,7 @@ void test_get_func_ip_test(void)
 	 */
 #ifndef __x86_64__
 	bpf_program__set_autoload(skel->progs.test6, false);
+	bpf_program__set_autoload(skel->progs.test7, false);
 #endif
 
 	err = get_func_ip_test__load(skel);
@@ -46,6 +47,7 @@ void test_get_func_ip_test(void)
 	ASSERT_EQ(skel->bss->test5_result, 1, "test5_result");
 #ifdef __x86_64__
 	ASSERT_EQ(skel->bss->test6_result, 1, "test6_result");
+	ASSERT_EQ(skel->bss->test7_result, 1, "test7_result");
 #endif
 
 cleanup:
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c
index acd587b6e859..a587aeca5ae0 100644
--- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c
+++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c
@@ -11,6 +11,7 @@ extern const void bpf_fentry_test3 __ksym;
 extern const void bpf_fentry_test4 __ksym;
 extern const void bpf_modify_return_test __ksym;
 extern const void bpf_fentry_test6 __ksym;
+extern const void bpf_fentry_test7 __ksym;
 
 __u64 test1_result = 0;
 SEC("fentry/bpf_fentry_test1")
@@ -71,3 +72,13 @@ int test6(struct pt_regs *ctx)
 	test6_result = (const void *) addr == &bpf_fentry_test6 + 5;
 	return 0;
 }
+
+__u64 test7_result = 0;
+SEC("kprobe/bpf_fentry_test7+5")
+int test7(struct pt_regs *ctx)
+{
+	__u64 addr = bpf_get_func_ip(ctx);
+
+	test7_result = (const void *) addr == &bpf_fentry_test7 + 5;
+	return 0;
+}
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function
  2021-07-21 21:58 [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe Jiri Olsa
  2021-07-21 21:58 ` [PATCH bpf-next 2/3] libbpf: Allow decimal offset for kprobes Jiri Olsa
@ 2021-07-21 21:58 ` Jiri Olsa
  2021-07-23  3:06   ` Andrii Nakryiko
  2021-07-22 12:45 ` [PATCH bpf-next 0/3] " Alan Maguire
  3 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2021-07-21 21:58 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Andrii Nakryiko, netdev, bpf, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Masami Hiramatsu,
	Alan Maguire

Exporting bpf_program__attach_kprobe_opts function.

Renaming bpf_program_attach_kprobe_opts to bpf_kprobe_opts
and adding 'sz' field for forward/backward compatiblity.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c   | 31 +++++++++++++++++--------------
 tools/lib/bpf/libbpf.h   | 14 ++++++++++++++
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 52f4f1d4f495..63a6239d8569 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10366,25 +10366,28 @@ static int perf_event_open_probe(bool uprobe, bool retprobe, const char *name,
 	return pfd;
 }
 
-struct bpf_program_attach_kprobe_opts {
-	bool retprobe;
-	unsigned long offset;
-};
-
-static struct bpf_link*
+struct bpf_link*
 bpf_program__attach_kprobe_opts(struct bpf_program *prog,
 				const char *func_name,
-				struct bpf_program_attach_kprobe_opts *opts)
+				struct bpf_kprobe_opts *opts)
 {
 	char errmsg[STRERR_BUFSIZE];
 	struct bpf_link *link;
+	unsigned long offset;
+	bool retprobe;
 	int pfd, err;
 
-	pfd = perf_event_open_probe(false /* uprobe */, opts->retprobe, func_name,
-				    opts->offset, -1 /* pid */);
+	if (!OPTS_VALID(opts, bpf_kprobe_opts))
+		return libbpf_err_ptr(-EINVAL);
+
+	retprobe = OPTS_GET(opts, retprobe, false);
+	offset = OPTS_GET(opts, offset, 0);
+
+	pfd = perf_event_open_probe(false /* uprobe */, retprobe, func_name,
+				    offset, -1 /* pid */);
 	if (pfd < 0) {
 		pr_warn("prog '%s': failed to create %s '%s' perf event: %s\n",
-			prog->name, opts->retprobe ? "kretprobe" : "kprobe", func_name,
+			prog->name, retprobe ? "kretprobe" : "kprobe", func_name,
 			libbpf_strerror_r(pfd, errmsg, sizeof(errmsg)));
 		return libbpf_err_ptr(pfd);
 	}
@@ -10393,7 +10396,7 @@ bpf_program__attach_kprobe_opts(struct bpf_program *prog,
 	if (err) {
 		close(pfd);
 		pr_warn("prog '%s': failed to attach to %s '%s': %s\n",
-			prog->name, opts->retprobe ? "kretprobe" : "kprobe", func_name,
+			prog->name, retprobe ? "kretprobe" : "kprobe", func_name,
 			libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
 		return libbpf_err_ptr(err);
 	}
@@ -10404,9 +10407,9 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
 					    bool retprobe,
 					    const char *func_name)
 {
-	struct bpf_program_attach_kprobe_opts opts = {
+	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts,
 		.retprobe = retprobe,
-	};
+	);
 
 	return bpf_program__attach_kprobe_opts(prog, func_name, &opts);
 }
@@ -10414,7 +10417,7 @@ struct bpf_link *bpf_program__attach_kprobe(struct bpf_program *prog,
 static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec,
 				      struct bpf_program *prog)
 {
-	struct bpf_program_attach_kprobe_opts opts;
+	DECLARE_LIBBPF_OPTS(bpf_kprobe_opts, opts);
 	unsigned long offset = 0;
 	struct bpf_link *link;
 	const char *func_name;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 6b08c1023609..da4a63a1265d 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -104,6 +104,16 @@ struct bpf_object_open_opts {
 };
 #define bpf_object_open_opts__last_field btf_custom_path
 
+struct bpf_kprobe_opts {
+	/* size of this struct, for forward/backward compatiblity */
+	size_t sz;
+	/* kprobe is return probe */
+	bool retprobe;
+	/* function's offset to install kprobe to */
+	unsigned long offset;
+};
+#define bpf_kprobe_opts__last_field offset
+
 LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
 LIBBPF_API struct bpf_object *
 bpf_object__open_file(const char *path, const struct bpf_object_open_opts *opts);
@@ -249,6 +259,10 @@ bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
 LIBBPF_API struct bpf_link *
 bpf_program__attach_kprobe(struct bpf_program *prog, bool retprobe,
 			   const char *func_name);
+LIBBPF_API struct bpf_link*
+bpf_program__attach_kprobe_opts(struct bpf_program *prog,
+                                const char *func_name,
+                                struct bpf_kprobe_opts *opts);
 LIBBPF_API struct bpf_link *
 bpf_program__attach_uprobe(struct bpf_program *prog, bool retprobe,
 			   pid_t pid, const char *binary_path,
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 5bfc10722647..887d372a3f27 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -372,6 +372,7 @@ LIBBPF_0.5.0 {
 	global:
 		bpf_map__initial_value;
 		bpf_map_lookup_and_delete_elem_flags;
+		bpf_program__attach_kprobe_opts;
 		bpf_object__gen_loader;
 		btf_dump__dump_type_data;
 		libbpf_set_strict_mode;
-- 
2.31.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe
  2021-07-21 21:58 ` [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe Jiri Olsa
@ 2021-07-22  7:15   ` Jiri Olsa
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2021-07-22  7:15 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: Andrii Nakryiko, netdev, bpf, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Masami Hiramatsu,
	Alan Maguire

On Wed, Jul 21, 2021 at 11:58:08PM +0200, Jiri Olsa wrote:
> Adding missing free for func pointer in attach_kprobe function.
> 

and of course..

Fixes: a2488b5f483f ("libbpf: Allow specification of "kprobe/function+offset"")

jirka

> Reported-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 4c153c379989..d46c2dd37be2 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -10431,6 +10431,7 @@ static struct bpf_link *attach_kprobe(const struct bpf_sec_def *sec,
>  		return libbpf_err_ptr(err);
>  	}
>  	if (opts.retprobe && offset != 0) {
> +		free(func);
>  		err = -EINVAL;
>  		pr_warn("kretprobes do not support offset specification\n");
>  		return libbpf_err_ptr(err);
> -- 
> 2.31.1
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function
  2021-07-21 21:58 [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
                   ` (2 preceding siblings ...)
  2021-07-21 21:58 ` [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
@ 2021-07-22 12:45 ` Alan Maguire
  3 siblings, 0 replies; 7+ messages in thread
From: Alan Maguire @ 2021-07-22 12:45 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, netdev,
	bpf, Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Masami Hiramatsu, Alan Maguire


On Wed, 21 Jul 2021, Jiri Olsa wrote:

> hi,
> making bpf_program__attach_kprobe_opts function exported,
> and other fixes suggested by Andrii in recent review [1][2].
>

Looks great! Feel free to add 

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Tested-by: Alan Maguire <alan.maguire@oracle.com>

> thanks,
> jirka
> 
> 
> [1] https://lore.kernel.org/bpf/CAEf4BzYELMgTv_RhW7qWNgOYc_mCyh8-VX0FUYabi_TU3OiGKw@mail.gmail.com/
> [2] https://lore.kernel.org/bpf/CAEf4Bzbk-nyenpc86jtEShset_ZSkapvpy3fG2gYKZEOY7uAQg@mail.gmail.com/
> 
> ---
> Jiri Olsa (3):
>       libbpf: Fix func leak in attach_kprobe
>       libbpf: Allow decimal offset for kprobes
>       libbpf: Export bpf_program__attach_kprobe_opts function
> 
>  tools/lib/bpf/libbpf.c                                    | 34 +++++++++++++++++++---------------
>  tools/lib/bpf/libbpf.h                                    | 14 ++++++++++++++
>  tools/lib/bpf/libbpf.map                                  |  1 +
>  tools/testing/selftests/bpf/prog_tests/get_func_ip_test.c |  2 ++
>  tools/testing/selftests/bpf/progs/get_func_ip_test.c      | 11 +++++++++++
>  5 files changed, 47 insertions(+), 15 deletions(-)
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function
  2021-07-21 21:58 ` [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
@ 2021-07-23  3:06   ` Andrii Nakryiko
  0 siblings, 0 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2021-07-23  3:06 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Andrii Nakryiko, Networking, bpf, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Masami Hiramatsu,
	Alan Maguire

On Wed, Jul 21, 2021 at 2:58 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> Exporting bpf_program__attach_kprobe_opts function.

I've updated all commits to use more customary imperative language
("export" instead of "exporting", "add" instead of "adding"). Also
fixed a few styling issues and changed the order of kprobe_opts field
(offset first, bool retprobe next), so that we don't have unnecessary
internal padding.

Pushed to bpf-next, thanks!

>
> Renaming bpf_program_attach_kprobe_opts to bpf_kprobe_opts
> and adding 'sz' field for forward/backward compatiblity.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c   | 31 +++++++++++++++++--------------
>  tools/lib/bpf/libbpf.h   | 14 ++++++++++++++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 32 insertions(+), 14 deletions(-)
>

[...]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-23  3:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 21:58 [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
2021-07-21 21:58 ` [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe Jiri Olsa
2021-07-22  7:15   ` Jiri Olsa
2021-07-21 21:58 ` [PATCH bpf-next 2/3] libbpf: Allow decimal offset for kprobes Jiri Olsa
2021-07-21 21:58 ` [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function Jiri Olsa
2021-07-23  3:06   ` Andrii Nakryiko
2021-07-22 12:45 ` [PATCH bpf-next 0/3] " Alan Maguire

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).