[PATCH bpf-next v3 08/11] bpf: Check subprog dynptr arguments in the common path

[Amery Hung <ameryhung@gmail.com>]

Subprog and helper/kfunc dynptr arguments ultimately use the same
process_dynptr_func() validation. Route the subprog dynptr branch
through check_func_arg() so register type, offset, and dynptr state
are checked in the common order.

check_func_arg() validates the register against the PTR_TO_STACK and
CONST_PTR_TO_DYNPTR compatibility set before dispatching to
process_dynptr_func(). Once the subprog path uses the common checker,
process_dynptr_func() has no caller that bypasses this validation. Remove
its now-redundant register-type check.

The existing wrong-register-type test passed a NULL local to a callee
that did not use the argument. Clang left the context pointer in R1,
while GCC materialized zero. The common checker rejected the values at
different stages and emitted different diagnostics.

Obtain a PTR_TO_BTF_ID from bpf_get_current_task_btf() and keep its
callee argument live. This makes both compilers exercise the intended
register-type mismatch.

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

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index efb20e50c974..0d554e95fdb5 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -8133,18 +8133,6 @@ static int process_dynptr_func(struct bpf_verifier_env *env, struct bpf_reg_stat
 {
 	int spi, err = 0;
 
-	if (reg->type != PTR_TO_STACK && reg->type != CONST_PTR_TO_DYNPTR) {
-		verbose(env,
-			"%s expected pointer to stack or const struct bpf_dynptr\n",
-			reg_arg_name(env, argno));
-		bpf_diag_call_arg_fmt(
-			env, insn_idx, argno, meta->func_name,
-			"Pass the address of a stack dynptr object, or use a const dynptr pointer returned by the verifier-supported path.",
-			"a dynptr argument must be a pointer to a dynptr stack slot or a verifier-provided const struct bpf_dynptr, but %s is %s",
-			reg_arg_name(env, argno), bpf_diag_reg_type_plain(env, reg->type));
-		return -EINVAL;
-	}
-
 	/*  MEM_UNINIT - Points to memory that is an appropriate candidate for
 	 *		 constructing a mutable bpf_dynptr object.
 	 *
@@ -10883,7 +10871,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 		nslots = btf_arg_slots(t);
 
 		if (arg_type == ARG_SCALAR || arg_type == ARG_IGNORE ||
-		    arg_type == ARG_PTR_TO_CTX ||
+		    arg_type == ARG_PTR_TO_CTX || arg_type == ARG_PTR_TO_DYNPTR ||
 		    base_type(arg_type) == ARG_PTR_TO_ARENA) {
 			ret = check_func_arg(env, arg, slot, 0, &meta, env->insn_idx);
 			if (ret)
@@ -10912,15 +10900,6 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
 					reg_arg_name(env, argno));
 				return -EINVAL;
 			}
-		} else if (arg_type == ARG_PTR_TO_DYNPTR) {
-			ret = check_func_arg_reg_off(env, reg, argno, ARG_PTR_TO_DYNPTR);
-			if (ret)
-				return ret;
-
-			ret = process_dynptr_func(env, reg, argno, env->insn_idx,
-						  arg_type, &meta);
-			if (ret)
-				return ret;
 		} else if (base_type(arg_type) == ARG_PTR_TO_BTF_ID) {
 			int err;
 
diff --git a/tools/testing/selftests/bpf/progs/dynptr_fail.c b/tools/testing/selftests/bpf/progs/dynptr_fail.c
index 9418dfe4d7b7..148cf4417322 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_fail.c
@@ -2053,19 +2053,18 @@ __noinline long global_call_bpf_dynptr(const struct bpf_dynptr *dynptr)
 	/* Avoid leaving this global function empty to avoid having the compiler
 	 * optimize away the call to this global function.
 	 */
+	__sink(dynptr);
 	__sink(ret);
 	return ret;
 }
 
 SEC("?raw_tp")
-__failure __msg("R1 expected pointer to stack or const struct bpf_dynptr")
+__failure __msg("R1 type=trusted_ptr_ expected=fp, dynptr_ptr")
 int test_dynptr_reg_type(void *ctx)
 {
-	struct task_struct *current = NULL;
-	/* R1 should be holding a PTR_TO_BTF_ID, so this shouldn't be a
-	 * reg->type that can be passed to a function accepting a
-	 * ARG_PTR_TO_DYNPTR | MEM_RDONLY. process_dynptr_func() should catch
-	 * this.
+	struct task_struct *current = bpf_get_current_task_btf();
+	/* R1 holds a PTR_TO_BTF_ID, which cannot be passed to a function
+	 * accepting ARG_PTR_TO_DYNPTR | MEM_RDONLY.
 	 */
 	global_call_bpf_dynptr((const struct bpf_dynptr *)current);
 	return 0;
-- 
2.52.0