bpf: Refactor btf_check_func_arg_match

This patch moved the subprog specific logic from
btf_check_func_arg_match() to the new btf_check_subprog_arg_match().
The core logic is left in btf_check_func_arg_match() which
will be reused later to check the kernel function call.

The "if (!btf_type_is_ptr(t))" is checked first to improve the
indentation which will be useful for a later patch.

Some of the "btf_kind_str[]" usages is replaced with the shortcut
"btf_type_str(t)".

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20210325015136.1544504-1-kafai@fb.com

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5a0801b..eaae618 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1545,8 +1545,8 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
                           struct btf_func_model *m);
 
 struct bpf_reg_state;
-int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
-                            struct bpf_reg_state *regs);
+int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
+                               struct bpf_reg_state *regs);
 int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
                          struct bpf_reg_state *reg);
 int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *prog,

diff --git a/include/linux/btf.h b/include/linux/btf.h
index 9c1b527..8a05687 100644 (file)
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -141,6 +141,11 @@ static inline bool btf_type_is_enum(const struct btf_type *t)
        return BTF_INFO_KIND(t->info) == BTF_KIND_ENUM;
 }
 
+static inline bool btf_type_is_scalar(const struct btf_type *t)
+{
+       return btf_type_is_int(t) || btf_type_is_enum(t);
+}
+
 static inline bool btf_type_is_typedef(const struct btf_type *t)
 {
        return BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF;

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 369faed..3c489ad 100644 (file)
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4377,7 +4377,7 @@ static u8 bpf_ctx_convert_map[] = {
 #undef BPF_LINK_TYPE
 
 static const struct btf_member *
-btf_get_prog_ctx_type(struct bpf_verifier_log *log, struct btf *btf,
+btf_get_prog_ctx_type(struct bpf_verifier_log *log, const struct btf *btf,
                      const struct btf_type *t, enum bpf_prog_type prog_type,
                      int arg)
 {
@@ -5362,122 +5362,135 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr
        return btf_check_func_type_match(log, btf1, t1, btf2, t2);
 }
 
-/* Compare BTF of a function with given bpf_reg_state.
- * Returns:
- * EFAULT - there is a verifier bug. Abort verification.
- * EINVAL - there is a type mismatch or BTF is not available.
- * 0 - BTF matches with what bpf_reg_state expects.
- * Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
- */
-int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
-                            struct bpf_reg_state *regs)
+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)
 {
        struct bpf_verifier_log *log = &env->log;
-       struct bpf_prog *prog = env->prog;
-       struct btf *btf = prog->aux->btf;
-       const struct btf_param *args;
+       const char *func_name, *ref_tname;
        const struct btf_type *t, *ref_t;
-       u32 i, nargs, btf_id, type_size;
-       const char *tname;
-       bool is_global;
-
-       if (!prog->aux->func_info)
-               return -EINVAL;
-
-       btf_id = prog->aux->func_info[subprog].type_id;
-       if (!btf_id)
-               return -EFAULT;
-
-       if (prog->aux->func_info_aux[subprog].unreliable)
-               return -EINVAL;
+       const struct btf_param *args;
+       u32 i, nargs;
 
-       t = btf_type_by_id(btf, btf_id);
+       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
                 */
-               bpf_log(log, "BTF of func#%d doesn't point to KIND_FUNC\n",
-                       subprog);
+               bpf_log(log, "BTF of func_id %u doesn't point to KIND_FUNC\n",
+                       func_id);
                return -EFAULT;
        }
-       tname = btf_name_by_offset(btf, t->name_off);
+       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", tname);
+               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", tname, nargs,
+               bpf_log(log, "Function %s has %d > %d args\n", func_name, nargs,
                        MAX_BPF_FUNC_REG_ARGS);
-               goto out;
+               return -EINVAL;
        }
 
-       is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
        /* check that BTF function arguments match actual types that the
         * verifier sees.
         */
        for (i = 0; i < nargs; i++) {
-               struct bpf_reg_state *reg = &regs[i + 1];
+               u32 regno = i + 1;
+               struct bpf_reg_state *reg = &regs[regno];
 
-               t = btf_type_by_id(btf, args[i].type);
-               while (btf_type_is_modifier(t))
-                       t = btf_type_by_id(btf, t->type);
-               if (btf_type_is_int(t) || btf_type_is_enum(t)) {
+               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", i + 1);
-                       goto out;
+                       bpf_log(log, "R%d is not a scalar\n", regno);
+                       return -EINVAL;
                }
-               if (btf_type_is_ptr(t)) {
+
+               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, NULL);
+               ref_tname = btf_name_by_offset(btf, ref_t->name_off);
+               if (btf_get_prog_ctx_type(log, btf, t, env->prog->type, i)) {
                        /* If function expects ctx type in BTF check that caller
                         * is passing PTR_TO_CTX.
                         */
-                       if (btf_get_prog_ctx_type(log, btf, t, prog->type, i)) {
-                               if (reg->type != PTR_TO_CTX) {
-                                       bpf_log(log,
-                                               "arg#%d expected pointer to ctx, but got %s\n",
-                                               i, btf_kind_str[BTF_INFO_KIND(t->info)]);
-                                       goto out;
-                               }
-                               if (check_ctx_reg(env, reg, i + 1))
-                                       goto out;
-                               continue;
+                       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;
                        }
+                       if (check_ctx_reg(env, reg, regno))
+                               return -EINVAL;
+               } else if (ptr_to_mem_ok) {
+                       const struct btf_type *resolve_ret;
+                       u32 type_size;
 
-                       if (!is_global)
-                               goto out;
-
-                       t = btf_type_skip_modifiers(btf, t->type, NULL);
-
-                       ref_t = btf_resolve_size(btf, t, &type_size);
-                       if (IS_ERR(ref_t)) {
+                       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(t), btf_name_by_offset(btf, t->name_off),
-                                       PTR_ERR(ref_t));
-                               goto out;
+                                       "arg#%d reference type('%s %s') size cannot be determined: %ld\n",
+                                       i, btf_type_str(ref_t), ref_tname,
+                                       PTR_ERR(resolve_ret));
+                               return -EINVAL;
                        }
 
-                       if (check_mem_reg(env, reg, i + 1, type_size))
-                               goto out;
-
-                       continue;
+                       if (check_mem_reg(env, reg, regno, type_size))
+                               return -EINVAL;
+               } else {
+                       return -EINVAL;
                }
-               bpf_log(log, "Unrecognized arg#%d type %s\n",
-                       i, btf_kind_str[BTF_INFO_KIND(t->info)]);
-               goto out;
        }
+
        return 0;
-out:
+}
+
+/* Compare BTF of a function with given bpf_reg_state.
+ * Returns:
+ * EFAULT - there is a verifier bug. Abort verification.
+ * EINVAL - there is a type mismatch or BTF is not available.
+ * 0 - BTF matches with what bpf_reg_state expects.
+ * Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
+ */
+int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
+                               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;
+
+       if (!prog->aux->func_info)
+               return -EINVAL;
+
+       btf_id = prog->aux->func_info[subprog].type_id;
+       if (!btf_id)
+               return -EFAULT;
+
+       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);
+
        /* 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.
         */
-       prog->aux->func_info_aux[subprog].unreliable = true;
-       return -EINVAL;
+       if (err)
+               prog->aux->func_info_aux[subprog].unreliable = true;
+       return err;
 }
 
 /* Convert BTF of a function into bpf_reg_state if possible

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b7df3f0..b31e62d 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5365,7 +5365,7 @@ static int __check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn
        func_info_aux = env->prog->aux->func_info_aux;
        if (func_info_aux)
                is_global = func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
-       err = btf_check_func_arg_match(env, subprog, caller->regs);
+       err = btf_check_subprog_arg_match(env, subprog, caller->regs);
        if (err == -EFAULT)
                return err;
        if (is_global) {
@@ -12288,7 +12288,7 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
                /* 1st arg to a function */
                regs[BPF_REG_1].type = PTR_TO_CTX;
                mark_reg_known_zero(env, regs, BPF_REG_1);
-               ret = btf_check_func_arg_match(env, subprog, regs);
+               ret = btf_check_subprog_arg_match(env, subprog, regs);
                if (ret == -EFAULT)
                        /* unlikely verifier bug. abort.
                         * ret == 0 and ret < 0 are sadly acceptable for