bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs
@ 2024-01-25 20:55 Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports() Andrii Nakryiko
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

This patch set follows recent changes that added btf_decl_tag-based argument
annotation support for global subprogs. This time we add ability to pass
PTR_TO_BTF_ID (BTF-aware kernel pointers) arguments into global subprograms.
We support explicitly trusted arguments only, for now.

First three patches are preparatory. Patches #1 and #3 does post-BPF token
code adjustments, to undo merge conflict avoidance measures. Patch #2 makes
PERF_EVENT type enforcement logic aligned with kernel-side logic.

Patch #4 adds logic for arg:trusted tag support on the verifier side. Default
semantic of such arguments is non-NULL, enforced on caller side. But patch #5
adds arg:maybe_null tag that can be combined with arg:trusted to make callee
explicitly do the NULL check, which helps implement "optional" PTR_TO_BTF_ID
arguments.

Patch #6 adds libbpf-side __arg_trusted and __arg_maybe_null macros. Patch #7
adds a bunch of tests validating __arg_trusted in combination with
__arg_maybe_null.

v1->v2:
  - added fix up to type enforcement changes, landed earlier;
  - dropped bpf_core_cast() changes, will post them separately, as they now
    are not used in added tests;
  - dropped arg:untrusted support (Alexei);
  - renamed arg:nullable to arg:maybe_null (Alexei);
  - and also added task_struct___local flavor tests (Alexei).

Andrii Nakryiko (7):
  libbpf: integrate __arg_ctx feature detector into kernel_supports()
  libbpf: fix __arg_ctx type enforcement for perf_event programs
  bpf: move arg:ctx type enforcement check inside the main logic loop
  bpf: add __arg_trusted global func arg tag
  bpf: add arg:maybe_null tag to be combined with trusted pointers
  libbpf: add __arg_trusted and __arg_maybe_null tag macros
  selftests/bpf: add trusted global subprog arg tests

 include/linux/bpf_verifier.h                  |   1 +
 kernel/bpf/btf.c                              | 130 +++++++++++----
 kernel/bpf/verifier.c                         |  24 +++
 tools/lib/bpf/bpf_helpers.h                   |   2 +
 tools/lib/bpf/features.c                      |  58 +++++++
 tools/lib/bpf/libbpf.c                        |  86 +++-------
 tools/lib/bpf/libbpf_internal.h               |   2 +
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_global_ptr_args.c      | 156 ++++++++++++++++++
 9 files changed, 366 insertions(+), 95 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c

-- 
2.34.1


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

* [PATCH v2 bpf-next 1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports()
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs Andrii Nakryiko
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

Now that feature detection code is in bpf-next tree, integrate __arg_ctx
kernel-side support into kernel_supports() framework.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/features.c        | 58 +++++++++++++++++++++++++++++
 tools/lib/bpf/libbpf.c          | 65 +--------------------------------
 tools/lib/bpf/libbpf_internal.h |  2 +
 3 files changed, 61 insertions(+), 64 deletions(-)

diff --git a/tools/lib/bpf/features.c b/tools/lib/bpf/features.c
index 5a5c766bf615..6b0738ad7063 100644
--- a/tools/lib/bpf/features.c
+++ b/tools/lib/bpf/features.c
@@ -407,6 +407,61 @@ static int probe_kern_btf_enum64(int token_fd)
 					     strs, sizeof(strs), token_fd));
 }
 
+static int probe_kern_arg_ctx_tag(int token_fd)
+{
+	static const char strs[] = "\0a\0b\0arg:ctx\0";
+	const __u32 types[] = {
+		/* [1] INT */
+		BTF_TYPE_INT_ENC(1 /* "a" */, BTF_INT_SIGNED, 0, 32, 4),
+		/* [2] PTR -> VOID */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0),
+		/* [3] FUNC_PROTO `int(void *a)` */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1),
+		BTF_PARAM_ENC(1 /* "a" */, 2),
+		/* [4] FUNC 'a' -> FUNC_PROTO (main prog) */
+		BTF_TYPE_ENC(1 /* "a" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 3),
+		/* [5] FUNC_PROTO `int(void *b __arg_ctx)` */
+		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1),
+		BTF_PARAM_ENC(3 /* "b" */, 2),
+		/* [6] FUNC 'b' -> FUNC_PROTO (subprog) */
+		BTF_TYPE_ENC(3 /* "b" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 5),
+		/* [7] DECL_TAG 'arg:ctx' -> func 'b' arg 'b' */
+		BTF_TYPE_DECL_TAG_ENC(5 /* "arg:ctx" */, 6, 0),
+	};
+	const struct bpf_insn insns[] = {
+		/* main prog */
+		BPF_CALL_REL(+1),
+		BPF_EXIT_INSN(),
+		/* global subprog */
+		BPF_EMIT_CALL(BPF_FUNC_get_func_ip), /* needs PTR_TO_CTX */
+		BPF_EXIT_INSN(),
+	};
+	const struct bpf_func_info_min func_infos[] = {
+		{ 0, 4 }, /* main prog -> FUNC 'a' */
+		{ 2, 6 }, /* subprog -> FUNC 'b' */
+	};
+	LIBBPF_OPTS(bpf_prog_load_opts, opts,
+		.token_fd = token_fd,
+		.prog_flags = token_fd ? BPF_F_TOKEN_FD : 0,
+	);
+	int prog_fd, btf_fd, insn_cnt = ARRAY_SIZE(insns);
+
+	btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs), token_fd);
+	if (btf_fd < 0)
+		return 0;
+
+	opts.prog_btf_fd = btf_fd;
+	opts.func_info = &func_infos;
+	opts.func_info_cnt = ARRAY_SIZE(func_infos);
+	opts.func_info_rec_size = sizeof(func_infos[0]);
+
+	prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, "det_arg_ctx",
+				"GPL", insns, insn_cnt, &opts);
+	close(btf_fd);
+
+	return probe_fd(prog_fd);
+}
+
 typedef int (*feature_probe_fn)(int /* token_fd */);
 
 static struct kern_feature_cache feature_cache;
@@ -476,6 +531,9 @@ static struct kern_feature_desc {
 	[FEAT_UPROBE_MULTI_LINK] = {
 		"BPF multi-uprobe link support", probe_uprobe_multi_link,
 	},
+	[FEAT_ARG_CTX_TAG] = {
+		"kernel-side __arg_ctx tag", probe_kern_arg_ctx_tag,
+	},
 };
 
 bool feat_supported(struct kern_feature_cache *cache, enum kern_feature_id feat_id)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index fa7094ff3e66..41ab7a21f868 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6462,69 +6462,6 @@ static int clone_func_btf_info(struct btf *btf, int orig_fn_id, struct bpf_progr
 	return fn_id;
 }
 
-static int probe_kern_arg_ctx_tag(void)
-{
-	/* To minimize merge conflicts with BPF token series that refactors
-	 * feature detection code a lot, we don't integrate
-	 * probe_kern_arg_ctx_tag() into kernel_supports() feature-detection
-	 * framework yet, doing our own caching internally.
-	 * This will be cleaned up a bit later when bpf/bpf-next trees settle.
-	 */
-	static int cached_result = -1;
-	static const char strs[] = "\0a\0b\0arg:ctx\0";
-	const __u32 types[] = {
-		/* [1] INT */
-		BTF_TYPE_INT_ENC(1 /* "a" */, BTF_INT_SIGNED, 0, 32, 4),
-		/* [2] PTR -> VOID */
-		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 0),
-		/* [3] FUNC_PROTO `int(void *a)` */
-		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1),
-		BTF_PARAM_ENC(1 /* "a" */, 2),
-		/* [4] FUNC 'a' -> FUNC_PROTO (main prog) */
-		BTF_TYPE_ENC(1 /* "a" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 3),
-		/* [5] FUNC_PROTO `int(void *b __arg_ctx)` */
-		BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC_PROTO, 0, 1), 1),
-		BTF_PARAM_ENC(3 /* "b" */, 2),
-		/* [6] FUNC 'b' -> FUNC_PROTO (subprog) */
-		BTF_TYPE_ENC(3 /* "b" */, BTF_INFO_ENC(BTF_KIND_FUNC, 0, BTF_FUNC_GLOBAL), 5),
-		/* [7] DECL_TAG 'arg:ctx' -> func 'b' arg 'b' */
-		BTF_TYPE_DECL_TAG_ENC(5 /* "arg:ctx" */, 6, 0),
-	};
-	const struct bpf_insn insns[] = {
-		/* main prog */
-		BPF_CALL_REL(+1),
-		BPF_EXIT_INSN(),
-		/* global subprog */
-		BPF_EMIT_CALL(BPF_FUNC_get_func_ip), /* needs PTR_TO_CTX */
-		BPF_EXIT_INSN(),
-	};
-	const struct bpf_func_info_min func_infos[] = {
-		{ 0, 4 }, /* main prog -> FUNC 'a' */
-		{ 2, 6 }, /* subprog -> FUNC 'b' */
-	};
-	LIBBPF_OPTS(bpf_prog_load_opts, opts);
-	int prog_fd, btf_fd, insn_cnt = ARRAY_SIZE(insns);
-
-	if (cached_result >= 0)
-		return cached_result;
-
-	btf_fd = libbpf__load_raw_btf((char *)types, sizeof(types), strs, sizeof(strs), 0);
-	if (btf_fd < 0)
-		return 0;
-
-	opts.prog_btf_fd = btf_fd;
-	opts.func_info = &func_infos;
-	opts.func_info_cnt = ARRAY_SIZE(func_infos);
-	opts.func_info_rec_size = sizeof(func_infos[0]);
-
-	prog_fd = bpf_prog_load(BPF_PROG_TYPE_KPROBE, "det_arg_ctx",
-				"GPL", insns, insn_cnt, &opts);
-	close(btf_fd);
-
-	cached_result = probe_fd(prog_fd);
-	return cached_result;
-}
-
 /* Check if main program or global subprog's function prototype has `arg:ctx`
  * argument tags, and, if necessary, substitute correct type to match what BPF
  * verifier would expect, taking into account specific program type. This
@@ -6549,7 +6486,7 @@ static int bpf_program_fixup_func_info(struct bpf_object *obj, struct bpf_progra
 		return 0;
 
 	/* don't do any fix ups if kernel natively supports __arg_ctx */
-	if (probe_kern_arg_ctx_tag() > 0)
+	if (kernel_supports(obj, FEAT_ARG_CTX_TAG))
 		return 0;
 
 	/* some BPF program types just don't have named context structs, so
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index 930cc9616527..757dde832d8c 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -358,6 +358,8 @@ enum kern_feature_id {
 	FEAT_SYSCALL_WRAPPER,
 	/* BPF multi-uprobe link support */
 	FEAT_UPROBE_MULTI_LINK,
+	/* Kernel supports arg:ctx tag (__arg_ctx) for global subprogs natively */
+	FEAT_ARG_CTX_TAG,
 	__FEAT_CNT,
 };
 
-- 
2.34.1


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

* [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports() Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-26 13:24   ` Eduard Zingerman
  2024-01-25 20:55 ` [PATCH v2 bpf-next 3/7] bpf: move arg:ctx type enforcement check inside the main logic loop Andrii Nakryiko
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

Adjust PERF_EVENT type enforcement around __arg_ctx to match exactly
what kernel is doing.

Fixes: 76ec90a996e3 ("libbpf: warn on unexpected __arg_ctx type when rewriting BTF")
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/libbpf.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 41ab7a21f868..db65ea59a05a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -33,6 +33,7 @@
 #include <linux/filter.h>
 #include <linux/limits.h>
 #include <linux/perf_event.h>
+#include <linux/bpf_perf_event.h>
 #include <linux/ring_buffer.h>
 #include <sys/epoll.h>
 #include <sys/ioctl.h>
@@ -6339,6 +6340,14 @@ static struct {
 	/* all other program types don't have "named" context structs */
 };
 
+/* forward declarations for arch-specific underlying types of bpf_user_pt_regs_t typedef,
+ * for below __builtin_types_compatible_p() checks;
+ * with this approach we don't need any extra arch-specific #ifdef guards
+ */
+struct pt_regs;
+struct user_pt_regs;
+struct user_regs_struct;
+
 static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_program *prog,
 				     const char *subprog_name, int arg_idx,
 				     int arg_type_id, const char *ctx_name)
@@ -6379,11 +6388,21 @@ static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_pro
 	/* special cases */
 	switch (prog->type) {
 	case BPF_PROG_TYPE_KPROBE:
-	case BPF_PROG_TYPE_PERF_EVENT:
 		/* `struct pt_regs *` is expected, but we need to fix up */
 		if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
 			return true;
 		break;
+	case BPF_PROG_TYPE_PERF_EVENT:
+		if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct pt_regs) &&
+		    btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
+			return 0;
+		if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct user_pt_regs) &&
+		    btf_is_struct(t) && strcmp(tname, "user_pt_regs") == 0)
+			return 0;
+		if (__builtin_types_compatible_p(bpf_user_pt_regs_t, struct user_regs_struct) &&
+		    btf_is_struct(t) && strcmp(tname, "user_regs_struct") == 0)
+			return 0;
+		break;
 	case BPF_PROG_TYPE_RAW_TRACEPOINT:
 	case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
 		/* allow u64* as ctx */
-- 
2.34.1


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

* [PATCH v2 bpf-next 3/7] bpf: move arg:ctx type enforcement check inside the main logic loop
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports() Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 4/7] bpf: add __arg_trusted global func arg tag Andrii Nakryiko
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

Now that bpf and bpf-next trees converged and we don't run the risk of
merge conflicts, move btf_validate_prog_ctx_type() into its most logical
place inside the main logic loop.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/btf.c | 21 ++++-----------------
 1 file changed, 4 insertions(+), 17 deletions(-)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index edef96ceffa3..9ec08cfb2967 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7112,6 +7112,10 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				bpf_log(log, "arg#%d has invalid combination of tags\n", i);
 				return -EINVAL;
 			}
+			if ((tags & ARG_TAG_CTX) &&
+			    btf_validate_prog_ctx_type(log, btf, t, i, prog_type,
+						       prog->expected_attach_type))
+				return -EINVAL;
 			sub->args[i].arg_type = ARG_PTR_TO_CTX;
 			continue;
 		}
@@ -7156,23 +7160,6 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 		return -EINVAL;
 	}
 
-	for (i = 0; i < nargs; i++) {
-		const char *tag;
-
-		if (sub->args[i].arg_type != ARG_PTR_TO_CTX)
-			continue;
-
-		/* check if arg has "arg:ctx" tag */
-		t = btf_type_by_id(btf, args[i].type);
-		tag = btf_find_decl_tag_value(btf, fn_t, i, "arg:");
-		if (IS_ERR_OR_NULL(tag) || strcmp(tag, "ctx") != 0)
-			continue;
-
-		if (btf_validate_prog_ctx_type(log, btf, t, i, prog_type,
-					       prog->expected_attach_type))
-			return -EINVAL;
-	}
-
 	sub->arg_cnt = nargs;
 	sub->args_cached = true;
 
-- 
2.34.1


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

* [PATCH v2 bpf-next 4/7] bpf: add __arg_trusted global func arg tag
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (2 preceding siblings ...)
  2024-01-25 20:55 ` [PATCH v2 bpf-next 3/7] bpf: move arg:ctx type enforcement check inside the main logic loop Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 5/7] bpf: add arg:maybe_null tag to be combined with trusted pointers Andrii Nakryiko
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

Add support for passing PTR_TO_BTF_ID registers to global subprogs.
Currently only PTR_TRUSTED flavor of PTR_TO_BTF_ID is supported.
Non-NULL semantics is assumed, so caller will be forced to prove
PTR_TO_BTF_ID can't be NULL.

Note, we disallow global subprogs to destroy passed in PTR_TO_BTF_ID
arguments, even the trusted one. We achieve that by not setting
ref_obj_id when validating subprog code. This basically enforces (in
Rust terms) borrowing semantics vs move semantics. Borrowing semantics
seems to be a better fit for isolated global subprog validation
approach.

Implementation-wise, we utilize existing logic for matching
user-provided BTF type to kernel-side BTF type, used by BPF CO-RE logic
and following same matching rules. We enforce a unique match for types.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 include/linux/bpf_verifier.h |  1 +
 kernel/bpf/btf.c             | 99 +++++++++++++++++++++++++++++++-----
 kernel/bpf/verifier.c        | 24 +++++++++
 3 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 7f5816482a10..0dcde339dc7e 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -610,6 +610,7 @@ struct bpf_subprog_arg_info {
 	enum bpf_arg_type arg_type;
 	union {
 		u32 mem_size;
+		u32 btf_id;
 	};
 };
 
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 9ec08cfb2967..ed7a05815984 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6985,9 +6985,77 @@ static bool btf_is_dynptr_ptr(const struct btf *btf, const struct btf_type *t)
 	return false;
 }
 
+struct bpf_cand_cache {
+	const char *name;
+	u32 name_len;
+	u16 kind;
+	u16 cnt;
+	struct {
+		const struct btf *btf;
+		u32 id;
+	} cands[];
+};
+
+static DEFINE_MUTEX(cand_cache_mutex);
+
+static struct bpf_cand_cache *
+bpf_core_find_cands(struct bpf_core_ctx *ctx, u32 local_type_id);
+
+static int btf_get_ptr_to_btf_id(struct bpf_verifier_log *log, int arg_idx,
+				 const struct btf *btf, const struct btf_type *t)
+{
+	struct bpf_cand_cache *cc;
+	struct bpf_core_ctx ctx = {
+		.btf = btf,
+		.log = log,
+	};
+	u32 kern_type_id, type_id;
+	int err = 0;
+
+	/* skip PTR and modifiers */
+	type_id = t->type;
+	t = btf_type_by_id(btf, t->type);
+	while (btf_type_is_modifier(t)) {
+		type_id = t->type;
+		t = btf_type_by_id(btf, t->type);
+	}
+
+	mutex_lock(&cand_cache_mutex);
+	cc = bpf_core_find_cands(&ctx, type_id);
+	if (IS_ERR(cc)) {
+		err = PTR_ERR(cc);
+		bpf_log(log, "arg#%d reference type('%s %s') candidate matching error: %d\n",
+			arg_idx, btf_type_str(t), __btf_name_by_offset(btf, t->name_off),
+			err);
+		goto cand_cache_unlock;
+	}
+	if (cc->cnt != 1) {
+		bpf_log(log, "arg#%d reference type('%s %s') %s\n",
+			arg_idx, btf_type_str(t), __btf_name_by_offset(btf, t->name_off),
+			cc->cnt == 0 ? "has no matches" : "is ambiguous");
+		err = cc->cnt == 0 ? -ENOENT : -ESRCH;
+		goto cand_cache_unlock;
+	}
+	if (btf_is_module(cc->cands[0].btf)) {
+		bpf_log(log, "arg#%d reference type('%s %s') points to kernel module type (unsupported)\n",
+			arg_idx, btf_type_str(t), __btf_name_by_offset(btf, t->name_off));
+		err = -EOPNOTSUPP;
+		goto cand_cache_unlock;
+	}
+	kern_type_id = cc->cands[0].id;
+
+cand_cache_unlock:
+	mutex_unlock(&cand_cache_mutex);
+	if (err)
+		return err;
+
+	return kern_type_id;
+}
+
 enum btf_arg_tag {
 	ARG_TAG_CTX = 0x1,
 	ARG_TAG_NONNULL = 0x2,
+	ARG_TAG_TRUSTED = 0x4,
 };
 
 /* Process BTF of a function to produce high-level expectation of function
@@ -7089,6 +7157,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 
 			if (strcmp(tag, "ctx") == 0) {
 				tags |= ARG_TAG_CTX;
+			} else if (strcmp(tag, "trusted") == 0) {
+				tags |= ARG_TAG_TRUSTED;
 			} else if (strcmp(tag, "nonnull") == 0) {
 				tags |= ARG_TAG_NONNULL;
 			} else {
@@ -7127,6 +7197,22 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 			sub->args[i].arg_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY;
 			continue;
 		}
+		if (tags & ARG_TAG_TRUSTED) {
+			int kern_type_id;
+
+			if (tags & ARG_TAG_NONNULL) {
+				bpf_log(log, "arg#%d has invalid combination of tags\n", i);
+				return -EINVAL;
+			}
+
+			kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
+			if (kern_type_id < 0)
+				return kern_type_id;
+
+			sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_TRUSTED;
+			sub->args[i].btf_id = kern_type_id;
+			continue;
+		}
 		if (is_global) { /* generic user data pointer */
 			u32 mem_size;
 
@@ -8229,17 +8315,6 @@ size_t bpf_core_essential_name_len(const char *name)
 	return n;
 }
 
-struct bpf_cand_cache {
-	const char *name;
-	u32 name_len;
-	u16 kind;
-	u16 cnt;
-	struct {
-		const struct btf *btf;
-		u32 id;
-	} cands[];
-};
-
 static void bpf_free_cands(struct bpf_cand_cache *cands)
 {
 	if (!cands->cnt)
@@ -8260,8 +8335,6 @@ static struct bpf_cand_cache *vmlinux_cand_cache[VMLINUX_CAND_CACHE_SIZE];
 #define MODULE_CAND_CACHE_SIZE 31
 static struct bpf_cand_cache *module_cand_cache[MODULE_CAND_CACHE_SIZE];
 
-static DEFINE_MUTEX(cand_cache_mutex);
-
 static void __print_cand_cache(struct bpf_verifier_log *log,
 			       struct bpf_cand_cache **cache,
 			       int cache_size)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fe833e831cb6..4879dee8d725 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9336,6 +9336,18 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 			ret = process_dynptr_func(env, regno, -1, arg->arg_type, 0);
 			if (ret)
 				return ret;
+		} else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
+			struct bpf_call_arg_meta meta;
+			int err;
+
+			if (register_is_null(reg) && type_may_be_null(arg->arg_type))
+				continue;
+
+			memset(&meta, 0, sizeof(meta)); /* leave func_id as zero */
+			err = check_reg_type(env, regno, arg->arg_type, &arg->btf_id, &meta);
+			err = err ?: check_func_arg_reg_off(env, reg, regno, arg->arg_type);
+			if (err)
+				return err;
 		} else {
 			bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
 				i, arg->arg_type);
@@ -20137,6 +20149,18 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
 				mark_reg_known_zero(env, regs, i);
 				reg->mem_size = arg->mem_size;
 				reg->id = ++env->id_gen;
+			} else if (base_type(arg->arg_type) == ARG_PTR_TO_BTF_ID) {
+				reg->type = PTR_TO_BTF_ID;
+				if (arg->arg_type & PTR_MAYBE_NULL)
+					reg->type |= PTR_MAYBE_NULL;
+				if (arg->arg_type & PTR_UNTRUSTED)
+					reg->type |= PTR_UNTRUSTED;
+				if (arg->arg_type & PTR_TRUSTED)
+					reg->type |= PTR_TRUSTED;
+				mark_reg_known_zero(env, regs, i);
+				reg->btf = bpf_get_btf_vmlinux(); /* can't fail at this point */
+				reg->btf_id = arg->btf_id;
+				reg->id = ++env->id_gen;
 			} else {
 				WARN_ONCE(1, "BUG: unhandled arg#%d type %d\n",
 					  i - BPF_REG_1, arg->arg_type);
-- 
2.34.1


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

* [PATCH v2 bpf-next 5/7] bpf: add arg:maybe_null tag to be combined with trusted pointers
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (3 preceding siblings ...)
  2024-01-25 20:55 ` [PATCH v2 bpf-next 4/7] bpf: add __arg_trusted global func arg tag Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 6/7] libbpf: add __arg_trusted and __arg_maybe_null tag macros Andrii Nakryiko
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team

Add ability to mark arg:trusted arguments with optional arg:maybe_null
tag to mark is as PTR_TO_BTF_ID_OR_NULL variant, which will allow
callers to pass NULL, and subsequently will force global subprog's code
to do NULL check. This allows to have "optional" PTR_TO_BTF_ID values
passed into global subprogs.

For now arg:maybe_null cannot be combined with anything else.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/btf.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ed7a05815984..270804ece93c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7056,6 +7056,7 @@ enum btf_arg_tag {
 	ARG_TAG_CTX = 0x1,
 	ARG_TAG_NONNULL = 0x2,
 	ARG_TAG_TRUSTED = 0x4,
+	ARG_TAG_MAYBE_NULL = 0x8,
 };
 
 /* Process BTF of a function to produce high-level expectation of function
@@ -7161,6 +7162,8 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				tags |= ARG_TAG_TRUSTED;
 			} else if (strcmp(tag, "nonnull") == 0) {
 				tags |= ARG_TAG_NONNULL;
+			} else if (strcmp(tag, "maybe_null") == 0) {
+				tags |= ARG_TAG_MAYBE_NULL;
 			} else {
 				bpf_log(log, "arg#%d has unsupported set of tags\n", i);
 				return -EOPNOTSUPP;
@@ -7210,12 +7213,19 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
 				return kern_type_id;
 
 			sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_TRUSTED;
+			if (tags & ARG_TAG_MAYBE_NULL)
+				sub->args[i].arg_type |= PTR_MAYBE_NULL;
 			sub->args[i].btf_id = kern_type_id;
 			continue;
 		}
 		if (is_global) { /* generic user data pointer */
 			u32 mem_size;
 
+			if (tags & ARG_TAG_MAYBE_NULL) {
+				bpf_log(log, "arg#%d has invalid combination of tags\n", i);
+				return -EINVAL;
+			}
+
 			t = btf_type_skip_modifiers(btf, t->type, NULL);
 			ref_t = btf_resolve_size(btf, t, &mem_size);
 			if (IS_ERR(ref_t)) {
-- 
2.34.1


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

* [PATCH v2 bpf-next 6/7] libbpf: add __arg_trusted and __arg_maybe_null tag macros
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (4 preceding siblings ...)
  2024-01-25 20:55 ` [PATCH v2 bpf-next 5/7] bpf: add arg:maybe_null tag to be combined with trusted pointers Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-25 20:55 ` [PATCH v2 bpf-next 7/7] selftests/bpf: add trusted global subprog arg tests Andrii Nakryiko
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team, Eduard Zingerman

Add __arg_trusted to annotate global func args that accept trusted
PTR_TO_BTF_ID arguments.

Also add __arg_maybe_null to combine with __arg_trusted (and maybe other
tags in the future) to force global subprog itself (i.e., callee) to do
NULL checks, as opposed to default non-NULL semantics (and thus caller's
responsibility to ensure non-NULL values).

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 tools/lib/bpf/bpf_helpers.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 2324cc42b017..5f5901187c94 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -190,6 +190,8 @@ enum libbpf_tristate {
 
 #define __arg_ctx __attribute__((btf_decl_tag("arg:ctx")))
 #define __arg_nonnull __attribute((btf_decl_tag("arg:nonnull")))
+#define __arg_maybe_null __attribute((btf_decl_tag("arg:maybe_null")))
+#define __arg_trusted __attribute((btf_decl_tag("arg:trusted")))
 
 #ifndef ___bpf_concat
 #define ___bpf_concat(a, b) a ## b
-- 
2.34.1


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

* [PATCH v2 bpf-next 7/7] selftests/bpf: add trusted global subprog arg tests
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (5 preceding siblings ...)
  2024-01-25 20:55 ` [PATCH v2 bpf-next 6/7] libbpf: add __arg_trusted and __arg_maybe_null tag macros Andrii Nakryiko
@ 2024-01-25 20:55 ` Andrii Nakryiko
  2024-01-29 17:29 ` [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Eduard Zingerman
  2024-01-29 20:50 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-25 20:55 UTC (permalink / raw)
  To: bpf, ast, daniel, martin.lau; +Cc: andrii, kernel-team, Eduard Zingerman

Add a bunch of test cases validating behavior of __arg_trusted and its
combination with __arg_maybe_null tag. We also validate CO-RE flavor
support by kernel for __arg_trusted args.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/verifier.c       |   2 +
 .../bpf/progs/verifier_global_ptr_args.c      | 156 ++++++++++++++++++
 2 files changed, 158 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index d62c5bf00e71..9c6072a19745 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -28,6 +28,7 @@
 #include "verifier_div0.skel.h"
 #include "verifier_div_overflow.skel.h"
 #include "verifier_global_subprogs.skel.h"
+#include "verifier_global_ptr_args.skel.h"
 #include "verifier_gotol.skel.h"
 #include "verifier_helper_access_var_len.skel.h"
 #include "verifier_helper_packet_access.skel.h"
@@ -140,6 +141,7 @@ void test_verifier_direct_stack_access_wraparound(void) { RUN(verifier_direct_st
 void test_verifier_div0(void)                 { RUN(verifier_div0); }
 void test_verifier_div_overflow(void)         { RUN(verifier_div_overflow); }
 void test_verifier_global_subprogs(void)      { RUN(verifier_global_subprogs); }
+void test_verifier_global_ptr_args(void)      { RUN(verifier_global_ptr_args); }
 void test_verifier_gotol(void)                { RUN(verifier_gotol); }
 void test_verifier_helper_access_var_len(void) { RUN(verifier_helper_access_var_len); }
 void test_verifier_helper_packet_access(void) { RUN(verifier_helper_packet_access); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
new file mode 100644
index 000000000000..64ce4fac3137
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_global_ptr_args.c
@@ -0,0 +1,156 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include <bpf/bpf_core_read.h>
+#include "bpf_misc.h"
+#include "xdp_metadata.h"
+#include "bpf_kfuncs.h"
+
+extern struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym __weak;
+extern void bpf_task_release(struct task_struct *p) __ksym __weak;
+
+__weak int subprog_trusted_task_maybe_null(struct task_struct *task __arg_trusted __arg_maybe_null)
+{
+	if (!task)
+		return 0;
+	return task->pid + task->tgid;
+}
+
+SEC("?kprobe")
+__success __log_level(2)
+__msg("Validating subprog_trusted_task_maybe_null() func#1...")
+__msg(": R1=trusted_ptr_or_null_task_struct(")
+int trusted_task_arg_maybe_null(void *ctx)
+{
+	struct task_struct *t = bpf_get_current_task_btf();
+
+	return subprog_trusted_task_maybe_null(t) + subprog_trusted_task_maybe_null(NULL);
+}
+
+__weak int subprog_trusted_task_nonnull(struct task_struct *task __arg_trusted)
+{
+	return task->pid + task->tgid;
+}
+
+SEC("?kprobe")
+__failure __log_level(2)
+__msg("R1 type=scalar expected=ptr_, trusted_ptr_, rcu_ptr_")
+__msg("Caller passes invalid args into func#1 ('subprog_trusted_task_nonnull')")
+int trusted_task_arg_nonnull_fail1(void *ctx)
+{
+	return subprog_trusted_task_nonnull(NULL);
+}
+
+SEC("?tp_btf/task_newtask")
+__failure __log_level(2)
+__msg("R1 type=ptr_or_null_ expected=ptr_, trusted_ptr_, rcu_ptr_")
+__msg("Caller passes invalid args into func#1 ('subprog_trusted_task_nonnull')")
+int trusted_task_arg_nonnull_fail2(void *ctx)
+{
+	struct task_struct *t = bpf_get_current_task_btf();
+	struct task_struct *maybe_null;
+	int res;
+
+	maybe_null = bpf_task_acquire(t);
+
+	 /* should fail, PTR_TO_BTF_ID_OR_NULL */
+	res = subprog_trusted_task_nonnull(maybe_null);
+
+	if (maybe_null)
+		bpf_task_release(maybe_null);
+
+	return res;
+}
+
+SEC("?kprobe")
+__success __log_level(2)
+__msg("Validating subprog_trusted_task_nonnull() func#1...")
+__msg(": R1=trusted_ptr_task_struct(")
+int trusted_task_arg_nonnull(void *ctx)
+{
+	struct task_struct *t = bpf_get_current_task_btf();
+
+	return subprog_trusted_task_nonnull(t);
+}
+
+struct task_struct___local {} __attribute__((preserve_access_index));
+
+__weak int subprog_maybe_null_task_flavor(
+	struct task_struct___local *task __arg_trusted __arg_maybe_null)
+{
+	char buf[16];
+
+	if (!task)
+		return 0;
+
+	return bpf_copy_from_user_task(&buf, sizeof(buf), NULL, (void *)task, 0);
+}
+
+SEC("?uprobe.s")
+__success __log_level(2)
+__msg("Validating subprog_maybe_null_task_flavor() func#1...")
+__msg(": R1=trusted_ptr_or_null_task_struct(")
+int flavor_ptr_maybe_null(void *ctx)
+{
+	struct task_struct___local *t = (void *)bpf_get_current_task_btf();
+
+	return subprog_maybe_null_task_flavor(t);
+}
+
+__weak int subprog_nonnull_task_flavor(struct task_struct___local *task __arg_trusted)
+{
+	char buf[16];
+
+	return bpf_copy_from_user_task(&buf, sizeof(buf), NULL, (void *)task, 0);
+}
+
+SEC("?uprobe.s")
+__success __log_level(2)
+__msg("Validating subprog_nonnull_task_flavor() func#1...")
+__msg(": R1=trusted_ptr_task_struct(")
+int flavor_ptr_nonnull(void *ctx)
+{
+	struct task_struct *t = bpf_get_current_task_btf();
+
+	return subprog_nonnull_task_flavor((void *)t);
+}
+
+__weak int subprog_trusted_destroy(struct task_struct *task __arg_trusted)
+{
+	bpf_task_release(task); /* should be rejected */
+
+	return 0;
+}
+
+SEC("?tp_btf/task_newtask")
+__failure __log_level(2)
+__msg("release kernel function bpf_task_release expects refcounted PTR_TO_BTF_ID")
+int BPF_PROG(trusted_destroy_fail, struct task_struct *task, u64 clone_flags)
+{
+	return subprog_trusted_destroy(task);
+}
+
+__weak int subprog_trusted_acq_rel(struct task_struct *task __arg_trusted)
+{
+	struct task_struct *owned;
+
+	owned = bpf_task_acquire(task);
+	if (!owned)
+		return 0;
+
+	bpf_task_release(owned); /* this one is OK, we acquired it locally */
+
+	return 0;
+}
+
+SEC("?tp_btf/task_newtask")
+__success __log_level(2)
+int BPF_PROG(trusted_acq_rel, struct task_struct *task, u64 clone_flags)
+{
+	return subprog_trusted_acq_rel(task);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.34.1


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

* Re: [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
  2024-01-25 20:55 ` [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs Andrii Nakryiko
@ 2024-01-26 13:24   ` Eduard Zingerman
  2024-01-26 19:06     ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Eduard Zingerman @ 2024-01-26 13:24 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, ast, daniel, martin.lau; +Cc: kernel-team

On Thu, 2024-01-25 at 12:55 -0800, Andrii Nakryiko wrote:
[...]

> @@ -6379,11 +6388,21 @@ static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_pro
>  	/* special cases */
>  	switch (prog->type) {
>  	case BPF_PROG_TYPE_KPROBE:
> -	case BPF_PROG_TYPE_PERF_EVENT:
>  		/* `struct pt_regs *` is expected, but we need to fix up */
>  		if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
>  			return true;
>  		break;

Sorry, this was probably discussed, but I got lost a bit.
Kernel side does not change pt_regs for BPF_PROG_TYPE_KPROBE
(in ./kernel/bpf/btf.c:btf_validate_prog_ctx_type)
but here we do, why do it differently?

[...]

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

* Re: [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
  2024-01-26 13:24   ` Eduard Zingerman
@ 2024-01-26 19:06     ` Andrii Nakryiko
  2024-01-26 21:32       ` Eduard Zingerman
  0 siblings, 1 reply; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-26 19:06 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team

On Fri, Jan 26, 2024 at 5:24 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2024-01-25 at 12:55 -0800, Andrii Nakryiko wrote:
> [...]
>
> > @@ -6379,11 +6388,21 @@ static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_pro
> >       /* special cases */
> >       switch (prog->type) {
> >       case BPF_PROG_TYPE_KPROBE:
> > -     case BPF_PROG_TYPE_PERF_EVENT:
> >               /* `struct pt_regs *` is expected, but we need to fix up */
> >               if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
> >                       return true;
> >               break;
>
> Sorry, this was probably discussed, but I got lost a bit.
> Kernel side does not change pt_regs for BPF_PROG_TYPE_KPROBE
> (in ./kernel/bpf/btf.c:btf_validate_prog_ctx_type)
> but here we do, why do it differently?
>

Hm... We do the same. After this patch w end up with this logic on
libbpf side (which matches kernel-side one, I believe):

for KPROBE => allow pt_regs (unconditionally)
for PERF_EVENT => allow user_regs_struct|user_pt_regs|pt_regs,
depending on bpf_user_pt_regs_t definition on host platform

That should match what the kernel is doing.


> [...]

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

* Re: [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
  2024-01-26 19:06     ` Andrii Nakryiko
@ 2024-01-26 21:32       ` Eduard Zingerman
  2024-01-26 22:21         ` Andrii Nakryiko
  0 siblings, 1 reply; 14+ messages in thread
From: Eduard Zingerman @ 2024-01-26 21:32 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team

On Fri, 2024-01-26 at 11:06 -0800, Andrii Nakryiko wrote:
> On Fri, Jan 26, 2024 at 5:24 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Thu, 2024-01-25 at 12:55 -0800, Andrii Nakryiko wrote:
> > [...]
> > 
> > > @@ -6379,11 +6388,21 @@ static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_pro
> > >       /* special cases */
> > >       switch (prog->type) {
> > >       case BPF_PROG_TYPE_KPROBE:
> > > -     case BPF_PROG_TYPE_PERF_EVENT:
> > >               /* `struct pt_regs *` is expected, but we need to fix up */
> > >               if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
> > >                       return true;
> > >               break;
> > 
> > Sorry, this was probably discussed, but I got lost a bit.
> > Kernel side does not change pt_regs for BPF_PROG_TYPE_KPROBE
> > (in ./kernel/bpf/btf.c:btf_validate_prog_ctx_type)
> > but here we do, why do it differently?
> > 
> 
> Hm... We do the same. After this patch w end up with this logic on
> libbpf side (which matches kernel-side one, I believe):
> 
> for KPROBE => allow pt_regs (unconditionally)
> for PERF_EVENT => allow user_regs_struct|user_pt_regs|pt_regs,
> depending on bpf_user_pt_regs_t definition on host platform
> 
> That should match what the kernel is doing.

Oh..., I see:
After (and before) this patch on libbpf side for KPROBE/pt_regs
need_func_arg_type_fixup() would return true,
thus bpf_program_fixup_func_info() would apply type transformation
(convert it to bpf_user_pt_regs_t).
And kernel before the arg:ctx series expected bpf_user_pt_regs_t
for global subprograms called from KPROBE programs,
hence old kernel would accept program with KPROBE/pt_regs
thanks to libbpf manipulations.

I was put off by need_func_arg_type_fixup() returning true,
thus requiring change, and btf_validate_prog_ctx_type()
just accepting pt_regs => not doing anything.

Thank you for explaining.

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

* Re: [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
  2024-01-26 21:32       ` Eduard Zingerman
@ 2024-01-26 22:21         ` Andrii Nakryiko
  0 siblings, 0 replies; 14+ messages in thread
From: Andrii Nakryiko @ 2024-01-26 22:21 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team

On Fri, Jan 26, 2024 at 1:32 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2024-01-26 at 11:06 -0800, Andrii Nakryiko wrote:
> > On Fri, Jan 26, 2024 at 5:24 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Thu, 2024-01-25 at 12:55 -0800, Andrii Nakryiko wrote:
> > > [...]
> > >
> > > > @@ -6379,11 +6388,21 @@ static bool need_func_arg_type_fixup(const struct btf *btf, const struct bpf_pro
> > > >       /* special cases */
> > > >       switch (prog->type) {
> > > >       case BPF_PROG_TYPE_KPROBE:
> > > > -     case BPF_PROG_TYPE_PERF_EVENT:
> > > >               /* `struct pt_regs *` is expected, but we need to fix up */
> > > >               if (btf_is_struct(t) && strcmp(tname, "pt_regs") == 0)
> > > >                       return true;
> > > >               break;
> > >
> > > Sorry, this was probably discussed, but I got lost a bit.
> > > Kernel side does not change pt_regs for BPF_PROG_TYPE_KPROBE
> > > (in ./kernel/bpf/btf.c:btf_validate_prog_ctx_type)
> > > but here we do, why do it differently?
> > >
> >
> > Hm... We do the same. After this patch w end up with this logic on
> > libbpf side (which matches kernel-side one, I believe):
> >
> > for KPROBE => allow pt_regs (unconditionally)
> > for PERF_EVENT => allow user_regs_struct|user_pt_regs|pt_regs,
> > depending on bpf_user_pt_regs_t definition on host platform
> >
> > That should match what the kernel is doing.
>
> Oh..., I see:
> After (and before) this patch on libbpf side for KPROBE/pt_regs
> need_func_arg_type_fixup() would return true,
> thus bpf_program_fixup_func_info() would apply type transformation
> (convert it to bpf_user_pt_regs_t).
> And kernel before the arg:ctx series expected bpf_user_pt_regs_t
> for global subprograms called from KPROBE programs,
> hence old kernel would accept program with KPROBE/pt_regs
> thanks to libbpf manipulations.

Yep, with libbpf it's always a "time travel" kind of thinking, taking
into account old kernels.

>
> I was put off by need_func_arg_type_fixup() returning true,
> thus requiring change, and btf_validate_prog_ctx_type()
> just accepting pt_regs => not doing anything.
>
> Thank you for explaining.

Well... I didn't explain all the above, you pieced it all together yourself ;)

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

* Re: [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (6 preceding siblings ...)
  2024-01-25 20:55 ` [PATCH v2 bpf-next 7/7] selftests/bpf: add trusted global subprog arg tests Andrii Nakryiko
@ 2024-01-29 17:29 ` Eduard Zingerman
  2024-01-29 20:50 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 14+ messages in thread
From: Eduard Zingerman @ 2024-01-29 17:29 UTC (permalink / raw)
  To: Andrii Nakryiko, bpf, ast, daniel, martin.lau; +Cc: kernel-team

On Thu, 2024-01-25 at 12:55 -0800, Andrii Nakryiko wrote:
> This patch set follows recent changes that added btf_decl_tag-based argument
> annotation support for global subprogs. This time we add ability to pass
> PTR_TO_BTF_ID (BTF-aware kernel pointers) arguments into global subprograms.
> We support explicitly trusted arguments only, for now.
> 
> First three patches are preparatory. Patches #1 and #3 does post-BPF token
> code adjustments, to undo merge conflict avoidance measures. Patch #2 makes
> PERF_EVENT type enforcement logic aligned with kernel-side logic.
> 
> Patch #4 adds logic for arg:trusted tag support on the verifier side. Default
> semantic of such arguments is non-NULL, enforced on caller side. But patch #5
> adds arg:maybe_null tag that can be combined with arg:trusted to make callee
> explicitly do the NULL check, which helps implement "optional" PTR_TO_BTF_ID
> arguments.
> 
> Patch #6 adds libbpf-side __arg_trusted and __arg_maybe_null macros. Patch #7
> adds a bunch of tests validating __arg_trusted in combination with
> __arg_maybe_null.
> 
> v1->v2:
>   - added fix up to type enforcement changes, landed earlier;
>   - dropped bpf_core_cast() changes, will post them separately, as they now
>     are not used in added tests;
>   - dropped arg:untrusted support (Alexei);
>   - renamed arg:nullable to arg:maybe_null (Alexei);
>   - and also added task_struct___local flavor tests (Alexei).

Full patch-set looks good to me.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

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

* Re: [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs
  2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
                   ` (7 preceding siblings ...)
  2024-01-29 17:29 ` [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Eduard Zingerman
@ 2024-01-29 20:50 ` patchwork-bot+netdevbpf
  8 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-01-29 20:50 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: bpf, ast, daniel, martin.lau, kernel-team

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Thu, 25 Jan 2024 12:55:03 -0800 you wrote:
> This patch set follows recent changes that added btf_decl_tag-based argument
> annotation support for global subprogs. This time we add ability to pass
> PTR_TO_BTF_ID (BTF-aware kernel pointers) arguments into global subprograms.
> We support explicitly trusted arguments only, for now.
> 
> First three patches are preparatory. Patches #1 and #3 does post-BPF token
> code adjustments, to undo merge conflict avoidance measures. Patch #2 makes
> PERF_EVENT type enforcement logic aligned with kernel-side logic.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next,1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports()
    https://git.kernel.org/bpf/bpf-next/c/0e6d0a9d2348
  - [v2,bpf-next,2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs
    https://git.kernel.org/bpf/bpf-next/c/9eea8fafe33e
  - [v2,bpf-next,3/7] bpf: move arg:ctx type enforcement check inside the main logic loop
    https://git.kernel.org/bpf/bpf-next/c/add9c58cd44e
  - [v2,bpf-next,4/7] bpf: add __arg_trusted global func arg tag
    (no matching commit)
  - [v2,bpf-next,5/7] bpf: add arg:maybe_null tag to be combined with trusted pointers
    (no matching commit)
  - [v2,bpf-next,6/7] libbpf: add __arg_trusted and __arg_maybe_null tag macros
    (no matching commit)
  - [v2,bpf-next,7/7] selftests/bpf: add trusted global subprog arg tests
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2024-01-29 20:50 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-25 20:55 [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 1/7] libbpf: integrate __arg_ctx feature detector into kernel_supports() Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 2/7] libbpf: fix __arg_ctx type enforcement for perf_event programs Andrii Nakryiko
2024-01-26 13:24   ` Eduard Zingerman
2024-01-26 19:06     ` Andrii Nakryiko
2024-01-26 21:32       ` Eduard Zingerman
2024-01-26 22:21         ` Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 3/7] bpf: move arg:ctx type enforcement check inside the main logic loop Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 4/7] bpf: add __arg_trusted global func arg tag Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 5/7] bpf: add arg:maybe_null tag to be combined with trusted pointers Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 6/7] libbpf: add __arg_trusted and __arg_maybe_null tag macros Andrii Nakryiko
2024-01-25 20:55 ` [PATCH v2 bpf-next 7/7] selftests/bpf: add trusted global subprog arg tests Andrii Nakryiko
2024-01-29 17:29 ` [PATCH v2 bpf-next 0/7] Trusted PTR_TO_BTF_ID arg support in global subprogs Eduard Zingerman
2024-01-29 20:50 ` patchwork-bot+netdevbpf

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