[PATCH bpf-next v2 03/23] bpf: Only compare func_id against BPF_FUNC_* for helper calls

[Amery Hung <ameryhung@gmail.com>]

Helpers are identified by enum bpf_func_id, while kfuncs are identified
by a BTF ID. Both values are stored in bpf_call_arg_meta::func_id, and
a kfunc BTF ID can have the same numeric value as a BPF_FUNC_* constant.

Later patches extend check_reg_type() and check_func_arg() to kfuncs.
A bare func_id comparison in those common paths could then mistake a
kfunc for a helper.

Introduce is_helper_call(), which first excludes kfunc metadata through
meta->btf, and use it for every BPF_FUNC_* comparison. This keeps
helper-specific behavior out of the shared path from the start.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/verifier.c | 32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d5e885435bb6..5bf31fbdaac1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8176,6 +8176,16 @@ static bool arg_type_is_dynptr(enum bpf_arg_type type)
 	return base_type(type) == ARG_PTR_TO_DYNPTR;
 }
 
+/*
+ * A kfunc is named by a BTF ID, which can take the same numeric value as an
+ * enum bpf_func_id. Only test meta->func_id against a BPF_FUNC_* once the call
+ * is known to be to a helper; meta->btf is set only for a kfunc.
+ */
+static bool is_helper_call(const struct bpf_call_arg_meta *meta, enum bpf_func_id func_id)
+{
+	return !meta->btf && meta->func_id == func_id;
+}
+
 static int resolve_map_arg_type(struct bpf_verifier_env *env,
 				 const struct bpf_call_arg_meta *meta,
 				 enum bpf_arg_type *arg_type)
@@ -8197,7 +8207,7 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env,
 		}
 		break;
 	case BPF_MAP_TYPE_BLOOM_FILTER:
-		if (meta->func_id == BPF_FUNC_map_peek_elem)
+		if (is_helper_call(meta, BPF_FUNC_map_peek_elem))
 			*arg_type = ARG_PTR_TO_MAP_VALUE;
 		break;
 	default:
@@ -8413,7 +8423,8 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
 		type &= ~DYNPTR_TYPE_FLAG_MASK;
 
 	/* Local kptr types are allowed as the source argument of bpf_kptr_xchg */
-	if (meta->func_id == BPF_FUNC_kptr_xchg && type_is_alloc(type) && reg_from_argno(argno) == BPF_REG_2) {
+	if (is_helper_call(meta, BPF_FUNC_kptr_xchg) && type_is_alloc(type) &&
+	    reg_from_argno(argno) == BPF_REG_2) {
 		type &= ~MEM_ALLOC;
 		type &= ~MEM_PERCPU;
 	}
@@ -8467,7 +8478,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
 		 * allows bpf_sk_release to work for multiple socket types.
 		 */
 		bool strict_type_match = arg_type_is_release(arg_type) &&
-					 meta->func_id != BPF_FUNC_sk_release;
+					 !is_helper_call(meta, BPF_FUNC_sk_release);
 
 		if (type_may_be_null(reg->type) &&
 		    (!type_may_be_null(arg_type) || arg_type_is_release(arg_type))) {
@@ -8488,7 +8499,7 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
 			arg_btf_id = compatible->btf_id;
 		}
 
-		if (meta->func_id == BPF_FUNC_kptr_xchg) {
+		if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) {
 			if (map_kptr_match_type(env, meta->kptr_field, reg, reg_from_argno(argno)))
 				return -EACCES;
 		} else {
@@ -8519,13 +8530,14 @@ static int check_reg_type(struct bpf_verifier_env *env, struct bpf_reg_state *re
 	case PTR_TO_BTF_ID | MEM_PERCPU | MEM_ALLOC:
 	case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF:
 	case PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU:
-		if (meta->func_id != BPF_FUNC_spin_lock && meta->func_id != BPF_FUNC_spin_unlock &&
-		    meta->func_id != BPF_FUNC_kptr_xchg) {
+		if (!is_helper_call(meta, BPF_FUNC_spin_lock) &&
+		    !is_helper_call(meta, BPF_FUNC_spin_unlock) &&
+		    !is_helper_call(meta, BPF_FUNC_kptr_xchg)) {
 			verifier_bug(env, "unimplemented handling of MEM_ALLOC");
 			return -EFAULT;
 		}
 		/* Check if local kptr in src arg matches kptr in dst arg */
-		if (meta->func_id == BPF_FUNC_kptr_xchg) {
+		if (is_helper_call(meta, BPF_FUNC_kptr_xchg)) {
 			int regno = reg_from_argno(argno);
 
 			if (regno == BPF_REG_2 &&
@@ -8948,7 +8960,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 		 * Disable raw mode for bpf_map_peek_elem() on a bloom filter. The helper reads
 		 * the value buffer as an input rather than filling it.
 		 */
-		if (meta->func_id == BPF_FUNC_map_peek_elem &&
+		if (is_helper_call(meta, BPF_FUNC_map_peek_elem) &&
 		    meta->map.ptr->map_type == BPF_MAP_TYPE_BLOOM_FILTER)
 			meta->arg_raw_mem.regno = 0;
 
@@ -8969,11 +8981,11 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
 			verbose(env, "can't spin_{lock,unlock} in rbtree cb\n");
 			return -EACCES;
 		}
-		if (meta->func_id == BPF_FUNC_spin_lock) {
+		if (is_helper_call(meta, BPF_FUNC_spin_lock)) {
 			err = process_spin_lock(env, reg, argno, PROCESS_SPIN_LOCK);
 			if (err)
 				return err;
-		} else if (meta->func_id == BPF_FUNC_spin_unlock) {
+		} else if (is_helper_call(meta, BPF_FUNC_spin_unlock)) {
 			err = process_spin_lock(env, reg, argno, 0);
 			if (err)
 				return err;
-- 
2.52.0