[PATCH RFCv2 bpf-next 0/4] bpf: Introduce kprobe multi wrapper attach

[PATCH RFCv2 bpf-next 0/4] bpf: Introduce kprobe multi wrapper attach

[Jiri Olsa]

hi,
adding support to attach both entry and return bpf program on single
kprobe multi link. The first RFC patchset is in [0].

Having entry together with return probe for given function is common
use case for tetragon, bpftrace and most likely for others.

At the moment if we want both entry and return probe to execute bpf
program we need to create two (entry and return probe) links. The link
for return probe creates extra entry probe to setup the return probe.
The extra entry probe execution could be omitted if we had a way to
use just single link for both entry and exit probe.

In addition it's possible to control the execution of the return probe
with the return value of the entry bpf program. If the entry program
returns 0 the return probe is installed and executed, otherwise it's
skip.

v2 changes:
  - adding 'kprobe.wrapper' program that is called both for entry and
    exit probe [Andrii]
  - I kept the interface that adds new flag in attr.link_create.kprobe_multi.flags,
    because I don't see it breaking backward compatibility and it's much simpler
    than new attach type, I tried to discuss this in [1], but I'm ok to change
    that if it turns out to be a problem

thanks,
jirka


[0] https://lore.kernel.org/bpf/20240207153550.856536-1-jolsa@kernel.org/
[1] https://lore.kernel.org/bpf/ZdhmKQ1_vpCJTS_U@krava/
---
Jiri Olsa (4):
      bpf: Add support for kprobe multi wrapper attach
      bpf: Add bpf_kprobe_multi_is_return kfunc
      libbpf: Add support for kprobe multi wrapper attach
      selftests/bpf: Add kprobe multi wrapper test

 include/uapi/linux/bpf.h                                   |   3 ++-
 kernel/bpf/btf.c                                           |   3 +++
 kernel/trace/bpf_trace.c                                   |  71 +++++++++++++++++++++++++++++++++++++++++++++++++++-------
 tools/include/uapi/linux/bpf.h                             |   3 ++-
 tools/lib/bpf/libbpf.c                                     |  38 ++++++++++++++++++++++++++++---
 tools/lib/bpf/libbpf.h                                     |   4 +++-
 tools/testing/selftests/bpf/bpf_kfuncs.h                   |   2 ++
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c |  49 ++++++++++++++++++++++++++++++++++++++++
 tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c   | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 259 insertions(+), 14 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c

[PATCH RFCv2 bpf-next 1/4] bpf: Add support for kprobe multi wrapper attach

[Jiri Olsa]

Adding support to attach bpf program for entry and return probe
of the same function. This is common usecase and at the moment
it requires to create two kprobe multi links.

Adding new attr.link_create.kprobe_multi.flags value that instructs
kernel to attach link program to both entry and exit probe.

It's possible to control execution of the bpf program on return
probe simply by returning zero or non zero from the entry bpf
program execution to execute or not respectively the bpf program
on return probe.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 include/uapi/linux/bpf.h       |  3 ++-
 kernel/trace/bpf_trace.c       | 24 ++++++++++++++++++------
 tools/include/uapi/linux/bpf.h |  3 ++-
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d2e6c5fcec01..a430855c5bcd 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
  * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
  */
 enum {
-	BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
+	BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
+	BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
 };
 
 /* link_create.uprobe_multi.flags used in LINK_CREATE command for
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 241ddf5e3895..726a8c71f0da 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2587,6 +2587,7 @@ struct bpf_kprobe_multi_link {
 	u32 mods_cnt;
 	struct module **mods;
 	u32 flags;
+	bool is_wrapper;
 };
 
 struct bpf_kprobe_multi_run_ctx {
@@ -2826,10 +2827,11 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 			  void *data)
 {
 	struct bpf_kprobe_multi_link *link;
+	int err;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
-	return 0;
+	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
+	return link->is_wrapper ? err : 0;
 }
 
 static void
@@ -2967,6 +2969,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	void __user *uaddrs;
 	u64 *cookies = NULL;
 	void __user *usyms;
+	bool is_wrapper;
 	int err;
 
 	/* no support for 32bit archs yet */
@@ -2977,9 +2980,12 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 		return -EINVAL;
 
 	flags = attr->link_create.kprobe_multi.flags;
-	if (flags & ~BPF_F_KPROBE_MULTI_RETURN)
+	if (flags & ~(BPF_F_KPROBE_MULTI_RETURN|
+		      BPF_F_KPROBE_MULTI_WRAPPER))
 		return -EINVAL;
 
+	is_wrapper = flags & BPF_F_KPROBE_MULTI_WRAPPER;
+
 	uaddrs = u64_to_user_ptr(attr->link_create.kprobe_multi.addrs);
 	usyms = u64_to_user_ptr(attr->link_create.kprobe_multi.syms);
 	if (!!uaddrs == !!usyms)
@@ -3054,15 +3060,21 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	if (err)
 		goto error;
 
-	if (flags & BPF_F_KPROBE_MULTI_RETURN)
-		link->fp.exit_handler = kprobe_multi_link_exit_handler;
-	else
+	if (is_wrapper) {
 		link->fp.entry_handler = kprobe_multi_link_handler;
+		link->fp.exit_handler = kprobe_multi_link_exit_handler;
+	} else {
+		if (flags & BPF_F_KPROBE_MULTI_RETURN)
+			link->fp.exit_handler = kprobe_multi_link_exit_handler;
+		else
+			link->fp.entry_handler = kprobe_multi_link_handler;
+	}
 
 	link->addrs = addrs;
 	link->cookies = cookies;
 	link->cnt = cnt;
 	link->flags = flags;
+	link->is_wrapper = is_wrapper;
 
 	if (cookies) {
 		/*
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d2e6c5fcec01..a430855c5bcd 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
  * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
  */
 enum {
-	BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
+	BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
+	BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
 };
 
 /* link_create.uprobe_multi.flags used in LINK_CREATE command for
-- 
2.43.2

[PATCH RFCv2 bpf-next 2/4] bpf: Add bpf_kprobe_multi_is_return kfunc

[Jiri Olsa]

Adding bpf_kprobe_multi_is_return kfunc that returns true if the
bpf program is executed from the exit probe of the kprobe multi
link attached in wrapper mode. It returns false otherwise.

Adding new kprobe hook for kprobe program type.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/btf.c         |  3 +++
 kernel/trace/bpf_trace.c | 49 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6ff0bd1a91d5..5ab55720e881 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -218,6 +218,7 @@ enum btf_kfunc_hook {
 	BTF_KFUNC_HOOK_SOCKET_FILTER,
 	BTF_KFUNC_HOOK_LWT,
 	BTF_KFUNC_HOOK_NETFILTER,
+	BTF_KFUNC_HOOK_KPROBE,
 	BTF_KFUNC_HOOK_MAX,
 };
 
@@ -8112,6 +8113,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
 		return BTF_KFUNC_HOOK_LWT;
 	case BPF_PROG_TYPE_NETFILTER:
 		return BTF_KFUNC_HOOK_NETFILTER;
+	case BPF_PROG_TYPE_KPROBE:
+		return BTF_KFUNC_HOOK_KPROBE;
 	default:
 		return BTF_KFUNC_HOOK_MAX;
 	}
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 726a8c71f0da..cb801c94b8fa 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2594,6 +2594,7 @@ struct bpf_kprobe_multi_run_ctx {
 	struct bpf_run_ctx run_ctx;
 	struct bpf_kprobe_multi_link *link;
 	unsigned long entry_ip;
+	bool is_return;
 };
 
 struct user_syms {
@@ -2793,11 +2794,13 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 
 static int
 kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
-			   unsigned long entry_ip, struct pt_regs *regs)
+			   unsigned long entry_ip, struct pt_regs *regs,
+			   bool is_return)
 {
 	struct bpf_kprobe_multi_run_ctx run_ctx = {
 		.link = link,
 		.entry_ip = entry_ip,
+		.is_return = is_return,
 	};
 	struct bpf_run_ctx *old_run_ctx;
 	int err;
@@ -2830,7 +2833,7 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 	int err;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
+	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false);
 	return link->is_wrapper ? err : 0;
 }
 
@@ -2842,7 +2845,7 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
 	struct bpf_kprobe_multi_link *link;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
+	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true);
 }
 
 static int symbols_cmp_r(const void *a, const void *b, const void *priv)
@@ -3111,6 +3114,46 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	kvfree(cookies);
 	return err;
 }
+
+__bpf_kfunc_start_defs();
+
+__bpf_kfunc bool bpf_kprobe_multi_is_return(void)
+{
+	struct bpf_kprobe_multi_run_ctx *run_ctx;
+
+	run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
+	return run_ctx->is_return;
+}
+
+__bpf_kfunc_end_defs();
+
+BTF_KFUNCS_START(kprobe_multi_kfunc_set_ids)
+BTF_ID_FLAGS(func, bpf_kprobe_multi_is_return)
+BTF_KFUNCS_END(kprobe_multi_kfunc_set_ids)
+
+static int bpf_kprobe_multi_filter(const struct bpf_prog *prog, u32 kfunc_id)
+{
+	if (!btf_id_set8_contains(&kprobe_multi_kfunc_set_ids, kfunc_id))
+		return 0;
+
+	if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
+		return -EACCES;
+
+	return 0;
+}
+
+static const struct btf_kfunc_id_set bpf_kprobe_multi_kfunc_set = {
+	.owner = THIS_MODULE,
+	.set = &kprobe_multi_kfunc_set_ids,
+	.filter = bpf_kprobe_multi_filter,
+};
+
+static int __init bpf_kprobe_multi_kfuncs_init(void)
+{
+	return register_btf_kfunc_id_set(BPF_PROG_TYPE_KPROBE, &bpf_kprobe_multi_kfunc_set);
+}
+
+late_initcall(bpf_kprobe_multi_kfuncs_init);
 #else /* !CONFIG_FPROBE */
 int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
-- 
2.43.2

[PATCH RFCv2 bpf-next 3/4] libbpf: Add support for kprobe multi wrapper attach

[Jiri Olsa]

Adding support for specify wrapper mode in bpf_kprobe_multi_opts
struct object and new bpf program loader section:

 SEC("kprobe.wrapper/bpf_fentry_test*")

to load program as kprobe multi wrapper.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/lib/bpf/libbpf.c | 38 +++++++++++++++++++++++++++++++++++---
 tools/lib/bpf/libbpf.h |  4 +++-
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 01f407591a92..5416d784c857 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -8944,6 +8944,7 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
 static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
+static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
 static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
@@ -8960,6 +8961,7 @@ static const struct bpf_sec_def section_defs[] = {
 	SEC_DEF("uretprobe.s+",		KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
 	SEC_DEF("kprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
 	SEC_DEF("kretprobe.multi+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
+	SEC_DEF("kprobe.wrapper+",	KPROBE,	BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_wrapper),
 	SEC_DEF("uprobe.multi+",	KPROBE,	BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
 	SEC_DEF("uretprobe.multi+",	KPROBE,	BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
 	SEC_DEF("uprobe.multi.s+",	KPROBE,	BPF_TRACE_UPROBE_MULTI, SEC_SLEEPABLE, attach_uprobe_multi),
@@ -11034,7 +11036,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 	int err, link_fd, prog_fd;
 	const __u64 *cookies;
 	const char **syms;
-	bool retprobe;
+	__u32 flags = 0;
 	size_t cnt;
 
 	if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
@@ -11065,13 +11067,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
 		cnt = res.cnt;
 	}
 
-	retprobe = OPTS_GET(opts, retprobe, false);
+	if (OPTS_GET(opts, retprobe, false))
+		flags |= BPF_F_KPROBE_MULTI_RETURN;
+	if (OPTS_GET(opts, wrapper, false))
+		flags |= BPF_F_KPROBE_MULTI_WRAPPER;
 
 	lopts.kprobe_multi.syms = syms;
 	lopts.kprobe_multi.addrs = addrs;
 	lopts.kprobe_multi.cookies = cookies;
 	lopts.kprobe_multi.cnt = cnt;
-	lopts.kprobe_multi.flags = retprobe ? BPF_F_KPROBE_MULTI_RETURN : 0;
+	lopts.kprobe_multi.flags = flags;
 
 	link = calloc(1, sizeof(*link));
 	if (!link) {
@@ -11187,6 +11192,33 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
 	return libbpf_get_error(*link);
 }
 
+static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link)
+{
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts,
+		.wrapper = true,
+	);
+	const char *spec;
+	char *pattern;
+	int n;
+
+	*link = NULL;
+
+	/* no auto-attach for SEC("kprobe.wrapper") */
+	if (strcmp(prog->sec_name, "kprobe.wrapper") == 0)
+		return 0;
+
+	spec = prog->sec_name + sizeof("kprobe.wrapper/") - 1;
+	n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
+	if (n < 1) {
+		pr_warn("kprobe wrapper pattern is invalid: %s\n", pattern);
+		return -EINVAL;
+	}
+
+	*link = bpf_program__attach_kprobe_multi_opts(prog, pattern, &opts);
+	free(pattern);
+	return libbpf_get_error(*link);
+}
+
 static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
 {
 	char *probe_type = NULL, *binary_path = NULL, *func_name = NULL;
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index 5723cbbfcc41..72f4e3ad295f 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -539,10 +539,12 @@ struct bpf_kprobe_multi_opts {
 	size_t cnt;
 	/* create return kprobes */
 	bool retprobe;
+	/* create wrapper kprobes */
+	bool wrapper;
 	size_t :0;
 };
 
-#define bpf_kprobe_multi_opts__last_field retprobe
+#define bpf_kprobe_multi_opts__last_field wrapper
 
 LIBBPF_API struct bpf_link *
 bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
-- 
2.43.2

[PATCH RFCv2 bpf-next 4/4] selftests/bpf: Add kprobe multi wrapper test

[Jiri Olsa]

Adding kprobe multi wrapper test and also testing the entry program
return value controls execution of the return probe program.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/testing/selftests/bpf/bpf_kfuncs.h      |   2 +
 .../bpf/prog_tests/kprobe_multi_test.c        |  49 +++++++++
 .../bpf/progs/kprobe_multi_wrapper.c          | 100 ++++++++++++++++++
 3 files changed, 151 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c

diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
index 14ebe7d9e1a3..9ad4c64b19e9 100644
--- a/tools/testing/selftests/bpf/bpf_kfuncs.h
+++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
@@ -75,4 +75,6 @@ extern void bpf_key_put(struct bpf_key *key) __ksym;
 extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
 				      struct bpf_dynptr *sig_ptr,
 				      struct bpf_key *trusted_keyring) __ksym;
+
+extern bool bpf_kprobe_multi_is_return(void) __ksym;
 #endif
diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 05000810e28e..1120c43b215c 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -4,6 +4,7 @@
 #include "trace_helpers.h"
 #include "kprobe_multi_empty.skel.h"
 #include "kprobe_multi_override.skel.h"
+#include "kprobe_multi_wrapper.skel.h"
 #include "bpf/libbpf_internal.h"
 #include "bpf/hashmap.h"
 
@@ -326,6 +327,52 @@ static void test_attach_api_fails(void)
 	kprobe_multi__destroy(skel);
 }
 
+static void test_wrapper_skel_api(void)
+{
+	struct kprobe_multi_wrapper *skel = NULL;
+	LIBBPF_OPTS(bpf_kprobe_multi_opts, opts);
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	struct bpf_link *link = NULL;
+	int err, prog_fd;
+
+	skel = kprobe_multi_wrapper__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "fentry_raw_skel_load"))
+		goto cleanup;
+
+	skel->bss->pid = getpid();
+
+	err =  kprobe_multi_wrapper__attach(skel);
+	if (!ASSERT_OK(err, " kprobe_multi_wrapper__attach"))
+		goto cleanup;
+
+	prog_fd = bpf_program__fd(skel->progs.trigger);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	ASSERT_OK(err, "test_run");
+	ASSERT_EQ(topts.retval, 0, "test_run");
+
+	ASSERT_EQ(skel->bss->kprobe_test1_result, 1, "kprobe_test1_result");
+	ASSERT_EQ(skel->bss->kprobe_test2_result, 1, "kprobe_test2_result");
+	ASSERT_EQ(skel->bss->kprobe_test3_result, 1, "kprobe_test3_result");
+	ASSERT_EQ(skel->bss->kprobe_test4_result, 1, "kprobe_test4_result");
+	ASSERT_EQ(skel->bss->kprobe_test5_result, 1, "kprobe_test5_result");
+	ASSERT_EQ(skel->bss->kprobe_test6_result, 1, "kprobe_test6_result");
+	ASSERT_EQ(skel->bss->kprobe_test7_result, 1, "kprobe_test7_result");
+	ASSERT_EQ(skel->bss->kprobe_test8_result, 1, "kprobe_test8_result");
+
+	ASSERT_EQ(skel->bss->kretprobe_test1_result, 0, "kretprobe_test1_result");
+	ASSERT_EQ(skel->bss->kretprobe_test2_result, 1, "kretprobe_test2_result");
+	ASSERT_EQ(skel->bss->kretprobe_test3_result, 0, "kretprobe_test3_result");
+	ASSERT_EQ(skel->bss->kretprobe_test4_result, 1, "kretprobe_test4_result");
+	ASSERT_EQ(skel->bss->kretprobe_test5_result, 0, "kretprobe_test5_result");
+	ASSERT_EQ(skel->bss->kretprobe_test6_result, 1, "kretprobe_test6_result");
+	ASSERT_EQ(skel->bss->kretprobe_test7_result, 0, "kretprobe_test7_result");
+	ASSERT_EQ(skel->bss->kretprobe_test8_result, 1, "kretprobe_test8_result");
+
+cleanup:
+	bpf_link__destroy(link);
+	kprobe_multi_wrapper__destroy(skel);
+}
+
 static size_t symbol_hash(long key, void *ctx __maybe_unused)
 {
 	return str_hash((const char *) key);
@@ -538,4 +585,6 @@ void test_kprobe_multi_test(void)
 		test_attach_api_fails();
 	if (test__start_subtest("attach_override"))
 		test_attach_override();
+	if (test__start_subtest("wrapper"))
+		test_wrapper_skel_api();
 }
diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c b/tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c
new file mode 100644
index 000000000000..975492bba8cc
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/kprobe_multi_wrapper.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <stdbool.h>
+#include "bpf_kfuncs.h"
+
+char _license[] SEC("license") = "GPL";
+
+extern const void bpf_fentry_test1 __ksym;
+extern const void bpf_fentry_test2 __ksym;
+extern const void bpf_fentry_test3 __ksym;
+extern const void bpf_fentry_test4 __ksym;
+extern const void bpf_fentry_test5 __ksym;
+extern const void bpf_fentry_test6 __ksym;
+extern const void bpf_fentry_test7 __ksym;
+extern const void bpf_fentry_test8 __ksym;
+
+int pid = 0;
+
+__u64 kprobe_test1_result = 0;
+__u64 kprobe_test2_result = 0;
+__u64 kprobe_test3_result = 0;
+__u64 kprobe_test4_result = 0;
+__u64 kprobe_test5_result = 0;
+__u64 kprobe_test6_result = 0;
+__u64 kprobe_test7_result = 0;
+__u64 kprobe_test8_result = 0;
+
+__u64 kretprobe_test1_result = 0;
+__u64 kretprobe_test2_result = 0;
+__u64 kretprobe_test3_result = 0;
+__u64 kretprobe_test4_result = 0;
+__u64 kretprobe_test5_result = 0;
+__u64 kretprobe_test6_result = 0;
+__u64 kretprobe_test7_result = 0;
+__u64 kretprobe_test8_result = 0;
+
+static int wrapper_check(void *ctx, bool is_return)
+{
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
+		return 1;
+
+	__u64 addr = bpf_get_func_ip(ctx);
+
+#define SET(__var, __addr) ({			\
+	if ((const void *) addr == __addr)	\
+		__var = 1;			\
+})
+
+	if (is_return) {
+		SET(kretprobe_test1_result, &bpf_fentry_test1);
+		SET(kretprobe_test2_result, &bpf_fentry_test2);
+		SET(kretprobe_test3_result, &bpf_fentry_test3);
+		SET(kretprobe_test4_result, &bpf_fentry_test4);
+		SET(kretprobe_test5_result, &bpf_fentry_test5);
+		SET(kretprobe_test6_result, &bpf_fentry_test6);
+		SET(kretprobe_test7_result, &bpf_fentry_test7);
+		SET(kretprobe_test8_result, &bpf_fentry_test8);
+	} else {
+		SET(kprobe_test1_result, &bpf_fentry_test1);
+		SET(kprobe_test2_result, &bpf_fentry_test2);
+		SET(kprobe_test3_result, &bpf_fentry_test3);
+		SET(kprobe_test4_result, &bpf_fentry_test4);
+		SET(kprobe_test5_result, &bpf_fentry_test5);
+		SET(kprobe_test6_result, &bpf_fentry_test6);
+		SET(kprobe_test7_result, &bpf_fentry_test7);
+		SET(kprobe_test8_result, &bpf_fentry_test8);
+	}
+
+#undef SET
+
+	/*
+	 * Force probes for function bpf_fentry_test[1357] not to
+	 * install and execute the return probe
+	 */
+	if (((const void *) addr == &bpf_fentry_test1) ||
+	    ((const void *) addr == &bpf_fentry_test3) ||
+	    ((const void *) addr == &bpf_fentry_test5) ||
+	    ((const void *) addr == &bpf_fentry_test7))
+		return 1;
+
+	return 0;
+}
+
+/*
+ * No tests in here, just to trigger 'bpf_fentry_test*'
+ * through tracing test_run
+ */
+SEC("fentry/bpf_modify_return_test")
+int BPF_PROG(trigger)
+{
+	return 0;
+}
+
+SEC("kprobe.wrapper/bpf_fentry_test*")
+int test_kprobe(struct pt_regs *ctx)
+{
+	return wrapper_check(ctx, bpf_kprobe_multi_is_return());
+}
-- 
2.43.2

Re: [PATCH RFCv2 bpf-next 1/4] bpf: Add support for kprobe multi wrapper attach

[Andrii Nakryiko]

On Wed, Feb 28, 2024 at 1:03 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support to attach bpf program for entry and return probe
> of the same function. This is common usecase and at the moment
> it requires to create two kprobe multi links.
>
> Adding new attr.link_create.kprobe_multi.flags value that instructs
> kernel to attach link program to both entry and exit probe.
>
> It's possible to control execution of the bpf program on return
> probe simply by returning zero or non zero from the entry bpf
> program execution to execute or not respectively the bpf program
> on return probe.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  include/uapi/linux/bpf.h       |  3 ++-
>  kernel/trace/bpf_trace.c       | 24 ++++++++++++++++++------
>  tools/include/uapi/linux/bpf.h |  3 ++-
>  3 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index d2e6c5fcec01..a430855c5bcd 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
>   * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
>   */
>  enum {
> -       BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
> +       BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
> +       BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
>  };
>
>  /* link_create.uprobe_multi.flags used in LINK_CREATE command for
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 241ddf5e3895..726a8c71f0da 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2587,6 +2587,7 @@ struct bpf_kprobe_multi_link {
>         u32 mods_cnt;
>         struct module **mods;
>         u32 flags;
> +       bool is_wrapper;

flags should be sufficient for this, why storing redundant bool field?

>  };
>
>  struct bpf_kprobe_multi_run_ctx {
> @@ -2826,10 +2827,11 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
>                           void *data)
>  {
>         struct bpf_kprobe_multi_link *link;
> +       int err;
>
>         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> -       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> -       return 0;
> +       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> +       return link->is_wrapper ? err : 0;
>  }
>
>  static void
> @@ -2967,6 +2969,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>         void __user *uaddrs;
>         u64 *cookies = NULL;
>         void __user *usyms;
> +       bool is_wrapper;
>         int err;
>
>         /* no support for 32bit archs yet */
> @@ -2977,9 +2980,12 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>                 return -EINVAL;
>
>         flags = attr->link_create.kprobe_multi.flags;
> -       if (flags & ~BPF_F_KPROBE_MULTI_RETURN)
> +       if (flags & ~(BPF_F_KPROBE_MULTI_RETURN|
> +                     BPF_F_KPROBE_MULTI_WRAPPER))

nit: spaces around | are missing, also keep on a single line?

>                 return -EINVAL;
>
> +       is_wrapper = flags & BPF_F_KPROBE_MULTI_WRAPPER;
> +
>         uaddrs = u64_to_user_ptr(attr->link_create.kprobe_multi.addrs);
>         usyms = u64_to_user_ptr(attr->link_create.kprobe_multi.syms);
>         if (!!uaddrs == !!usyms)
> @@ -3054,15 +3060,21 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>         if (err)
>                 goto error;
>
> -       if (flags & BPF_F_KPROBE_MULTI_RETURN)
> -               link->fp.exit_handler = kprobe_multi_link_exit_handler;
> -       else
> +       if (is_wrapper) {
>                 link->fp.entry_handler = kprobe_multi_link_handler;
> +               link->fp.exit_handler = kprobe_multi_link_exit_handler;
> +       } else {
> +               if (flags & BPF_F_KPROBE_MULTI_RETURN)
> +                       link->fp.exit_handler = kprobe_multi_link_exit_handler;
> +               else
> +                       link->fp.entry_handler = kprobe_multi_link_handler;
> +       }
>

how about:

if (!(flags & BPF_F_KPROBE_MULTI_RETURN))
    link->fp.entry_handler = kprobe_multi_link_handler;
if (flags & (BPF_F_KPROBE_MULTI_RETURN | BPF_F_KPROBE_MULTI_WRAPPER))
    link->fp.exit_handler = kprobe_multi_link_exit_handler;


>         link->addrs = addrs;
>         link->cookies = cookies;
>         link->cnt = cnt;
>         link->flags = flags;
> +       link->is_wrapper = is_wrapper;
>
>         if (cookies) {
>                 /*
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index d2e6c5fcec01..a430855c5bcd 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
>   * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
>   */
>  enum {
> -       BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
> +       BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
> +       BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
>  };
>
>  /* link_create.uprobe_multi.flags used in LINK_CREATE command for
> --
> 2.43.2
>

Re: [PATCH RFCv2 bpf-next 2/4] bpf: Add bpf_kprobe_multi_is_return kfunc

[Andrii Nakryiko]

On Wed, Feb 28, 2024 at 1:03 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding bpf_kprobe_multi_is_return kfunc that returns true if the
> bpf program is executed from the exit probe of the kprobe multi
> link attached in wrapper mode. It returns false otherwise.
>
> Adding new kprobe hook for kprobe program type.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/bpf/btf.c         |  3 +++
>  kernel/trace/bpf_trace.c | 49 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 49 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 6ff0bd1a91d5..5ab55720e881 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -218,6 +218,7 @@ enum btf_kfunc_hook {
>         BTF_KFUNC_HOOK_SOCKET_FILTER,
>         BTF_KFUNC_HOOK_LWT,
>         BTF_KFUNC_HOOK_NETFILTER,
> +       BTF_KFUNC_HOOK_KPROBE,
>         BTF_KFUNC_HOOK_MAX,
>  };
>
> @@ -8112,6 +8113,8 @@ static int bpf_prog_type_to_kfunc_hook(enum bpf_prog_type prog_type)
>                 return BTF_KFUNC_HOOK_LWT;
>         case BPF_PROG_TYPE_NETFILTER:
>                 return BTF_KFUNC_HOOK_NETFILTER;
> +       case BPF_PROG_TYPE_KPROBE:
> +               return BTF_KFUNC_HOOK_KPROBE;
>         default:
>                 return BTF_KFUNC_HOOK_MAX;
>         }
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 726a8c71f0da..cb801c94b8fa 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2594,6 +2594,7 @@ struct bpf_kprobe_multi_run_ctx {
>         struct bpf_run_ctx run_ctx;
>         struct bpf_kprobe_multi_link *link;
>         unsigned long entry_ip;
> +       bool is_return;
>  };
>
>  struct user_syms {
> @@ -2793,11 +2794,13 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
>
>  static int
>  kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> -                          unsigned long entry_ip, struct pt_regs *regs)
> +                          unsigned long entry_ip, struct pt_regs *regs,
> +                          bool is_return)
>  {
>         struct bpf_kprobe_multi_run_ctx run_ctx = {
>                 .link = link,
>                 .entry_ip = entry_ip,
> +               .is_return = is_return,
>         };
>         struct bpf_run_ctx *old_run_ctx;
>         int err;
> @@ -2830,7 +2833,7 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
>         int err;
>
>         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> -       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> +       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false);
>         return link->is_wrapper ? err : 0;
>  }
>
> @@ -2842,7 +2845,7 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
>         struct bpf_kprobe_multi_link *link;
>
>         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> -       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> +       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true);
>  }
>
>  static int symbols_cmp_r(const void *a, const void *b, const void *priv)
> @@ -3111,6 +3114,46 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
>         kvfree(cookies);
>         return err;
>  }
> +
> +__bpf_kfunc_start_defs();
> +
> +__bpf_kfunc bool bpf_kprobe_multi_is_return(void)

and for uprobes we'll have bpf_uprobe_multi_is_return?...

BTW, have you tried implementing a "session cookie" idea?


> +{
> +       struct bpf_kprobe_multi_run_ctx *run_ctx;
> +
> +       run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
> +       return run_ctx->is_return;
> +}
> +
> +__bpf_kfunc_end_defs();
> +
> +BTF_KFUNCS_START(kprobe_multi_kfunc_set_ids)
> +BTF_ID_FLAGS(func, bpf_kprobe_multi_is_return)
> +BTF_KFUNCS_END(kprobe_multi_kfunc_set_ids)
> +
> +static int bpf_kprobe_multi_filter(const struct bpf_prog *prog, u32 kfunc_id)
> +{
> +       if (!btf_id_set8_contains(&kprobe_multi_kfunc_set_ids, kfunc_id))
> +               return 0;
> +
> +       if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
> +               return -EACCES;
> +
> +       return 0;
> +}
> +
> +static const struct btf_kfunc_id_set bpf_kprobe_multi_kfunc_set = {
> +       .owner = THIS_MODULE,
> +       .set = &kprobe_multi_kfunc_set_ids,
> +       .filter = bpf_kprobe_multi_filter,
> +};
> +
> +static int __init bpf_kprobe_multi_kfuncs_init(void)
> +{
> +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_KPROBE, &bpf_kprobe_multi_kfunc_set);
> +}
> +
> +late_initcall(bpf_kprobe_multi_kfuncs_init);
>  #else /* !CONFIG_FPROBE */
>  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>  {
> --
> 2.43.2
>

Re: [PATCH RFCv2 bpf-next 3/4] libbpf: Add support for kprobe multi wrapper attach

[Andrii Nakryiko]

On Wed, Feb 28, 2024 at 1:03 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding support for specify wrapper mode in bpf_kprobe_multi_opts
> struct object and new bpf program loader section:
>
>  SEC("kprobe.wrapper/bpf_fentry_test*")
>
> to load program as kprobe multi wrapper.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 38 +++++++++++++++++++++++++++++++++++---
>  tools/lib/bpf/libbpf.h |  4 +++-
>  2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 01f407591a92..5416d784c857 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8944,6 +8944,7 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
>  static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> +static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
>  static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> @@ -8960,6 +8961,7 @@ static const struct bpf_sec_def section_defs[] = {
>         SEC_DEF("uretprobe.s+",         KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
>         SEC_DEF("kprobe.multi+",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
>         SEC_DEF("kretprobe.multi+",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> +       SEC_DEF("kprobe.wrapper+",      KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_wrapper),
>         SEC_DEF("uprobe.multi+",        KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
>         SEC_DEF("uretprobe.multi+",     KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
>         SEC_DEF("uprobe.multi.s+",      KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_SLEEPABLE, attach_uprobe_multi),
> @@ -11034,7 +11036,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>         int err, link_fd, prog_fd;
>         const __u64 *cookies;
>         const char **syms;
> -       bool retprobe;
> +       __u32 flags = 0;
>         size_t cnt;
>
>         if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> @@ -11065,13 +11067,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
>                 cnt = res.cnt;
>         }
>
> -       retprobe = OPTS_GET(opts, retprobe, false);
> +       if (OPTS_GET(opts, retprobe, false))
> +               flags |= BPF_F_KPROBE_MULTI_RETURN;
> +       if (OPTS_GET(opts, wrapper, false))
> +               flags |= BPF_F_KPROBE_MULTI_WRAPPER;

probably error out if both retprobe and wrapper are set?

>
>         lopts.kprobe_multi.syms = syms;
>         lopts.kprobe_multi.addrs = addrs;
>         lopts.kprobe_multi.cookies = cookies;
>         lopts.kprobe_multi.cnt = cnt;
> -       lopts.kprobe_multi.flags = retprobe ? BPF_F_KPROBE_MULTI_RETURN : 0;
> +       lopts.kprobe_multi.flags = flags;
>
>         link = calloc(1, sizeof(*link));
>         if (!link) {
> @@ -11187,6 +11192,33 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
>         return libbpf_get_error(*link);
>  }
>
> +static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> +{
> +       LIBBPF_OPTS(bpf_kprobe_multi_opts, opts,
> +               .wrapper = true,
> +       );

nit: keep on a single line?

> +       const char *spec;
> +       char *pattern;
> +       int n;
> +
> +       *link = NULL;
> +
> +       /* no auto-attach for SEC("kprobe.wrapper") */
> +       if (strcmp(prog->sec_name, "kprobe.wrapper") == 0)
> +               return 0;
> +
> +       spec = prog->sec_name + sizeof("kprobe.wrapper/") - 1;
> +       n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
> +       if (n < 1) {
> +               pr_warn("kprobe wrapper pattern is invalid: %s\n", pattern);
> +               return -EINVAL;
> +       }
> +
> +       *link = bpf_program__attach_kprobe_multi_opts(prog, pattern, &opts);
> +       free(pattern);

is it guaranteed that free() won't clobber errno? or should we record
it right after attach call (and stop using libbpf_get_error())?


> +       return libbpf_get_error(*link);
> +}
> +
>  static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
>  {
>         char *probe_type = NULL, *binary_path = NULL, *func_name = NULL;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5723cbbfcc41..72f4e3ad295f 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -539,10 +539,12 @@ struct bpf_kprobe_multi_opts {
>         size_t cnt;
>         /* create return kprobes */
>         bool retprobe;
> +       /* create wrapper kprobes */
> +       bool wrapper;
>         size_t :0;
>  };
>
> -#define bpf_kprobe_multi_opts__last_field retprobe
> +#define bpf_kprobe_multi_opts__last_field wrapper
>
>  LIBBPF_API struct bpf_link *
>  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> --
> 2.43.2
>

Re: [PATCH RFCv2 bpf-next 2/4] bpf: Add bpf_kprobe_multi_is_return kfunc

[Jiri Olsa]

On Wed, Feb 28, 2024 at 05:23:45PM -0800, Andrii Nakryiko wrote:

SNIP

> >  static int
> >  kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> > -                          unsigned long entry_ip, struct pt_regs *regs)
> > +                          unsigned long entry_ip, struct pt_regs *regs,
> > +                          bool is_return)
> >  {
> >         struct bpf_kprobe_multi_run_ctx run_ctx = {
> >                 .link = link,
> >                 .entry_ip = entry_ip,
> > +               .is_return = is_return,
> >         };
> >         struct bpf_run_ctx *old_run_ctx;
> >         int err;
> > @@ -2830,7 +2833,7 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> >         int err;
> >
> >         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > -       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > +       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false);
> >         return link->is_wrapper ? err : 0;
> >  }
> >
> > @@ -2842,7 +2845,7 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
> >         struct bpf_kprobe_multi_link *link;
> >
> >         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > -       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > +       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true);
> >  }
> >
> >  static int symbols_cmp_r(const void *a, const void *b, const void *priv)
> > @@ -3111,6 +3114,46 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >         kvfree(cookies);
> >         return err;
> >  }
> > +
> > +__bpf_kfunc_start_defs();
> > +
> > +__bpf_kfunc bool bpf_kprobe_multi_is_return(void)
> 
> and for uprobes we'll have bpf_uprobe_multi_is_return?...

yes, but now I'm thinking maybe we could also have 'session' api and
have single 'bpf_session_is_return' because both kprobe and uprobe
are KPROBE program type.. and align it together with other session
kfuncs:

  bpf_session_is_return
  bpf_session_set_cookie
  bpf_session_get_cookie

> 
> BTW, have you tried implementing a "session cookie" idea?

yep, with a little fix [0] it's working on top of Masami's 'fprobe over fgraph'
changes, you can check last 2 patches in [1] .. I did not do this on top of the
current fprobe/rethook kernel code, because it seems it's about to go away

I still need to implement that on top of uprobes and I will send rfc, so we can
see all of it and discuss the interface

jirka


[0] https://lore.kernel.org/bpf/ZdyKaRiI-PnG80Q0@krava/
[1] https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/log/?h=bpf/session_data

> 
> 
> > +{
> > +       struct bpf_kprobe_multi_run_ctx *run_ctx;
> > +
> > +       run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
> > +       return run_ctx->is_return;
> > +}
> > +
> > +__bpf_kfunc_end_defs();
> > +
> > +BTF_KFUNCS_START(kprobe_multi_kfunc_set_ids)
> > +BTF_ID_FLAGS(func, bpf_kprobe_multi_is_return)
> > +BTF_KFUNCS_END(kprobe_multi_kfunc_set_ids)
> > +
> > +static int bpf_kprobe_multi_filter(const struct bpf_prog *prog, u32 kfunc_id)
> > +{
> > +       if (!btf_id_set8_contains(&kprobe_multi_kfunc_set_ids, kfunc_id))
> > +               return 0;
> > +
> > +       if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
> > +               return -EACCES;
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct btf_kfunc_id_set bpf_kprobe_multi_kfunc_set = {
> > +       .owner = THIS_MODULE,
> > +       .set = &kprobe_multi_kfunc_set_ids,
> > +       .filter = bpf_kprobe_multi_filter,
> > +};
> > +
> > +static int __init bpf_kprobe_multi_kfuncs_init(void)
> > +{
> > +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_KPROBE, &bpf_kprobe_multi_kfunc_set);
> > +}
> > +
> > +late_initcall(bpf_kprobe_multi_kfuncs_init);
> >  #else /* !CONFIG_FPROBE */
> >  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >  {
> > --
> > 2.43.2
> >

Re: [PATCH RFCv2 bpf-next 1/4] bpf: Add support for kprobe multi wrapper attach

[Jiri Olsa]

On Wed, Feb 28, 2024 at 05:23:05PM -0800, Andrii Nakryiko wrote:
> On Wed, Feb 28, 2024 at 1:03 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support to attach bpf program for entry and return probe
> > of the same function. This is common usecase and at the moment
> > it requires to create two kprobe multi links.
> >
> > Adding new attr.link_create.kprobe_multi.flags value that instructs
> > kernel to attach link program to both entry and exit probe.
> >
> > It's possible to control execution of the bpf program on return
> > probe simply by returning zero or non zero from the entry bpf
> > program execution to execute or not respectively the bpf program
> > on return probe.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  include/uapi/linux/bpf.h       |  3 ++-
> >  kernel/trace/bpf_trace.c       | 24 ++++++++++++++++++------
> >  tools/include/uapi/linux/bpf.h |  3 ++-
> >  3 files changed, 22 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index d2e6c5fcec01..a430855c5bcd 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
> >   * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
> >   */
> >  enum {
> > -       BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
> > +       BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
> > +       BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
> >  };
> >
> >  /* link_create.uprobe_multi.flags used in LINK_CREATE command for
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index 241ddf5e3895..726a8c71f0da 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2587,6 +2587,7 @@ struct bpf_kprobe_multi_link {
> >         u32 mods_cnt;
> >         struct module **mods;
> >         u32 flags;
> > +       bool is_wrapper;
> 
> flags should be sufficient for this, why storing redundant bool field?

true

> 
> >  };
> >
> >  struct bpf_kprobe_multi_run_ctx {
> > @@ -2826,10 +2827,11 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> >                           void *data)
> >  {
> >         struct bpf_kprobe_multi_link *link;
> > +       int err;
> >
> >         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > -       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > -       return 0;
> > +       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > +       return link->is_wrapper ? err : 0;
> >  }
> >
> >  static void
> > @@ -2967,6 +2969,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >         void __user *uaddrs;
> >         u64 *cookies = NULL;
> >         void __user *usyms;
> > +       bool is_wrapper;
> >         int err;
> >
> >         /* no support for 32bit archs yet */
> > @@ -2977,9 +2980,12 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >                 return -EINVAL;
> >
> >         flags = attr->link_create.kprobe_multi.flags;
> > -       if (flags & ~BPF_F_KPROBE_MULTI_RETURN)
> > +       if (flags & ~(BPF_F_KPROBE_MULTI_RETURN|
> > +                     BPF_F_KPROBE_MULTI_WRAPPER))
> 
> nit: spaces around | are missing, also keep on a single line?

ok

> 
> >                 return -EINVAL;
> >
> > +       is_wrapper = flags & BPF_F_KPROBE_MULTI_WRAPPER;
> > +
> >         uaddrs = u64_to_user_ptr(attr->link_create.kprobe_multi.addrs);
> >         usyms = u64_to_user_ptr(attr->link_create.kprobe_multi.syms);
> >         if (!!uaddrs == !!usyms)
> > @@ -3054,15 +3060,21 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> >         if (err)
> >                 goto error;
> >
> > -       if (flags & BPF_F_KPROBE_MULTI_RETURN)
> > -               link->fp.exit_handler = kprobe_multi_link_exit_handler;
> > -       else
> > +       if (is_wrapper) {
> >                 link->fp.entry_handler = kprobe_multi_link_handler;
> > +               link->fp.exit_handler = kprobe_multi_link_exit_handler;
> > +       } else {
> > +               if (flags & BPF_F_KPROBE_MULTI_RETURN)
> > +                       link->fp.exit_handler = kprobe_multi_link_exit_handler;
> > +               else
> > +                       link->fp.entry_handler = kprobe_multi_link_handler;
> > +       }
> >
> 
> how about:
> 
> if (!(flags & BPF_F_KPROBE_MULTI_RETURN))
>     link->fp.entry_handler = kprobe_multi_link_handler;
> if (flags & (BPF_F_KPROBE_MULTI_RETURN | BPF_F_KPROBE_MULTI_WRAPPER))
>     link->fp.exit_handler = kprobe_multi_link_exit_handler;

I have another changes on top of that, which add more handlers,
so I guess I wanted to keep it more explicit, but your suggestion
is simpler, will change

thanks,
jirka

> 
> 
> >         link->addrs = addrs;
> >         link->cookies = cookies;
> >         link->cnt = cnt;
> >         link->flags = flags;
> > +       link->is_wrapper = is_wrapper;
> >
> >         if (cookies) {
> >                 /*
> > diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> > index d2e6c5fcec01..a430855c5bcd 100644
> > --- a/tools/include/uapi/linux/bpf.h
> > +++ b/tools/include/uapi/linux/bpf.h
> > @@ -1247,7 +1247,8 @@ enum bpf_perf_event_type {
> >   * BPF_TRACE_KPROBE_MULTI attach type to create return probe.
> >   */
> >  enum {
> > -       BPF_F_KPROBE_MULTI_RETURN = (1U << 0)
> > +       BPF_F_KPROBE_MULTI_RETURN  = (1U << 0),
> > +       BPF_F_KPROBE_MULTI_WRAPPER = (1U << 1),
> >  };
> >
> >  /* link_create.uprobe_multi.flags used in LINK_CREATE command for
> > --
> > 2.43.2
> >

Re: [PATCH RFCv2 bpf-next 3/4] libbpf: Add support for kprobe multi wrapper attach

[Jiri Olsa]

On Wed, Feb 28, 2024 at 05:23:57PM -0800, Andrii Nakryiko wrote:
> On Wed, Feb 28, 2024 at 1:03 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding support for specify wrapper mode in bpf_kprobe_multi_opts
> > struct object and new bpf program loader section:
> >
> >  SEC("kprobe.wrapper/bpf_fentry_test*")
> >
> > to load program as kprobe multi wrapper.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/bpf/libbpf.c | 38 +++++++++++++++++++++++++++++++++++---
> >  tools/lib/bpf/libbpf.h |  4 +++-
> >  2 files changed, 38 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index 01f407591a92..5416d784c857 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -8944,6 +8944,7 @@ static int attach_tp(const struct bpf_program *prog, long cookie, struct bpf_lin
> >  static int attach_raw_tp(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >  static int attach_trace(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >  static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > +static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >  static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >  static int attach_lsm(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> >  static int attach_iter(const struct bpf_program *prog, long cookie, struct bpf_link **link);
> > @@ -8960,6 +8961,7 @@ static const struct bpf_sec_def section_defs[] = {
> >         SEC_DEF("uretprobe.s+",         KPROBE, 0, SEC_SLEEPABLE, attach_uprobe),
> >         SEC_DEF("kprobe.multi+",        KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> >         SEC_DEF("kretprobe.multi+",     KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_multi),
> > +       SEC_DEF("kprobe.wrapper+",      KPROBE, BPF_TRACE_KPROBE_MULTI, SEC_NONE, attach_kprobe_wrapper),
> >         SEC_DEF("uprobe.multi+",        KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
> >         SEC_DEF("uretprobe.multi+",     KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_NONE, attach_uprobe_multi),
> >         SEC_DEF("uprobe.multi.s+",      KPROBE, BPF_TRACE_UPROBE_MULTI, SEC_SLEEPABLE, attach_uprobe_multi),
> > @@ -11034,7 +11036,7 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >         int err, link_fd, prog_fd;
> >         const __u64 *cookies;
> >         const char **syms;
> > -       bool retprobe;
> > +       __u32 flags = 0;
> >         size_t cnt;
> >
> >         if (!OPTS_VALID(opts, bpf_kprobe_multi_opts))
> > @@ -11065,13 +11067,16 @@ bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> >                 cnt = res.cnt;
> >         }
> >
> > -       retprobe = OPTS_GET(opts, retprobe, false);
> > +       if (OPTS_GET(opts, retprobe, false))
> > +               flags |= BPF_F_KPROBE_MULTI_RETURN;
> > +       if (OPTS_GET(opts, wrapper, false))
> > +               flags |= BPF_F_KPROBE_MULTI_WRAPPER;
> 
> probably error out if both retprobe and wrapper are set?

ok

> 
> >
> >         lopts.kprobe_multi.syms = syms;
> >         lopts.kprobe_multi.addrs = addrs;
> >         lopts.kprobe_multi.cookies = cookies;
> >         lopts.kprobe_multi.cnt = cnt;
> > -       lopts.kprobe_multi.flags = retprobe ? BPF_F_KPROBE_MULTI_RETURN : 0;
> > +       lopts.kprobe_multi.flags = flags;
> >
> >         link = calloc(1, sizeof(*link));
> >         if (!link) {
> > @@ -11187,6 +11192,33 @@ static int attach_kprobe_multi(const struct bpf_program *prog, long cookie, stru
> >         return libbpf_get_error(*link);
> >  }
> >
> > +static int attach_kprobe_wrapper(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> > +{
> > +       LIBBPF_OPTS(bpf_kprobe_multi_opts, opts,
> > +               .wrapper = true,
> > +       );
> 
> nit: keep on a single line?

ok

> 
> > +       const char *spec;
> > +       char *pattern;
> > +       int n;
> > +
> > +       *link = NULL;
> > +
> > +       /* no auto-attach for SEC("kprobe.wrapper") */
> > +       if (strcmp(prog->sec_name, "kprobe.wrapper") == 0)
> > +               return 0;
> > +
> > +       spec = prog->sec_name + sizeof("kprobe.wrapper/") - 1;
> > +       n = sscanf(spec, "%m[a-zA-Z0-9_.*?]", &pattern);
> > +       if (n < 1) {
> > +               pr_warn("kprobe wrapper pattern is invalid: %s\n", pattern);
> > +               return -EINVAL;
> > +       }
> > +
> > +       *link = bpf_program__attach_kprobe_multi_opts(prog, pattern, &opts);
> > +       free(pattern);
> 
> is it guaranteed that free() won't clobber errno? or should we record
> it right after attach call (and stop using libbpf_get_error())?

hum, I copy&pasted from the attach_kprobe_multi, so did not think of that ;-)

anyway man page says:
  The free() function returns no value, and preserves errno.

so we're good

thanks,
jirka

> 
> 
> > +       return libbpf_get_error(*link);
> > +}
> > +
> >  static int attach_uprobe_multi(const struct bpf_program *prog, long cookie, struct bpf_link **link)
> >  {
> >         char *probe_type = NULL, *binary_path = NULL, *func_name = NULL;
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index 5723cbbfcc41..72f4e3ad295f 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -539,10 +539,12 @@ struct bpf_kprobe_multi_opts {
> >         size_t cnt;
> >         /* create return kprobes */
> >         bool retprobe;
> > +       /* create wrapper kprobes */
> > +       bool wrapper;
> >         size_t :0;
> >  };
> >
> > -#define bpf_kprobe_multi_opts__last_field retprobe
> > +#define bpf_kprobe_multi_opts__last_field wrapper
> >
> >  LIBBPF_API struct bpf_link *
> >  bpf_program__attach_kprobe_multi_opts(const struct bpf_program *prog,
> > --
> > 2.43.2
> >

Re: [PATCH RFCv2 bpf-next 2/4] bpf: Add bpf_kprobe_multi_is_return kfunc

[Andrii Nakryiko]

On Thu, Feb 29, 2024 at 2:16 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Wed, Feb 28, 2024 at 05:23:45PM -0800, Andrii Nakryiko wrote:
>
> SNIP
>
> > >  static int
> > >  kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> > > -                          unsigned long entry_ip, struct pt_regs *regs)
> > > +                          unsigned long entry_ip, struct pt_regs *regs,
> > > +                          bool is_return)
> > >  {
> > >         struct bpf_kprobe_multi_run_ctx run_ctx = {
> > >                 .link = link,
> > >                 .entry_ip = entry_ip,
> > > +               .is_return = is_return,
> > >         };
> > >         struct bpf_run_ctx *old_run_ctx;
> > >         int err;
> > > @@ -2830,7 +2833,7 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
> > >         int err;
> > >
> > >         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > > -       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > > +       err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false);
> > >         return link->is_wrapper ? err : 0;
> > >  }
> > >
> > > @@ -2842,7 +2845,7 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
> > >         struct bpf_kprobe_multi_link *link;
> > >
> > >         link = container_of(fp, struct bpf_kprobe_multi_link, fp);
> > > -       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs);
> > > +       kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true);
> > >  }
> > >
> > >  static int symbols_cmp_r(const void *a, const void *b, const void *priv)
> > > @@ -3111,6 +3114,46 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
> > >         kvfree(cookies);
> > >         return err;
> > >  }
> > > +
> > > +__bpf_kfunc_start_defs();
> > > +
> > > +__bpf_kfunc bool bpf_kprobe_multi_is_return(void)
> >
> > and for uprobes we'll have bpf_uprobe_multi_is_return?...
>
> yes, but now I'm thinking maybe we could also have 'session' api and
> have single 'bpf_session_is_return' because both kprobe and uprobe
> are KPROBE program type.. and align it together with other session
> kfuncs:
>
>   bpf_session_is_return
>   bpf_session_set_cookie
>   bpf_session_get_cookie
>

We can do that. But I was thinking more of a

u64 *bpf_session_cookie()

which would return a read/write pointer that BPF program can
manipulate. Instead of doing two calls (get_cookie + set_cookie), it
would be one call. Is there any benefit to having separate set/get
cookie calls?

> >
> > BTW, have you tried implementing a "session cookie" idea?
>
> yep, with a little fix [0] it's working on top of Masami's 'fprobe over fgraph'
> changes, you can check last 2 patches in [1] .. I did not do this on top of the
> current fprobe/rethook kernel code, because it seems it's about to go away

do you know what is the timeline for fprobe over fgraph work to be finished?

>
> I still need to implement that on top of uprobes and I will send rfc, so we can
> see all of it and discuss the interface
>

great, yeah, I think the session cookie idea should go in at the same
time, if possible, so that we can assume it is supported for new
[ku]probe.wrapper programs.


> jirka
>
>
> [0] https://lore.kernel.org/bpf/ZdyKaRiI-PnG80Q0@krava/
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git/log/?h=bpf/session_data
>
> >
> >
> > > +{
> > > +       struct bpf_kprobe_multi_run_ctx *run_ctx;
> > > +
> > > +       run_ctx = container_of(current->bpf_ctx, struct bpf_kprobe_multi_run_ctx, run_ctx);
> > > +       return run_ctx->is_return;
> > > +}
> > > +
> > > +__bpf_kfunc_end_defs();
> > > +
> > > +BTF_KFUNCS_START(kprobe_multi_kfunc_set_ids)
> > > +BTF_ID_FLAGS(func, bpf_kprobe_multi_is_return)
> > > +BTF_KFUNCS_END(kprobe_multi_kfunc_set_ids)
> > > +
> > > +static int bpf_kprobe_multi_filter(const struct bpf_prog *prog, u32 kfunc_id)
> > > +{
> > > +       if (!btf_id_set8_contains(&kprobe_multi_kfunc_set_ids, kfunc_id))
> > > +               return 0;
> > > +
> > > +       if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI)
> > > +               return -EACCES;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static const struct btf_kfunc_id_set bpf_kprobe_multi_kfunc_set = {
> > > +       .owner = THIS_MODULE,
> > > +       .set = &kprobe_multi_kfunc_set_ids,
> > > +       .filter = bpf_kprobe_multi_filter,
> > > +};
> > > +
> > > +static int __init bpf_kprobe_multi_kfuncs_init(void)
> > > +{
> > > +       return register_btf_kfunc_id_set(BPF_PROG_TYPE_KPROBE, &bpf_kprobe_multi_kfunc_set);
> > > +}
> > > +
> > > +late_initcall(bpf_kprobe_multi_kfuncs_init);
> > >  #else /* !CONFIG_FPROBE */
> > >  int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> > >  {
> > > --
> > > 2.43.2
> > >

Re: [PATCH RFCv2 bpf-next 2/4] bpf: Add bpf_kprobe_multi_is_return kfunc

[Jiri Olsa]

On Fri, Mar 01, 2024 at 10:01:16AM -0800, Andrii Nakryiko wrote:

sNIP

> > > > +__bpf_kfunc bool bpf_kprobe_multi_is_return(void)
> > >
> > > and for uprobes we'll have bpf_uprobe_multi_is_return?...
> >
> > yes, but now I'm thinking maybe we could also have 'session' api and
> > have single 'bpf_session_is_return' because both kprobe and uprobe
> > are KPROBE program type.. and align it together with other session
> > kfuncs:
> >
> >   bpf_session_is_return
> >   bpf_session_set_cookie
> >   bpf_session_get_cookie
> >
> 
> We can do that. But I was thinking more of a
> 
> u64 *bpf_session_cookie()
> 
> which would return a read/write pointer that BPF program can
> manipulate. Instead of doing two calls (get_cookie + set_cookie), it
> would be one call. Is there any benefit to having separate set/get
> cookie calls?

ok, that would be easier, will check on that

> 
> > >
> > > BTW, have you tried implementing a "session cookie" idea?
> >
> > yep, with a little fix [0] it's working on top of Masami's 'fprobe over fgraph'
> > changes, you can check last 2 patches in [1] .. I did not do this on top of the
> > current fprobe/rethook kernel code, because it seems it's about to go away
> 
> do you know what is the timeline for fprobe over fgraph work to be finished?

good question ;-) Masami, any idea?

fwiw there's new version needed for [1] fix

[1] https://lore.kernel.org/bpf/ZdyKaRiI-PnG80Q0@krava/

> 
> >
> > I still need to implement that on top of uprobes and I will send rfc, so we can
> > see all of it and discuss the interface
> >
> 
> great, yeah, I think the session cookie idea should go in at the same
> time, if possible, so that we can assume it is supported for new
> [ku]probe.wrapper programs.

makes sense, even though with new kfuncs detection stuff,
it will be easy to find out

jirka