[PATCH bpf-next v1 0/4] Unify helper and kfunc call_arg_meta

[PATCH bpf-next v1 0/4] Unify helper and kfunc call_arg_meta

[Amery Hung]

Hi all,

I am working toward unifying helper and kfunc handling in the verifier.
Historically, kfunc verification was mostly done separately from helper.
This causes code/logic duplication and may introduce subtle bugs as the
same checks are implemnented differently.

To address this, I plan to unify function call verification, which will
operate on a common function call descriptor bpf_func_proto.

As the first step, this patchset merges bpf_{kfunc_}call_arg_meta of
kfunc and helepr. The structure is used as a scratch pad for different
helper/kfunc verification mechanisms. The previous object tracking
refactor patchset has unified some of them. This patch handles the rest
and finally merge the two different structures.

The next step will be generating bpf_func_proto for kfunc before the
main verification loop and align kfunc and helper argument types.

Then hopefully helper and kfunc can go throguh the same call verification
path.


Amery Hung (4):
  bpf: Remove dynptr check in check_stack_range_initialized()
  bpf: Factor out raw_mode-related fields in bpf_call_arg_meta
  bpf: Unify helper and kfunc allocation-size argument handling
  bpf: Unify helper and kfunc call argument meta

 include/linux/bpf_verifier.h                  |  55 ++--
 kernel/bpf/cfg.c                              |   2 +-
 kernel/bpf/verifier.c                         | 253 +++++++-----------
 .../selftests/bpf/prog_tests/kfunc_call.c     |   2 +-
 .../testing/selftests/bpf/progs/dynptr_fail.c |   4 +-
 5 files changed, 146 insertions(+), 170 deletions(-)

-- 
2.52.0

[PATCH bpf-next v1 1/4] bpf: Remove dynptr check in check_stack_range_initialized()

[Amery Hung]

For a MEM_UNINIT ("raw mode") helper argument,
check_stack_range_initialized() open-coded a scan that rejected any
STACK_DYNPTR slot in the range with "potential write to dynptr". This
duplicated, and was stricter than, the handling that runs when the
buffer is actually marked initialized. check_helper_call() later replays
the write byte by byte via check_mem_access(), which goes through
destroy_if_dynptr_stack_slot(), which rejects overwritting a referenced
dynptr. Therefore drop the redundant scan and rely on check_mem_access().

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/verifier.c                         | 25 -------------------
 .../testing/selftests/bpf/progs/dynptr_fail.c |  4 +--
 2 files changed, 2 insertions(+), 27 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 03e2202cca13..7dd961ede88d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6733,31 +6733,6 @@ static int check_stack_range_initialized(
 	}
 
 	if (meta && meta->raw_mode) {
-		/* Ensure we won't be overwriting dynptrs when simulating byte
-		 * by byte access in check_helper_call using meta.access_size.
-		 * This would be a problem if we have a helper in the future
-		 * which takes:
-		 *
-		 *	helper(uninit_mem, len, dynptr)
-		 *
-		 * Now, uninint_mem may overlap with dynptr pointer. Hence, it
-		 * may end up writing to dynptr itself when touching memory from
-		 * arg 1. This can be relaxed on a case by case basis for known
-		 * safe cases, but reject due to the possibilitiy of aliasing by
-		 * default.
-		 */
-		for (i = min_off; i < max_off + access_size; i++) {
-			int stack_off = -i - 1;
-
-			spi = bpf_get_spi(i);
-			/* raw_mode may write past allocated_stack */
-			if (state->allocated_stack <= stack_off)
-				continue;
-			if (state->stack[spi].slot_type[stack_off % BPF_REG_SIZE] == STACK_DYNPTR) {
-				verbose(env, "potential write to dynptr at off=%d disallowed\n", i);
-				return -EACCES;
-			}
-		}
 		meta->access_size = access_size;
 		meta->regno = reg_from_argno(argno);
 		return 0;
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index 344fb2aa0813..94489ac64da8 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -1112,7 +1112,7 @@ int dynptr_overwrite_ref(void *ctx)
 
 /* Reject writes to dynptr slot from bpf_dynptr_read */
 SEC("?raw_tp")
-__failure __msg("potential write to dynptr at off=-16")
+__failure __msg("cannot overwrite referenced dynptr")
 int dynptr_read_into_slot(void *ctx)
 {
 	union {
@@ -1558,7 +1558,7 @@ int BPF_PROG(skb_invalid_ctx_fexit, void *skb)
 
 /* Reject writes to dynptr slot for uninit arg */
 SEC("?raw_tp")
-__failure __msg("potential write to dynptr at off=-16")
+__failure __msg("cannot overwrite referenced dynptr")
 int uninit_write_into_slot(void *ctx)
 {
 	struct {
-- 
2.52.0

[PATCH bpf-next v1 2/4] bpf: Factor out raw_mode-related fields in bpf_call_arg_meta

[Amery Hung]

To prepare for unifying the helper and kfunc call_arg_meta, group the
scattered MEM_UNINIT ("raw") memory argument fields (raw_mode, regno and
access_size) into a new struct arg_raw_mem_desc. The intention is to
make it clear about when these are set and used instead of fields with
overly generic names.

Identify the raw argument once, up front, in check_raw_mode_ok() (
like check_proto_release_reg() does for release_regno), recording its
regno. check_stack_range_initialized() now recognizes the raw buffer by
matching that regno, so the separate raw_mode flag is no longer needed.

Also drop the bogus raw_mode assignment in the ARG_PTR_TO_MAP_VALUE case:
arg_type_is_raw_mem() only matches ARG_PTR_TO_MEM | MEM_UNINIT, and a map
value pointer is handled by check_map_access(), never reaching
check_stack_range_initialized(), so tagging it raw mode had no effect.

No functional change intended. This patch does not enable raw_mode
memory access for kfunc (i.e., uninit stack will not be allowed to be
passed to kfunc for unprivileged programs). Existing kfuncs with arguments
tagged with __uninit are either priviledged or dynptr kfuncs, which take
another path to make sure the access is checked by check_mem_access().

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 include/linux/bpf_verifier.h | 10 ++++++++
 kernel/bpf/verifier.c        | 50 +++++++++++++++---------------------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 317e99b9acc0..056b99d26cca 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1464,6 +1464,15 @@ struct ref_obj_desc {
 	u8 cnt;
 };
 
+/*
+ * A memory argument a call fills in. The verifier allows the stack to be uninitialized if
+ * the range is a known constant. Stack slots are marked as STACK_MISC by check_mem_access().
+ */
+struct arg_raw_mem_desc {
+	u8 regno;
+	int size;
+};
+
 struct bpf_kfunc_call_arg_meta {
 	/* In parameters */
 	struct btf *btf;
@@ -1510,6 +1519,7 @@ struct bpf_kfunc_call_arg_meta {
 	struct bpf_map_desc map;
 	struct bpf_dynptr_desc dynptr;
 	struct ref_obj_desc ref_obj;
+	struct arg_raw_mem_desc arg_raw_mem;
 	u64 mem_size;
 };
 
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7dd961ede88d..e2a3ed67d2eb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -256,11 +256,9 @@ struct bpf_call_arg_meta {
 	struct bpf_map_desc map;
 	struct bpf_dynptr_desc dynptr;
 	struct ref_obj_desc ref_obj;
-	bool raw_mode;
+	struct arg_raw_mem_desc arg_raw_mem;
 	bool pkt_access;
 	u8 release_regno;
-	int regno;
-	int access_size;
 	int mem_size;
 	u64 msize_max_value;
 	int func_id;
@@ -6690,6 +6688,8 @@ static int check_stack_range_initialized(
 	 * but BTF based global subprog validation isn't accurate enough.
 	 */
 	bool allow_poison = access_size < 0 || clobber;
+	/* The call will initialize the memory; uninitialized stack allowed */
+	bool raw_mode = meta && meta->arg_raw_mem.regno == reg_from_argno(argno);
 
 	access_size = abs(access_size);
 
@@ -6725,16 +6725,14 @@ static int check_stack_range_initialized(
 		 * helper return since specific bounds are unknown what may
 		 * cause uninitialized stack leaking.
 		 */
-		if (meta && meta->raw_mode)
-			meta = NULL;
+		raw_mode = false;
 
 		min_off = reg_smin(reg) + off;
 		max_off = reg_smax(reg) + off;
 	}
 
-	if (meta && meta->raw_mode) {
-		meta->access_size = access_size;
-		meta->regno = reg_from_argno(argno);
+	if (raw_mode) {
+		meta->arg_raw_mem.size = access_size;
 		return 0;
 	}
 
@@ -8403,7 +8401,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			verifier_bug(env, "invalid map_ptr to access map->value");
 			return -EFAULT;
 		}
-		meta->raw_mode = arg_type & MEM_UNINIT;
 		err = check_helper_mem_access(env, reg, argno_from_reg(regno), meta->map.ptr->value_size,
 					      arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
 					      false, meta);
@@ -8446,7 +8443,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		/* The access to this pointer is only checked when we hit the
 		 * next is_mem_size argument below.
 		 */
-		meta->raw_mode = arg_type & MEM_UNINIT;
 		if (arg_type & MEM_FIXED_SIZE) {
 			err = check_helper_mem_access(env, reg, argno_from_reg(regno), fn->arg_size[arg],
 						      arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
@@ -8797,26 +8793,19 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
 	return -EINVAL;
 }
 
-static bool check_raw_mode_ok(const struct bpf_func_proto *fn)
+static bool check_raw_mode_ok(const struct bpf_func_proto *fn, struct bpf_call_arg_meta *meta)
 {
-	int count = 0;
+	int i;
 
-	if (arg_type_is_raw_mem(fn->arg1_type))
-		count++;
-	if (arg_type_is_raw_mem(fn->arg2_type))
-		count++;
-	if (arg_type_is_raw_mem(fn->arg3_type))
-		count++;
-	if (arg_type_is_raw_mem(fn->arg4_type))
-		count++;
-	if (arg_type_is_raw_mem(fn->arg5_type))
-		count++;
+	for (i = 0; i < ARRAY_SIZE(fn->arg_type); i++) {
+		if (!arg_type_is_raw_mem(fn->arg_type[i]))
+			continue;
+		if (meta->arg_raw_mem.regno)
+			return false;
+		meta->arg_raw_mem.regno = i + 1;
+	}
 
-	/* We only support one arg being in raw mode at the moment,
-	 * which is sufficient for the helper functions we have
-	 * right now.
-	 */
-	return count <= 1;
+	return true;
 }
 
 static bool check_args_pair_invalid(const struct bpf_func_proto *fn, int arg)
@@ -8906,7 +8895,7 @@ static bool check_proto_release_reg(const struct bpf_func_proto *fn, struct bpf_
 
 static int check_func_proto(const struct bpf_func_proto *fn, struct bpf_call_arg_meta *meta)
 {
-	return check_raw_mode_ok(fn) &&
+	return check_raw_mode_ok(fn, meta) &&
 	       check_arg_pair_ok(fn) &&
 	       check_mem_arg_rw_flag_ok(fn) &&
 	       check_proto_release_reg(fn, meta) &&
@@ -10303,8 +10292,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	/* Mark slots with STACK_MISC in case of raw mode, stack offset
 	 * is inferred from register state.
 	 */
-	for (i = 0; i < meta.access_size; i++) {
-		err = check_mem_access(env, insn_idx, regs + meta.regno, argno_from_reg(meta.regno), i, BPF_B,
+	for (i = 0; i < meta.arg_raw_mem.size; i++) {
+		err = check_mem_access(env, insn_idx, regs + meta.arg_raw_mem.regno,
+				       argno_from_reg(meta.arg_raw_mem.regno), i, BPF_B,
 				       BPF_WRITE, -1, false, false);
 		if (err)
 			return err;
-- 
2.52.0

[PATCH bpf-next v1 3/4] bpf: Unify helper and kfunc allocation-size argument handling

[Amery Hung]

The constant "size of the PTR_TO_MEM returned in R0" argument is handled by
both helpers (ARG_CONST_ALLOC_SIZE_OR_ZERO) and kfuncs (__rdonly_buf_size /
__rdwr_buf_size), each with its own meta field (meta->mem_size,
meta->r0_size) and duplicated validation. Add struct arg_alloc_mem_desc
and a shared process_const_alloc_mem_size(), and replace both fields with
meta->arg_alloc_mem.

The desc records presence with a 'found' flag instead of using a non-zero
size as the sentinel. This also fixes a pre-existing bug on the kfunc
return path: "no size argument" was tested as r0_size == 0, so an explicit
__rdonly_buf_size/__rdwr_buf_size of 0 was treated as absent and fell
through to btf_resolve_size(), giving R0 the size of the pointed-to return
type instead of 0. With 'found', an explicit zero size is honored and
btf_resolve_size() is used only when no size argument was passed.

The size is stored in a u32, matching regs[R0].mem_size. The U32_MAX
check now apply to both helper and kfunc through
process_const_alloc_mem_size().

Fold bpf_session_cookie return size assignment into current kfunc return
size resolution path.

Note that verifier saves kfunc return size through r0_size instead of
mem_size. The later has no active readers so remove it.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 include/linux/bpf_verifier.h                  |  9 +-
 kernel/bpf/verifier.c                         | 87 ++++++++++---------
 .../selftests/bpf/prog_tests/kfunc_call.c     |  2 +-
 3 files changed, 55 insertions(+), 43 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 056b99d26cca..3ed788ec6afc 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1473,6 +1473,12 @@ struct arg_raw_mem_desc {
 	int size;
 };
 
+/* Size of PTR_TO_MEM returned, taken from a constant allocation-size argument */
+struct arg_alloc_mem_desc {
+	u32 size;
+	bool found;
+};
+
 struct bpf_kfunc_call_arg_meta {
 	/* In parameters */
 	struct btf *btf;
@@ -1484,7 +1490,6 @@ struct bpf_kfunc_call_arg_meta {
 	u8 release_regno;
 	bool r0_rdonly;
 	u32 ret_btf_id;
-	u64 r0_size;
 	u32 subprogno;
 	struct {
 		u64 value;
@@ -1520,7 +1525,7 @@ struct bpf_kfunc_call_arg_meta {
 	struct bpf_dynptr_desc dynptr;
 	struct ref_obj_desc ref_obj;
 	struct arg_raw_mem_desc arg_raw_mem;
-	u64 mem_size;
+	struct arg_alloc_mem_desc arg_alloc_mem;
 };
 
 int bpf_get_helper_proto(struct bpf_verifier_env *env, int func_id,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e2a3ed67d2eb..fcd9715eff01 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -257,9 +257,9 @@ struct bpf_call_arg_meta {
 	struct bpf_dynptr_desc dynptr;
 	struct ref_obj_desc ref_obj;
 	struct arg_raw_mem_desc arg_raw_mem;
+	struct arg_alloc_mem_desc arg_alloc_mem;
 	bool pkt_access;
 	u8 release_regno;
-	int mem_size;
 	u64 msize_max_value;
 	int func_id;
 	struct btf *btf;
@@ -6974,6 +6974,40 @@ static int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg
 	return err;
 }
 
+static int process_const_alloc_mem_size(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
+					int regno, argno_t argno,
+					struct arg_alloc_mem_desc *arg_alloc_mem)
+{
+	int err;
+
+	if (arg_alloc_mem->found) {
+		verifier_bug(env, "only one allocation size argument permitted");
+		return -EFAULT;
+	}
+
+	if (!tnum_is_const(reg->var_off)) {
+		verbose(env, "%s is not a const\n", reg_arg_name(env, argno));
+		return -EINVAL;
+	}
+
+	if (reg->var_off.value > U32_MAX) {
+		verbose(env, "%s allocation size exceeds u32 max\n", reg_arg_name(env, argno));
+		return -EINVAL;
+	}
+
+	if (regno >= 0)
+		err = mark_chain_precision(env, regno);
+	else
+		err = mark_stack_arg_precision(env, arg_idx_from_argno(argno));
+	if (err)
+		return err;
+
+	arg_alloc_mem->size = reg->var_off.value;
+	arg_alloc_mem->found = true;
+
+	return 0;
+}
+
 static int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *mem_reg,
 				    struct bpf_reg_state *size_reg, argno_t mem_argno, argno_t size_argno)
 {
@@ -8474,13 +8508,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			return err;
 		break;
 	case ARG_CONST_ALLOC_SIZE_OR_ZERO:
-		if (!tnum_is_const(reg->var_off)) {
-			verbose(env, "R%d is not a known constant'\n",
-				regno);
-			return -EACCES;
-		}
-		meta->mem_size = reg->var_off.value;
-		err = mark_chain_precision(env, regno);
+		err = process_const_alloc_mem_size(env, reg, regno, argno, &meta->arg_alloc_mem);
 		if (err)
 			return err;
 		break;
@@ -10512,7 +10540,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	case RET_PTR_TO_MEM:
 		mark_reg_known_zero(env, regs, BPF_REG_0);
 		regs[BPF_REG_0].type = PTR_TO_MEM | ret_flag;
-		regs[BPF_REG_0].mem_size = meta.mem_size;
+		regs[BPF_REG_0].mem_size = meta.arg_alloc_mem.size;
 		break;
 	case RET_PTR_TO_MEM_OR_BTF_ID:
 	{
@@ -12062,28 +12090,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 			}
 
 			if (is_ret_buf_sz) {
-				if (meta->r0_size) {
-					verbose(env, "2 or more rdonly/rdwr_buf_size parameters for kfunc");
-					return -EINVAL;
-				}
-
-				if (!tnum_is_const(reg->var_off)) {
-					verbose(env, "%s is not a const\n",
-						reg_arg_name(env, argno));
-					return -EINVAL;
-				}
-
-				meta->r0_size = reg->var_off.value;
-				if (meta->r0_size > U32_MAX) {
-					verbose(env, "%s rdonly/rdwr_buf_size exceeds u32 max\n",
-						reg_arg_name(env, argno));
-					return -EINVAL;
-				}
-				if (regno >= 0)
-					ret = mark_chain_precision(env, regno);
-				else
-					ret = mark_stack_arg_precision(env, i);
-				if (ret)
+				ret = process_const_alloc_mem_size(env, reg, regno, argno, &meta->arg_alloc_mem);
+				if (ret < 0)
 					return ret;
 			}
 			continue;
@@ -13062,11 +13070,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		}
 	}
 
-	if (meta.func_id == special_kfunc_list[KF_bpf_session_cookie]) {
-		meta.r0_size = sizeof(u64);
-		meta.r0_rdonly = false;
-	}
-
 	if (is_bpf_wq_set_callback_kfunc(meta.func_id)) {
 		err = push_callback_call(env, insn, insn_idx, meta.subprogno,
 					 set_timer_callback_state);
@@ -13199,15 +13202,19 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			/* kfunc returning 'void *' is equivalent to returning scalar */
 			mark_reg_unknown(env, regs, BPF_REG_0);
 		} else if (!__btf_type_is_struct(ptr_type)) {
-			if (!meta.r0_size) {
+			if (!meta.arg_alloc_mem.found) {
 				__u32 sz;
 
 				if (!IS_ERR(btf_resolve_size(desc_btf, ptr_type, &sz))) {
-					meta.r0_size = sz;
+					meta.arg_alloc_mem.found = true;
+					meta.arg_alloc_mem.size = sz;
 					meta.r0_rdonly = true;
 				}
+
+				if (meta.func_id == special_kfunc_list[KF_bpf_session_cookie])
+					meta.r0_rdonly = false;
 			}
-			if (!meta.r0_size) {
+			if (!meta.arg_alloc_mem.found) {
 				ptr_type_name = btf_name_by_offset(desc_btf,
 								   ptr_type->name_off);
 				verbose(env,
@@ -13220,7 +13227,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 
 			mark_reg_known_zero(env, regs, BPF_REG_0);
 			regs[BPF_REG_0].type = PTR_TO_MEM;
-			regs[BPF_REG_0].mem_size = meta.r0_size;
+			regs[BPF_REG_0].mem_size = meta.arg_alloc_mem.size;
 
 			if (meta.r0_rdonly)
 				regs[BPF_REG_0].type |= MEM_RDONLY;
diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index 67a30bf69509..c9fce95d220e 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -66,7 +66,7 @@ static struct kfunc_test_params kfunc_tests[] = {
 	TC_FAIL(kfunc_call_test_get_mem_fail_rdonly, 0, "R0 cannot write into rdonly_mem"),
 	TC_FAIL(kfunc_call_test_get_mem_fail_use_after_free, 0, "invalid mem access 'scalar'"),
 	TC_FAIL(kfunc_call_test_get_mem_fail_oob, 0, "min value is outside of the allowed memory range"),
-	TC_FAIL(kfunc_call_test_get_mem_fail_oversized, 0, "rdonly/rdwr_buf_size exceeds u32 max"),
+	TC_FAIL(kfunc_call_test_get_mem_fail_oversized, 0, "allocation size exceeds u32 max"),
 	TC_FAIL(kfunc_call_test_get_mem_fail_not_const, 0, "is not a const"),
 	TC_FAIL(kfunc_call_test_mem_acquire_fail, 0, "acquire kernel function does not return PTR_TO_BTF_ID"),
 	TC_FAIL(kfunc_call_test_pointer_arg_type_mismatch, 0, "R1 expected pointer to ctx, but got scalar"),
-- 
2.52.0

[PATCH bpf-next v1 4/4] bpf: Unify helper and kfunc call argument meta

[Amery Hung]

Helper and kfunc argument checking carried two separate meta structs: the
verifier-local struct bpf_call_arg_meta and bpf_kfunc_call_arg_meta.
Merge them into a single struct bpf_call_arg_meta. This is groundwork for
sharing argument checking between helpers and kfuncs.

While merging, drop the btf_id field from the helper meta since it is
never used.

No functional change.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 include/linux/bpf_verifier.h | 40 +++++++++------
 kernel/bpf/cfg.c             |  2 +-
 kernel/bpf/verifier.c        | 95 +++++++++++++++---------------------
 3 files changed, 63 insertions(+), 74 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 3ed788ec6afc..5fbc3dfe5b19 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1479,18 +1479,24 @@ struct arg_alloc_mem_desc {
 	bool found;
 };
 
-struct bpf_kfunc_call_arg_meta {
-	/* In parameters */
+struct bpf_call_arg_meta {
+	/* Common */
 	struct btf *btf;
 	u32 func_id;
-	u32 kfunc_flags;
-	const struct btf_type *func_proto;
-	const char *func_name;
-	/* Out parameters */
 	u8 release_regno;
-	bool r0_rdonly;
 	u32 ret_btf_id;
 	u32 subprogno;
+	struct bpf_map_desc map;
+	struct bpf_dynptr_desc dynptr;
+	struct ref_obj_desc ref_obj;
+	struct arg_raw_mem_desc arg_raw_mem;
+	struct arg_alloc_mem_desc arg_alloc_mem;
+
+	/* Only set by kfunc */
+	bool r0_rdonly;
+	u32 kfunc_flags;
+	const struct btf_type *func_proto;
+	const char *func_name;
 	struct {
 		u64 value;
 		bool found;
@@ -1521,29 +1527,31 @@ struct bpf_kfunc_call_arg_meta {
 		u8 spi;
 		u8 frameno;
 	} iter;
-	struct bpf_map_desc map;
-	struct bpf_dynptr_desc dynptr;
-	struct ref_obj_desc ref_obj;
-	struct arg_raw_mem_desc arg_raw_mem;
-	struct arg_alloc_mem_desc arg_alloc_mem;
+
+	/* Only set by helper */
+	bool pkt_access;
+	u64 msize_max_value;
+	s64 const_map_key;
+	struct btf *ret_btf;
+	struct btf_field *kptr_field;
 };
 
 int bpf_get_helper_proto(struct bpf_verifier_env *env, int func_id,
 			 const struct bpf_func_proto **ptr);
 int bpf_fetch_kfunc_arg_meta(struct bpf_verifier_env *env, s32 func_id,
-			     s16 offset, struct bpf_kfunc_call_arg_meta *meta);
+			     s16 offset, struct bpf_call_arg_meta *meta);
 bool bpf_is_async_callback_calling_insn(struct bpf_insn *insn);
 bool bpf_is_sync_callback_calling_insn(struct bpf_insn *insn);
-static inline bool bpf_is_iter_next_kfunc(struct bpf_kfunc_call_arg_meta *meta)
+static inline bool bpf_is_iter_next_kfunc(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_ITER_NEXT;
 }
 
-static inline bool bpf_is_kfunc_sleepable(struct bpf_kfunc_call_arg_meta *meta)
+static inline bool bpf_is_kfunc_sleepable(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_SLEEPABLE;
 }
-bool bpf_is_kfunc_pkt_changing(struct bpf_kfunc_call_arg_meta *meta);
+bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta);
 struct bpf_iarray *bpf_iarray_realloc(struct bpf_iarray *old, size_t n_elem);
 int bpf_copy_insn_array_uniq(struct bpf_map *map, u32 start, u32 end, u32 *off);
 bool bpf_insn_is_cond_jump(u8 code);
diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
index 26d37066465f..db3416a7c904 100644
--- a/kernel/bpf/cfg.c
+++ b/kernel/bpf/cfg.c
@@ -491,7 +491,7 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
 					return ret;
 			}
 		} else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {
-			struct bpf_kfunc_call_arg_meta meta;
+			struct bpf_call_arg_meta meta;
 
 			ret = bpf_fetch_kfunc_arg_meta(env, insn->imm, insn->off, &meta);
 			if (ret == 0 && bpf_is_iter_next_kfunc(&meta)) {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fcd9715eff01..5ccb00a8c84c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -252,25 +252,6 @@ static int validate_ref_obj(struct bpf_verifier_env *env, struct ref_obj_desc *r
 	return 0;
 }
 
-struct bpf_call_arg_meta {
-	struct bpf_map_desc map;
-	struct bpf_dynptr_desc dynptr;
-	struct ref_obj_desc ref_obj;
-	struct arg_raw_mem_desc arg_raw_mem;
-	struct arg_alloc_mem_desc arg_alloc_mem;
-	bool pkt_access;
-	u8 release_regno;
-	u64 msize_max_value;
-	int func_id;
-	struct btf *btf;
-	u32 btf_id;
-	struct btf *ret_btf;
-	u32 ret_btf_id;
-	u32 subprogno;
-	struct btf_field *kptr_field;
-	s64 const_map_key;
-};
-
 struct bpf_kfunc_meta {
 	struct btf *btf;
 	const struct btf_type *proto;
@@ -928,10 +909,10 @@ static void __mark_reg_known_zero(struct bpf_reg_state *reg);
 
 static bool in_rcu_cs(struct bpf_verifier_env *env);
 
-static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta);
+static bool is_kfunc_rcu_protected(struct bpf_call_arg_meta *meta);
 
 static int mark_stack_slots_iter(struct bpf_verifier_env *env,
-				 struct bpf_kfunc_call_arg_meta *meta,
+				 struct bpf_call_arg_meta *meta,
 				 struct bpf_reg_state *reg, int insn_idx,
 				 struct btf *btf, u32 btf_id, int nr_slots)
 {
@@ -1064,7 +1045,7 @@ static int acquire_irq_state(struct bpf_verifier_env *env, int insn_idx);
 static int release_irq_state(struct bpf_verifier_state *state, int id);
 
 static int mark_stack_slot_irq_flag(struct bpf_verifier_env *env,
-				     struct bpf_kfunc_call_arg_meta *meta,
+				     struct bpf_call_arg_meta *meta,
 				     struct bpf_reg_state *reg, int insn_idx,
 				     int kfunc_class)
 {
@@ -7246,7 +7227,7 @@ static int process_timer_helper(struct bpf_verifier_env *env, struct bpf_reg_sta
 }
 
 static int process_timer_kfunc(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno,
-			       struct bpf_kfunc_call_arg_meta *meta)
+			       struct bpf_call_arg_meta *meta)
 {
 	return process_timer_func(env, reg, argno, &meta->map);
 }
@@ -7413,23 +7394,23 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat
 	return err;
 }
 
-static bool is_iter_kfunc(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_iter_kfunc(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & (KF_ITER_NEW | KF_ITER_NEXT | KF_ITER_DESTROY);
 }
 
-static bool is_iter_new_kfunc(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_iter_new_kfunc(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_ITER_NEW;
 }
 
 
-static bool is_iter_destroy_kfunc(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_iter_destroy_kfunc(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_ITER_DESTROY;
 }
 
-static bool is_kfunc_arg_iter(struct bpf_kfunc_call_arg_meta *meta, int arg_idx,
+static bool is_kfunc_arg_iter(struct bpf_call_arg_meta *meta, int arg_idx,
 			      const struct btf_param *arg)
 {
 	/* btf_check_iter_kfuncs() guarantees that first argument of any iter
@@ -7443,7 +7424,7 @@ static bool is_kfunc_arg_iter(struct bpf_kfunc_call_arg_meta *meta, int arg_idx,
 }
 
 static int process_iter_arg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno, int insn_idx,
-			    struct bpf_kfunc_call_arg_meta *meta)
+			    struct bpf_call_arg_meta *meta)
 {
 	struct bpf_func_state *state = bpf_func(env, reg);
 	const struct btf_type *t;
@@ -7610,7 +7591,7 @@ static int widen_imprecise_scalars(struct bpf_verifier_env *env,
 }
 
 static struct bpf_reg_state *get_iter_from_state(struct bpf_verifier_state *cur_st,
-						 struct bpf_kfunc_call_arg_meta *meta)
+						 struct bpf_call_arg_meta *meta)
 {
 	int iter_frameno = meta->iter.frameno;
 	int iter_spi = meta->iter.spi;
@@ -7697,7 +7678,7 @@ static struct bpf_reg_state *get_iter_from_state(struct bpf_verifier_state *cur_
  *     bpf_iter_num_destroy(&it);
  */
 static int process_iter_next_call(struct bpf_verifier_env *env, int insn_idx,
-				  struct bpf_kfunc_call_arg_meta *meta)
+				  struct bpf_call_arg_meta *meta)
 {
 	struct bpf_verifier_state *cur_st = env->cur_state, *queued_st, *prev_st;
 	struct bpf_func_state *cur_fr = cur_st->frame[cur_st->curframe], *queued_fr;
@@ -10748,27 +10729,27 @@ static void mark_btf_func_reg_size(struct bpf_verifier_env *env, u32 regno,
 	return __mark_btf_func_reg_size(env, cur_regs(env), regno, reg_size);
 }
 
-static bool is_kfunc_acquire(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_acquire(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_ACQUIRE;
 }
 
-static bool is_kfunc_release(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_release(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_RELEASE;
 }
 
-static bool is_kfunc_destructive(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_destructive(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_DESTRUCTIVE;
 }
 
-static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_rcu(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_RCU;
 }
 
-static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_rcu_protected(struct bpf_call_arg_meta *meta)
 {
 	return meta->kfunc_flags & KF_RCU_PROTECTED;
 }
@@ -10989,7 +10970,7 @@ static bool is_kfunc_arg_prog_aux(const struct btf *btf, const struct btf_param
  * To determine whether an argument is implicit, we compare its position
  * against the number of arguments in the prototype w/o implicit args.
  */
-static bool is_kfunc_arg_implicit(const struct bpf_kfunc_call_arg_meta *meta, u32 arg_idx)
+static bool is_kfunc_arg_implicit(const struct bpf_call_arg_meta *meta, u32 arg_idx)
 {
 	const struct btf_type *func, *func_proto;
 	u32 argn;
@@ -11288,7 +11269,7 @@ static bool is_task_work_add_kfunc(u32 func_id)
 	       func_id == special_kfunc_list[KF_bpf_task_work_schedule_resume];
 }
 
-static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_ret_null(struct bpf_call_arg_meta *meta)
 {
 	if (is_bpf_refcount_acquire_kfunc(meta->func_id) && meta->arg_owning_ref)
 		return false;
@@ -11296,34 +11277,34 @@ static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
 	return meta->kfunc_flags & KF_RET_NULL;
 }
 
-static bool is_kfunc_bpf_rcu_read_lock(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_bpf_rcu_read_lock(struct bpf_call_arg_meta *meta)
 {
 	return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_lock];
 }
 
-static bool is_kfunc_bpf_rcu_read_unlock(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_bpf_rcu_read_unlock(struct bpf_call_arg_meta *meta)
 {
 	return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_unlock];
 }
 
-static bool is_kfunc_bpf_preempt_disable(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_bpf_preempt_disable(struct bpf_call_arg_meta *meta)
 {
 	return meta->func_id == special_kfunc_list[KF_bpf_preempt_disable];
 }
 
-static bool is_kfunc_bpf_preempt_enable(struct bpf_kfunc_call_arg_meta *meta)
+static bool is_kfunc_bpf_preempt_enable(struct bpf_call_arg_meta *meta)
 {
 	return meta->func_id == special_kfunc_list[KF_bpf_preempt_enable];
 }
 
-bool bpf_is_kfunc_pkt_changing(struct bpf_kfunc_call_arg_meta *meta)
+bool bpf_is_kfunc_pkt_changing(struct bpf_call_arg_meta *meta)
 {
 	return meta->func_id == special_kfunc_list[KF_bpf_xdp_pull_data];
 }
 
 static enum kfunc_ptr_arg_type
 get_kfunc_ptr_arg_type(struct bpf_verifier_env *env, struct bpf_func_state *caller,
-		       struct bpf_reg_state *regs, struct bpf_kfunc_call_arg_meta *meta,
+		       struct bpf_reg_state *regs, struct bpf_call_arg_meta *meta,
 		       const struct btf_type *t, const struct btf_type *ref_t,
 		       const char *ref_tname, const struct btf_param *args,
 		       int arg, int nargs, argno_t argno, struct bpf_reg_state *reg)
@@ -11429,7 +11410,7 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
 					struct bpf_reg_state *reg,
 					const struct btf_type *ref_t,
 					const char *ref_tname, u32 ref_id,
-					struct bpf_kfunc_call_arg_meta *meta,
+					struct bpf_call_arg_meta *meta,
 					int arg, argno_t argno)
 {
 	const struct btf_type *reg_ref_t;
@@ -11499,7 +11480,7 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
 }
 
 static int process_irq_flag(struct bpf_verifier_env *env, struct bpf_reg_state *reg, argno_t argno,
-			     struct bpf_kfunc_call_arg_meta *meta)
+			     struct bpf_call_arg_meta *meta)
 {
 	int err, spi, kfunc_class = IRQ_NATIVE_KFUNC;
 	bool irq_save;
@@ -11824,7 +11805,7 @@ static bool check_kfunc_is_graph_node_api(struct bpf_verifier_env *env,
 static int
 __process_kf_arg_ptr_to_graph_root(struct bpf_verifier_env *env,
 				   struct bpf_reg_state *reg, argno_t argno,
-				   struct bpf_kfunc_call_arg_meta *meta,
+				   struct bpf_call_arg_meta *meta,
 				   enum btf_field_type head_field_type,
 				   struct btf_field **head_field)
 {
@@ -11874,7 +11855,7 @@ __process_kf_arg_ptr_to_graph_root(struct bpf_verifier_env *env,
 
 static int process_kf_arg_ptr_to_list_head(struct bpf_verifier_env *env,
 					   struct bpf_reg_state *reg, argno_t argno,
-					   struct bpf_kfunc_call_arg_meta *meta)
+					   struct bpf_call_arg_meta *meta)
 {
 	return __process_kf_arg_ptr_to_graph_root(env, reg, argno, meta, BPF_LIST_HEAD,
 							  &meta->arg_list_head.field);
@@ -11882,7 +11863,7 @@ static int process_kf_arg_ptr_to_list_head(struct bpf_verifier_env *env,
 
 static int process_kf_arg_ptr_to_rbtree_root(struct bpf_verifier_env *env,
 					     struct bpf_reg_state *reg, argno_t argno,
-					     struct bpf_kfunc_call_arg_meta *meta)
+					     struct bpf_call_arg_meta *meta)
 {
 	return __process_kf_arg_ptr_to_graph_root(env, reg, argno, meta, BPF_RB_ROOT,
 							  &meta->arg_rbtree_root.field);
@@ -11891,7 +11872,7 @@ static int process_kf_arg_ptr_to_rbtree_root(struct bpf_verifier_env *env,
 static int
 __process_kf_arg_ptr_to_graph_node(struct bpf_verifier_env *env,
 				   struct bpf_reg_state *reg, argno_t argno,
-				   struct bpf_kfunc_call_arg_meta *meta,
+				   struct bpf_call_arg_meta *meta,
 				   enum btf_field_type head_field_type,
 				   enum btf_field_type node_field_type,
 				   struct btf_field **node_field)
@@ -11956,7 +11937,7 @@ __process_kf_arg_ptr_to_graph_node(struct bpf_verifier_env *env,
 
 static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env,
 					   struct bpf_reg_state *reg, argno_t argno,
-					   struct bpf_kfunc_call_arg_meta *meta)
+					   struct bpf_call_arg_meta *meta)
 {
 	return __process_kf_arg_ptr_to_graph_node(env, reg, argno, meta,
 						  BPF_LIST_HEAD, BPF_LIST_NODE,
@@ -11965,7 +11946,7 @@ static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env,
 
 static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
 					     struct bpf_reg_state *reg, argno_t argno,
-					     struct bpf_kfunc_call_arg_meta *meta)
+					     struct bpf_call_arg_meta *meta)
 {
 	return __process_kf_arg_ptr_to_graph_node(env, reg, argno, meta,
 						  BPF_RB_ROOT, BPF_RB_NODE,
@@ -11994,7 +11975,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
 	}
 }
 
-static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta,
+static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 			    int insn_idx)
 {
 	const char *func_name = meta->func_name, *ref_tname;
@@ -12573,7 +12554,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 int bpf_fetch_kfunc_arg_meta(struct bpf_verifier_env *env,
 			     s32 func_id,
 			     s16 offset,
-			     struct bpf_kfunc_call_arg_meta *meta)
+			     struct bpf_call_arg_meta *meta)
 {
 	struct bpf_kfunc_meta kfunc;
 	int err;
@@ -12732,7 +12713,7 @@ s64 bpf_kfunc_stack_access_bytes(struct bpf_verifier_env *env, struct bpf_insn *
 				 int arg, int insn_idx)
 {
 	struct bpf_insn_aux_data *aux = &env->insn_aux_data[insn_idx];
-	struct bpf_kfunc_call_arg_meta meta;
+	struct bpf_call_arg_meta meta;
 	const struct btf_param *args;
 	const struct btf_type *t, *ref_t;
 	const struct btf *btf;
@@ -12793,7 +12774,7 @@ s64 bpf_kfunc_stack_access_bytes(struct bpf_verifier_env *env, struct bpf_insn *
  *  0  - fall-through to 'else' branch
  * < 0 - not fall-through to 'else' branch, return error
  */
-static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_call_arg_meta *meta,
+static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
 			       struct bpf_reg_state *regs, struct bpf_insn_aux_data *insn_aux,
 			       const struct btf_type *ptr_type, struct btf *desc_btf)
 {
@@ -12972,7 +12953,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	struct bpf_reg_state *regs = cur_regs(env);
 	const char *func_name, *ptr_type_name;
 	const struct btf_type *t, *ptr_type;
-	struct bpf_kfunc_call_arg_meta meta;
+	struct bpf_call_arg_meta meta;
 	struct bpf_insn_aux_data *insn_aux;
 	int err, insn_idx = *insn_idx_p;
 	const struct btf_param *args;
@@ -16726,7 +16707,7 @@ bool bpf_verifier_inlines_helper_call(struct bpf_verifier_env *env, s32 imm)
 bool bpf_get_call_summary(struct bpf_verifier_env *env, struct bpf_insn *call,
 			  struct bpf_call_summary *cs)
 {
-	struct bpf_kfunc_call_arg_meta meta;
+	struct bpf_call_arg_meta meta;
 	const struct bpf_func_proto *fn;
 	int i;
 
-- 
2.52.0

Re: [PATCH bpf-next v1 4/4] bpf: Unify helper and kfunc call argument meta

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Uninitialized stack memory read in `check_kfunc_call` when handling kfunc calls in signed BPF programs.
--

--- Patch [4]: [PATCH bpf-next v1 4/4] bpf: Unify helper and kfunc call argument meta ---
commit b1105689a4b2bf6e2532e6d414d410ef55c0694b
Author: Amery Hung <ameryhung@gmail.com>

bpf: Unify helper and kfunc call argument meta
    
Helper and kfunc argument checking carried two separate meta structs: the
verifier-local struct bpf_call_arg_meta and bpf_kfunc_call_arg_meta.
Merge them into a single struct bpf_call_arg_meta. This is groundwork for
sharing argument checking between helpers and kfuncs.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index fcd9715eff017..5ccb00a8c84ce 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12972,7 +12953,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	struct bpf_reg_state *regs = cur_regs(env);
>  	const char *func_name, *ptr_type_name;
>  	const struct btf_type *t, *ptr_type;
> -	struct bpf_kfunc_call_arg_meta meta;
> +	struct bpf_call_arg_meta meta;
>  	struct bpf_insn_aux_data *insn_aux;
>  	int err, insn_idx = *insn_idx_p;
>  	const struct btf_param *args;

[Severity: High]
This is a pre-existing issue, but does this lead to reading an uninitialized
pointer if bpf_fetch_kfunc_arg_meta() returns early?

In check_kfunc_call() in kernel/bpf/verifier.c, the local variable meta is
declared without initialization and passed to bpf_fetch_kfunc_arg_meta().
If the call to fetch_kfunc_meta() inside it returns an error like -EACCES,
it returns early before initializing meta:

kernel/bpf/verifier.c:bpf_fetch_kfunc_arg_meta() {
	struct bpf_kfunc_meta kfunc;
	int err;

	err = fetch_kfunc_meta(env, func_id, offset, &kfunc);
	if (err)
		return err;
    ...
}

Back in check_kfunc_call(), the error path evaluates meta.func_name:

	err = bpf_fetch_kfunc_arg_meta(env, insn->imm, insn->off, &meta);
	if (err == -EACCES && meta.func_name)
		verbose(env, "calling kernel function %s is not allowed\n", meta.func_name);

If the stack memory is uninitialized, could meta.func_name contain a garbage
pointer that leads to a crash or an info leak when dereferenced by vsnprintf
during the verbose() call?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709230242.2003459-1-ameryhung@gmail.com?part=4

Re: [PATCH bpf-next v1 2/4] bpf: Factor out raw_mode-related fields in bpf_call_arg_meta

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch drops the detection of `MEM_UNINIT` raw memory arguments for `ARG_PTR_TO_MAP_VALUE`, which incorrectly breaks valid BPF programs that pass uninitialized stack buffers to helpers like `bpf_map_pop_elem` and `bpf_map_peek_elem`.
--

--- Patch [2]: [PATCH bpf-next v1 2/4] bpf: Factor out raw_mode-related fields in bpf_call_arg_meta ---
commit bbac2673fbeb11bb9179553dc669702134b2c290
Author: Amery Hung <ameryhung@gmail.com>

    bpf: Factor out raw_mode-related fields in bpf_call_arg_meta
    
    To prepare for unifying the helper and kfunc call_arg_meta, group the
    scattered MEM_UNINIT ("raw") memory argument fields (raw_mode, regno and
    access_size) into a new struct arg_raw_mem_desc.

>     Also drop the bogus raw_mode assignment in the ARG_PTR_TO_MAP_VALUE case:
>     arg_type_is_raw_mem() only matches ARG_PTR_TO_MEM | MEM_UNINIT, and a map
>     value pointer is handled by check_map_access(), never reaching
>     check_stack_range_initialized(), so tagging it raw mode had no effect.

[Severity: High]
Is this assumption accurate? If a BPF program calls a helper like
bpf_map_pop_elem() and passes a stack buffer (PTR_TO_STACK) as the map
value buffer, wouldn't check_helper_mem_access() call
check_stack_range_initialized() to validate the stack access?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7dd961ede88df..e2a3ed67d2eb5 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6690,6 +6688,8 @@ static int check_stack_range_initialized(
>  	 * but BTF based global subprog validation isn't accurate enough.
>  	 */
>  	bool allow_poison = access_size < 0 || clobber;
> +	/* The call will initialize the memory; uninitialized stack allowed */
> +	bool raw_mode = meta && meta->arg_raw_mem.regno == reg_from_argno(argno);
>  
>  	access_size = abs(access_size);

[Severity: High]
Because arg_type_is_raw_mem() only checks ARG_PTR_TO_MEM, arg_raw_mem.regno
will not be set for ARG_PTR_TO_MAP_VALUE arguments.

Consequently, raw_mode will evaluate to false here. Does this cause the
verifier to fail to bypass the uninitialized read checks, rejecting valid
unprivileged programs with an 'invalid read from stack' error?

[ ... ]

> @@ -8403,7 +8401,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
>  			verifier_bug(env, "invalid map_ptr to access map->value");
>  			return -EFAULT;
>  		}
> -		meta->raw_mode = arg_type & MEM_UNINIT;
>  		err = check_helper_mem_access(env, reg, argno_from_reg(regno), meta->map.ptr->value_size,
>  					      arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
>  					      false, meta);

[Severity: High]
Without this MEM_UNINIT assignment for ARG_PTR_TO_MAP_VALUE, does the
verifier incorrectly reject valid programs passing uninitialized stack
memory?

[ ... ]

> @@ -10303,8 +10292,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  	/* Mark slots with STACK_MISC in case of raw mode, stack offset
>  	 * is inferred from register state.
>  	 */
> -	for (i = 0; i < meta.access_size; i++) {
> -		err = check_mem_access(env, insn_idx, regs + meta.regno, argno_from_reg(meta.regno), i, BPF_B,
> +	for (i = 0; i < meta.arg_raw_mem.size; i++) {
> +		err = check_mem_access(env, insn_idx, regs + meta.arg_raw_mem.regno,
> +				       argno_from_reg(meta.arg_raw_mem.regno), i, BPF_B,
>  				       BPF_WRITE, -1, false, false);

[Severity: High]
Since meta.arg_raw_mem.size remains 0 for ARG_PTR_TO_MAP_VALUE, does this
cause check_helper_call() to skip this post-processing loop entirely? 

Skipping this could miss crucial state scrubbing when a privileged program
uses special slots like STACK_DYNPTR as a map value buffer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709230242.2003459-1-ameryhung@gmail.com?part=2

Re: [PATCH bpf-next v1 2/4] bpf: Factor out raw_mode-related fields in bpf_call_arg_meta

[Amery Hung]

On Thu, Jul 9, 2026 at 4:17 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The patch drops the detection of `MEM_UNINIT` raw memory arguments for `ARG_PTR_TO_MAP_VALUE`, which incorrectly breaks valid BPF programs that pass uninitialized stack buffers to helpers like `bpf_map_pop_elem` and `bpf_map_peek_elem`.
> --
>
> --- Patch [2]: [PATCH bpf-next v1 2/4] bpf: Factor out raw_mode-related fields in bpf_call_arg_meta ---
> commit bbac2673fbeb11bb9179553dc669702134b2c290
> Author: Amery Hung <ameryhung@gmail.com>
>
>     bpf: Factor out raw_mode-related fields in bpf_call_arg_meta
>
>     To prepare for unifying the helper and kfunc call_arg_meta, group the
>     scattered MEM_UNINIT ("raw") memory argument fields (raw_mode, regno and
>     access_size) into a new struct arg_raw_mem_desc.
>
> >     Also drop the bogus raw_mode assignment in the ARG_PTR_TO_MAP_VALUE case:
> >     arg_type_is_raw_mem() only matches ARG_PTR_TO_MEM | MEM_UNINIT, and a map
> >     value pointer is handled by check_map_access(), never reaching
> >     check_stack_range_initialized(), so tagging it raw mode had no effect.
>
> [Severity: High]
> Is this assumption accurate? If a BPF program calls a helper like
> bpf_map_pop_elem() and passes a stack buffer (PTR_TO_STACK) as the map
> value buffer, wouldn't check_helper_mem_access() call
> check_stack_range_initialized() to validate the stack access?

I was wrong. This patch will cause a regression. raw_mode should not
be dropped for ARG_PTR_TO_MAP_VALUE.

>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 7dd961ede88df..e2a3ed67d2eb5 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -6690,6 +6688,8 @@ static int check_stack_range_initialized(
> >        * but BTF based global subprog validation isn't accurate enough.
> >        */
> >       bool allow_poison = access_size < 0 || clobber;
> > +     /* The call will initialize the memory; uninitialized stack allowed */
> > +     bool raw_mode = meta && meta->arg_raw_mem.regno == reg_from_argno(argno);
> >
> >       access_size = abs(access_size);
>
> [Severity: High]
> Because arg_type_is_raw_mem() only checks ARG_PTR_TO_MEM, arg_raw_mem.regno
> will not be set for ARG_PTR_TO_MAP_VALUE arguments.
>
> Consequently, raw_mode will evaluate to false here. Does this cause the
> verifier to fail to bypass the uninitialized read checks, rejecting valid
> unprivileged programs with an 'invalid read from stack' error?
>
> [ ... ]
>
> > @@ -8403,7 +8401,6 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
> >                       verifier_bug(env, "invalid map_ptr to access map->value");
> >                       return -EFAULT;
> >               }
> > -             meta->raw_mode = arg_type & MEM_UNINIT;
> >               err = check_helper_mem_access(env, reg, argno_from_reg(regno), meta->map.ptr->value_size,
> >                                             arg_type & MEM_WRITE ? BPF_WRITE : BPF_READ,
> >                                             false, meta);
>
> [Severity: High]
> Without this MEM_UNINIT assignment for ARG_PTR_TO_MAP_VALUE, does the
> verifier incorrectly reject valid programs passing uninitialized stack
> memory?
>
> [ ... ]
>
> > @@ -10303,8 +10292,9 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
> >       /* Mark slots with STACK_MISC in case of raw mode, stack offset
> >        * is inferred from register state.
> >        */
> > -     for (i = 0; i < meta.access_size; i++) {
> > -             err = check_mem_access(env, insn_idx, regs + meta.regno, argno_from_reg(meta.regno), i, BPF_B,
> > +     for (i = 0; i < meta.arg_raw_mem.size; i++) {
> > +             err = check_mem_access(env, insn_idx, regs + meta.arg_raw_mem.regno,
> > +                                    argno_from_reg(meta.arg_raw_mem.regno), i, BPF_B,
> >                                      BPF_WRITE, -1, false, false);
>
> [Severity: High]
> Since meta.arg_raw_mem.size remains 0 for ARG_PTR_TO_MAP_VALUE, does this
> cause check_helper_call() to skip this post-processing loop entirely?
>
> Skipping this could miss crucial state scrubbing when a privileged program
> uses special slots like STACK_DYNPTR as a map value buffer.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709230242.2003459-1-ameryhung@gmail.com?part=2

Re: [PATCH bpf-next v1 4/4] bpf: Unify helper and kfunc call argument meta

[Amery Hung]

On Thu, Jul 9, 2026 at 4:16 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Uninitialized stack memory read in `check_kfunc_call` when handling kfunc calls in signed BPF programs.
> --
>
> --- Patch [4]: [PATCH bpf-next v1 4/4] bpf: Unify helper and kfunc call argument meta ---
> commit b1105689a4b2bf6e2532e6d414d410ef55c0694b
> Author: Amery Hung <ameryhung@gmail.com>
>
> bpf: Unify helper and kfunc call argument meta
>
> Helper and kfunc argument checking carried two separate meta structs: the
> verifier-local struct bpf_call_arg_meta and bpf_kfunc_call_arg_meta.
> Merge them into a single struct bpf_call_arg_meta. This is groundwork for
> sharing argument checking between helpers and kfuncs.
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index fcd9715eff017..5ccb00a8c84ce 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> > @@ -12972,7 +12953,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> >       struct bpf_reg_state *regs = cur_regs(env);
> >       const char *func_name, *ptr_type_name;
> >       const struct btf_type *t, *ptr_type;
> > -     struct bpf_kfunc_call_arg_meta meta;
> > +     struct bpf_call_arg_meta meta;
> >       struct bpf_insn_aux_data *insn_aux;
> >       int err, insn_idx = *insn_idx_p;
> >       const struct btf_param *args;
>
> [Severity: High]
> This is a pre-existing issue, but does this lead to reading an uninitialized
> pointer if bpf_fetch_kfunc_arg_meta() returns early?
>
> In check_kfunc_call() in kernel/bpf/verifier.c, the local variable meta is
> declared without initialization and passed to bpf_fetch_kfunc_arg_meta().
> If the call to fetch_kfunc_meta() inside it returns an error like -EACCES,
> it returns early before initializing meta:
>
> kernel/bpf/verifier.c:bpf_fetch_kfunc_arg_meta() {
>         struct bpf_kfunc_meta kfunc;
>         int err;
>
>         err = fetch_kfunc_meta(env, func_id, offset, &kfunc);
>         if (err)
>                 return err;
>     ...
> }
>
> Back in check_kfunc_call(), the error path evaluates meta.func_name:
>
>         err = bpf_fetch_kfunc_arg_meta(env, insn->imm, insn->off, &meta);
>         if (err == -EACCES && meta.func_name)
>                 verbose(env, "calling kernel function %s is not allowed\n", meta.func_name);
>
> If the stack memory is uninitialized, could meta.func_name contain a garbage
> pointer that leads to a crash or an info leak when dereferenced by vsnprintf
> during the verbose() call?

Confirmed the bug is reachable. I will include the fix in the next respin.

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260709230242.2003459-1-ameryhung@gmail.com?part=4

Re: [PATCH bpf-next v1 3/4] bpf: Unify helper and kfunc allocation-size argument handling

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2574 bytes --]

>     bpf: Unify helper and kfunc allocation-size argument handling
>
>     The constant "size of the PTR_TO_MEM returned in R0" argument is handled by
>     both helpers (ARG_CONST_ALLOC_SIZE_OR_ZERO) and kfuncs (__rdonly_buf_size /
>     __rdwr_buf_size), each with its own meta field (meta->mem_size,
>     meta->r0_size) and duplicated validation. Add struct arg_alloc_mem_desc
>     and a shared process_const_alloc_mem_size(), and replace both fields with
>     meta->arg_alloc_mem.
>
>     The desc records presence with a 'found' flag instead of using a non-zero
>     size as the sentinel. This also fixes a pre-existing bug on the kfunc
>     return path: "no size argument" was tested as r0_size == 0, so an explicit
>     __rdonly_buf_size/__rdwr_buf_size of 0 was treated as absent and fell
>     through to btf_resolve_size(), giving R0 the size of the pointed-to return
>     type instead of 0. With 'found', an explicit zero size is honored and
>     btf_resolve_size() is used only when no size argument was passed.
>
>     The size is stored in a u32, matching regs[R0].mem_size. The U32_MAX
>     check now apply to both helper and kfunc through
>     process_const_alloc_mem_size().
>
>     Fold bpf_session_cookie return size assignment into current kfunc return
>     size resolution path.
>
>     Note that verifier saves kfunc return size through r0_size instead of
>     mem_size. The later has no active readers so remove it.
>
>     Signed-off-by: Amery Hung <ameryhung@gmail.com>

The changelog describes fixing a pre-existing bug on the kfunc return path,
where an explicit __rdonly_buf_size/__rdwr_buf_size of 0 was treated as
absent and fell through to btf_resolve_size().

That sentinel-based fallback looks like it dates back to when fixed-size
memory pointer returns were added for kfuncs, where the
"if (!meta.r0_size) { ... btf_resolve_size(...) }" block first appeared
alongside the rdonly_buf_size/rdwr_buf_size handling that set meta->r0_size.

Should this carry a Fixes: tag so the fix can be backported?

  Fixes: f4b4eee6169b ("bpf: add support for fixed-size memory pointer returns for kfuncs")

This isn't a bug, but would it be worth tidying the changelog wording?
"The later has no active readers" reads as "The latter", and "The U32_MAX
check now apply to both" reads as "now applies".


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/29057017398