bpf: reuse subprog argument parsing logic for subprog call checks

Remove duplicated BTF parsing logic when it comes to subprog call check.
Instead, use (potentially cached) results of btf_prepare_func_args() to
abstract away expectations of each subprog argument in generic terms
(e.g., "this is pointer to context", or "this is a pointer to memory of
size X"), and then use those simple high-level argument type
expectations to validate actual register states to check if they match
expectations.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231215011334.2307144-6-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f48e49f..3c9e6d0 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9249,101 +9249,54 @@ err_out:
        return err;
 }
 
-static int btf_check_func_arg_match(struct bpf_verifier_env *env,
-                                   const struct btf *btf, u32 func_id,
-                                   struct bpf_reg_state *regs,
-                                   bool ptr_to_mem_ok)
+static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
+                                   const struct btf *btf,
+                                   struct bpf_reg_state *regs)
 {
-       enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+       struct bpf_subprog_info *sub = subprog_info(env, subprog);
        struct bpf_verifier_log *log = &env->log;
-       const char *func_name, *ref_tname;
-       const struct btf_type *t, *ref_t;
-       const struct btf_param *args;
-       u32 i, nargs, ref_id;
+       u32 i;
        int ret;
 
-       t = btf_type_by_id(btf, func_id);
-       if (!t || !btf_type_is_func(t)) {
-               /* These checks were already done by the verifier while loading
-                * struct bpf_func_info or in add_kfunc_call().
-                */
-               bpf_log(log, "BTF of func_id %u doesn't point to KIND_FUNC\n",
-                       func_id);
-               return -EFAULT;
-       }
-       func_name = btf_name_by_offset(btf, t->name_off);
-
-       t = btf_type_by_id(btf, t->type);
-       if (!t || !btf_type_is_func_proto(t)) {
-               bpf_log(log, "Invalid BTF of func %s\n", func_name);
-               return -EFAULT;
-       }
-       args = (const struct btf_param *)(t + 1);
-       nargs = btf_type_vlen(t);
-       if (nargs > MAX_BPF_FUNC_REG_ARGS) {
-               bpf_log(log, "Function %s has %d > %d args\n", func_name, nargs,
-                       MAX_BPF_FUNC_REG_ARGS);
-               return -EINVAL;
-       }
+       ret = btf_prepare_func_args(env, subprog);
+       if (ret)
+               return ret;
 
        /* check that BTF function arguments match actual types that the
         * verifier sees.
         */
-       for (i = 0; i < nargs; i++) {
-               enum bpf_arg_type arg_type = ARG_DONTCARE;
+       for (i = 0; i < sub->arg_cnt; i++) {
                u32 regno = i + 1;
                struct bpf_reg_state *reg = &regs[regno];
+               struct bpf_subprog_arg_info *arg = &sub->args[i];
 
-               t = btf_type_skip_modifiers(btf, args[i].type, NULL);
-               if (btf_type_is_scalar(t)) {
-                       if (reg->type == SCALAR_VALUE)
-                               continue;
-                       bpf_log(log, "R%d is not a scalar\n", regno);
-                       return -EINVAL;
-               }
-
-               if (!btf_type_is_ptr(t)) {
-                       bpf_log(log, "Unrecognized arg#%d type %s\n",
-                               i, btf_type_str(t));
-                       return -EINVAL;
-               }
-
-               ref_t = btf_type_skip_modifiers(btf, t->type, &ref_id);
-               ref_tname = btf_name_by_offset(btf, ref_t->name_off);
-
-               ret = check_func_arg_reg_off(env, reg, regno, arg_type);
-               if (ret < 0)
-                       return ret;
-
-               if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) {
+               if (arg->arg_type == ARG_ANYTHING) {
+                       if (reg->type != SCALAR_VALUE) {
+                               bpf_log(log, "R%d is not a scalar\n", regno);
+                               return -EINVAL;
+                       }
+               } else if (arg->arg_type == ARG_PTR_TO_CTX) {
+                       ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
+                       if (ret < 0)
+                               return ret;
                        /* If function expects ctx type in BTF check that caller
                         * is passing PTR_TO_CTX.
                         */
                        if (reg->type != PTR_TO_CTX) {
-                               bpf_log(log,
-                                       "arg#%d expected pointer to ctx, but got %s\n",
-                                       i, btf_type_str(t));
-                               return -EINVAL;
-                       }
-               } else if (ptr_to_mem_ok) {
-                       const struct btf_type *resolve_ret;
-                       u32 type_size;
-
-                       resolve_ret = btf_resolve_size(btf, ref_t, &type_size);
-                       if (IS_ERR(resolve_ret)) {
-                               bpf_log(log,
-                                       "arg#%d reference type('%s %s') size cannot be determined: %ld\n",
-                                       i, btf_type_str(ref_t), ref_tname,
-                                       PTR_ERR(resolve_ret));
+                               bpf_log(log, "arg#%d expects pointer to ctx\n", i);
                                return -EINVAL;
                        }
+               } else if (base_type(arg->arg_type) == ARG_PTR_TO_MEM) {
+                       ret = check_func_arg_reg_off(env, reg, regno, ARG_DONTCARE);
+                       if (ret < 0)
+                               return ret;
 
-                       if (check_mem_reg(env, reg, regno, type_size))
+                       if (check_mem_reg(env, reg, regno, arg->mem_size))
                                return -EINVAL;
                } else {
-                       bpf_log(log, "reg type unsupported for arg#%d function %s#%d\n", i,
-                               func_name, func_id);
-                       return -EINVAL;
+                       bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
+                               i, arg->arg_type);
+                       return -EFAULT;
                }
        }
 
@@ -9358,11 +9311,10 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
  * Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
  */
 static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
-                                 struct bpf_reg_state *regs)
+                                 struct bpf_reg_state *regs)
 {
        struct bpf_prog *prog = env->prog;
        struct btf *btf = prog->aux->btf;
-       bool is_global;
        u32 btf_id;
        int err;
 
@@ -9376,9 +9328,7 @@ static int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
        if (prog->aux->func_info_aux[subprog].unreliable)
                return -EINVAL;
 
-       is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
-       err = btf_check_func_arg_match(env, btf, btf_id, regs, is_global);
-
+       err = btf_check_func_arg_match(env, subprog, btf, regs);
        /* Compiler optimizations can remove arguments from static functions
         * or mismatched type can be passed into a global function.
         * In such cases mark the function as unreliable from BTF point of view.

diff --git a/tools/testing/selftests/bpf/progs/test_global_func5.c b/tools/testing/selftests/bpf/progs/test_global_func5.c
index cc55aed..257c056 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_global_func5.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func5.c
@@ -26,7 +26,7 @@ int f3(int val, struct __sk_buff *skb)
 }
 
 SEC("tc")
-__failure __msg("expected pointer to ctx, but got PTR")
+__failure __msg("expects pointer to ctx")
 int global_func5(struct __sk_buff *skb)
 {
        return f1(skb) + f2(2, skb) + f3(3, skb);