Merge branch 'bpf-reject-mem_alloc-btf-accesses-past-bounds'

Yiyang Chen says:

====================
bpf: Reject MEM_ALLOC BTF accesses past bounds

BTF struct walks can relax the top-level struct-size check for trailing
flexible arrays. That relaxation must not let a PTR_TO_BTF_ID | MEM_ALLOC
access escape the bytes allocated by bpf_obj_new() or bpf_percpu_obj_new().

Patch 1 rejects MEM_ALLOC BTF walks whose access range reaches past the
current struct size before applying the flexible-array relaxation. This now
also applies to struct ID matching used by kfunc and kptr type checks.
Patch 2 adds a linked_list negative loader case for this path.

Changes in v3:
- Pass the flexible-array walk policy through btf_struct_ids_match() callers,
  so MEM_ALLOC kfunc/kptr type checks use the same bounds rule.
- Rename the btf_struct_walk() parameter to walk_flex_arrays.
- Rebase onto current bpf-next.

v2:
https://lore.kernel.org/bpf/cover.1782197377.git.chenyy23@mails.tsinghua.edu.cn/

v1:
https://lore.kernel.org/bpf/cover.1782100805.git.chenyy23@mails.tsinghua.edu.cn/
====================

Link: https://patch.msgid.link/cover.1782807039.git.chenyy23@mails.tsinghua.edu.cn
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

include/linux/bpf.h

@@ -3146,7 +3146,7 @@ int btf_struct_access(struct bpf_verifier_log *log,
 bool btf_struct_ids_match(struct bpf_verifier_log *log,
 			  const struct btf *btf, u32 id, int off,
 			  const struct btf *need_btf, u32 need_type_id,
-			  bool strict);
+			  bool strict, bool walk_flex_arrays);
 
 int btf_distill_func_proto(struct bpf_verifier_log *log,
 			   struct btf *btf,

kernel/bpf/btf.c

@@ -7108,7 +7108,7 @@ enum bpf_struct_walk_result {
 static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 			   const struct btf_type *t, int off, int size,
 			   u32 *next_btf_id, enum bpf_type_flag *flag,
-			   const char **field_name)
+			   const char **field_name, bool walk_flex_arrays)
 {
 	u32 i, moff, mtrue_end, msize = 0, total_nelems = 0;
 	const struct btf_type *mtype, *elem_type = NULL;
@@ -7135,11 +7135,14 @@ static int btf_struct_walk(struct bpf_verifier_log *log, const struct btf *btf,
 		*flag |= PTR_UNTRUSTED;
 
 	if (off + size > t->size) {
+		struct btf_array *array_elem;
+
+		if (!walk_flex_arrays)
+			goto error;
+
 		/* If the last element is a variable size array, we may
 		 * need to relax the rule.
 		 */
-		struct btf_array *array_elem;
-
 		if (vlen == 0)
 			goto error;
 
@@ -7404,7 +7407,8 @@ int btf_struct_access(struct bpf_verifier_log *log,
 
 	t = btf_type_by_id(btf, id);
 	do {
-		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_flag, field_name);
+		err = btf_struct_walk(log, btf, t, off, size, &id, &tmp_flag,
+				      field_name, !type_is_alloc(reg->type));
 
 		switch (err) {
 		case WALK_PTR:
@@ -7463,7 +7467,7 @@ bool btf_types_are_same(const struct btf *btf1, u32 id1,
 bool btf_struct_ids_match(struct bpf_verifier_log *log,
 			  const struct btf *btf, u32 id, int off,
 			  const struct btf *need_btf, u32 need_type_id,
-			  bool strict)
+			  bool strict, bool walk_flex_arrays)
 {
 	const struct btf_type *type;
 	enum bpf_type_flag flag = 0;
@@ -7482,7 +7486,8 @@ bool btf_struct_ids_match(struct bpf_verifier_log *log,
 	type = btf_type_by_id(btf, id);
 	if (!type)
 		return false;
-	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL);
+	err = btf_struct_walk(log, btf, type, off, 1, &id, &flag, NULL,
+			      walk_flex_arrays);
 	if (err != WALK_STRUCT)
 		return false;
 

kernel/bpf/verifier.c

@@ -4379,7 +4379,8 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
 	 */
 	if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, reg->var_off.value,
 				  kptr_field->kptr.btf, kptr_field->kptr.btf_id,
-				  kptr_field->type != BPF_KPTR_UNREF))
+				  kptr_field->type != BPF_KPTR_UNREF,
+				  !type_is_alloc(reg->type)))
 		goto bad_type;
 	return 0;
 bad_type:
@@ -7970,7 +7971,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
 
 			if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id,
 						  reg->var_off.value, btf_vmlinux, *arg_btf_id,
-						  strict_type_match)) {
+						  strict_type_match, !type_is_alloc(reg->type))) {
 				verbose(env, "%s is of type %s but %s is expected\n",
 					reg_arg_name(env, argno),
 					btf_type_name(reg->btf, reg->btf_id),
@@ -11436,7 +11437,8 @@ static int process_kf_arg_ptr_to_btf_id(struct bpf_verifier_env *env,
 	reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id, &reg_ref_id);
 	reg_ref_tname = btf_name_by_offset(reg_btf, reg_ref_t->name_off);
 	struct_same = btf_struct_ids_match(&env->log, reg_btf, reg_ref_id, reg->var_off.value,
-					   meta->btf, ref_id, strict_type_match);
+					   meta->btf, ref_id, strict_type_match,
+					   !type_is_alloc(reg->type));
 	/* If kfunc is accepting a projection type (ie. __sk_buff), it cannot
 	 * actually use it -- it must cast to the underlying type. So we allow
 	 * caller to pass in the underlying type.
@@ -11883,7 +11885,8 @@ __process_kf_arg_ptr_to_graph_node(struct bpf_verifier_env *env,
 	et = btf_type_by_id(field->graph_root.btf, field->graph_root.value_btf_id);
 	t = btf_type_by_id(reg->btf, reg->btf_id);
 	if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->graph_root.btf,
-				  field->graph_root.value_btf_id, true)) {
+				  field->graph_root.value_btf_id, true,
+				  !type_is_alloc(reg->type))) {
 		verbose(env, "operation on %s expects arg#1 %s at offset=%d "
 			"in struct %s, but arg is at offset=%d in struct %s\n",
 			btf_field_type_name(head_field_type),

tools/testing/selftests/bpf/prog_tests/linked_list.c

@@ -68,6 +68,7 @@ static struct {
 	{ "obj_type_id_oor", "local type ID argument must be in range [0, U32_MAX]" },
 	{ "obj_new_no_composite", "bpf_obj_new/bpf_percpu_obj_new type ID argument must be of a struct" },
 	{ "obj_new_no_struct", "bpf_obj_new/bpf_percpu_obj_new type ID argument must be of a struct" },
+	{ "obj_new_flex_array", "access beyond struct obj_new_flex" },
 	{ "obj_drop_non_zero_off", "R1 must have zero offset when passed to release func" },
 	{ "new_null_ret", "R0 invalid mem access 'ptr_or_null_'" },
 	{ "obj_new_acq", "Unreleased reference id=" },

tools/testing/selftests/bpf/progs/linked_list_fail.c

@@ -167,6 +167,16 @@ CHECK_OP(push_back);
 #undef CHECK_OP
 #undef INIT
 
+struct obj_new_flex_elem {
+	int lo;
+	int hi;
+};
+
+struct obj_new_flex {
+	int hdr;
+	struct obj_new_flex_elem cells[];
+};
+
 SEC("?kprobe/xyz")
 int map_compat_kprobe(void *ctx)
 {
@@ -230,6 +240,19 @@ int obj_new_no_struct(void *ctx)
 	return 0;
 }
 
+SEC("?tc")
+int obj_new_flex_array(void *ctx)
+{
+	struct obj_new_flex *p;
+
+	p = bpf_obj_new_impl(bpf_core_type_id_local(struct obj_new_flex), NULL);
+	if (!p)
+		return 0;
+	p->cells[0].hi = 42;
+	bpf_obj_drop_impl(p, NULL);
+	return 0;
+}
+
 SEC("?tc")
 int obj_drop_non_zero_off(void *ctx)
 {