[PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function

[PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function

[Jiri Olsa]

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(-)

[PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe

[Jiri Olsa]

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

Re: [PATCH bpf-next 1/3] libbpf: Fix func leak in attach_kprobe

[Jiri Olsa]

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
> 

[PATCH bpf-next 2/3] libbpf: Allow decimal offset for kprobes

[Jiri Olsa]

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

[PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function

[Jiri Olsa]

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

Re: [PATCH bpf-next 3/3] libbpf: Export bpf_program__attach_kprobe_opts function

[Andrii Nakryiko]

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(-)
>

[...]

Re: [PATCH bpf-next 0/3] libbpf: Export bpf_program__attach_kprobe_opts function

[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(-)
> 
>