bpf: reuse btf_prepare_func_args() check for main program BTF validation

Instead of btf_check_subprog_arg_match(), use btf_prepare_func_args()
logic to validate "trustworthiness" of main BPF program's BTF information,
if it is present.

We ignored results of original BTF check anyway, often times producing
confusing and ominously-sounding "reg type unsupported for arg#0
function" message, which has no apparent effect on program correctness
and verification process.

All the -EFAULT returning sanity checks are already performed in
check_btf_info_early(), so there is zero reason to have this duplication
of logic between btf_check_subprog_call() and btf_check_subprog_arg_match().
Dropping btf_check_subprog_arg_match() simplifies
btf_check_func_arg_match() further removing `bool processing_call` flag.

One subtle bit that was done by btf_check_subprog_arg_match() was
potentially marking main program's BTF as unreliable. We do this
explicitly now with a dedicated simple check, preserving the original
behavior, but now based on well factored btf_prepare_func_args() logic.

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

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c050c82cc9a5f8f14b14296a10db84b281b58540..d0d7eff22b8a9647c5cfc87a16eb119bdc0fe156 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2466,8 +2466,6 @@ int btf_distill_func_proto(struct bpf_verifier_log *log,
                           struct btf_func_model *m);
 
 struct bpf_reg_state;
-int btf_check_subprog_arg_match(struct bpf_verifier_env *env, int subprog,
-                               struct bpf_reg_state *regs);
 int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
                           struct bpf_reg_state *regs);
 int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog);

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index be2104e5f2f5b7f60882961d86777b44ff5a53c4..33d9a1c73f6e1ece67f0ec7717cb3dcab3d6a295 100644 (file)
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6768,8 +6768,7 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr
 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,
-                                   bool processing_call)
+                                   bool ptr_to_mem_ok)
 {
        enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
        struct bpf_verifier_log *log = &env->log;
@@ -6842,7 +6841,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
                                        i, btf_type_str(t));
                                return -EINVAL;
                        }
-               } else if (ptr_to_mem_ok && processing_call) {
+               } else if (ptr_to_mem_ok) {
                        const struct btf_type *resolve_ret;
                        u32 type_size;
 
@@ -6867,55 +6866,12 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
        return 0;
 }
 
-/* Compare BTF of a function declaration 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, false);
-
-       /* 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.
-        */
-       if (err)
-               prog->aux->func_info_aux[subprog].unreliable = true;
-       return err;
-}
-
 /* Compare BTF of a function call 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.
- *
- * NOTE: the code is duplicated from btf_check_subprog_arg_match()
- * because btf_check_func_arg_match() is still doing both. Once that
- * function is split in 2, we can call from here btf_check_subprog_arg_match()
- * first, and then treat the calling part in a new code path.
  */
 int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
                           struct bpf_reg_state *regs)
@@ -6937,7 +6893,7 @@ int btf_check_subprog_call(struct bpf_verifier_env *env, int subprog,
                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, true);
+       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.

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6555785b9f6378dc79ecf04f88d0fb2fda759550..c26e9ab5226c7658059df4002cdec952f3b03991 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19904,6 +19904,7 @@ static void free_states(struct bpf_verifier_env *env)
 static int do_check_common(struct bpf_verifier_env *env, int subprog)
 {
        bool pop_log = !(env->log.level & BPF_LOG_LEVEL2);
+       struct bpf_subprog_info *sub = subprog_info(env, subprog);
        struct bpf_verifier_state *state;
        struct bpf_reg_state *regs;
        int ret, i;
@@ -19930,9 +19931,9 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
        state->first_insn_idx = env->subprog_info[subprog].start;
        state->last_insn_idx = -1;
 
+
        regs = state->frame[state->curframe]->regs;
        if (subprog || env->prog->type == BPF_PROG_TYPE_EXT) {
-               struct bpf_subprog_info *sub = subprog_info(env, subprog);
                const char *sub_name = subprog_name(env, subprog);
                struct bpf_subprog_arg_info *arg;
                struct bpf_reg_state *reg;
@@ -19979,21 +19980,19 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
                        }
                }
        } else {
+               /* if main BPF program has associated BTF info, validate that
+                * it's matching expected signature, and otherwise mark BTF
+                * info for main program as unreliable
+                */
+               if (env->prog->aux->func_info_aux) {
+                       ret = btf_prepare_func_args(env, 0);
+                       if (ret || sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_PTR_TO_CTX)
+                               env->prog->aux->func_info_aux[0].unreliable = true;
+               }
+
                /* 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_subprog_arg_match(env, subprog, regs);
-               if (ret == -EFAULT)
-                       /* unlikely verifier bug. abort.
-                        * ret == 0 and ret < 0 are sadly acceptable for
-                        * main() function due to backward compatibility.
-                        * Like socket filter program may be written as:
-                        * int bpf_prog(struct pt_regs *ctx)
-                        * and never dereference that ctx in the program.
-                        * 'struct pt_regs' is a type mismatch for socket
-                        * filter that should be using 'struct __sk_buff'.
-                        */
-                       goto out;
        }
 
        ret = do_check(env);

diff --git a/tools/testing/selftests/bpf/prog_tests/log_fixup.c b/tools/testing/selftests/bpf/prog_tests/log_fixup.c
index effd78b2a657348b995387106bfa0a3c53def3d1..7a3fa2ff567b1ce1487f7e964cf49d523c6522bd 100644 (file)
--- a/tools/testing/selftests/bpf/prog_tests/log_fixup.c
+++ b/tools/testing/selftests/bpf/prog_tests/log_fixup.c
@@ -169,9 +169,9 @@ void test_log_fixup(void)
        if (test__start_subtest("bad_core_relo_trunc_none"))
                bad_core_relo(0, TRUNC_NONE /* full buf */);
        if (test__start_subtest("bad_core_relo_trunc_partial"))
-               bad_core_relo(300, TRUNC_PARTIAL /* truncate original log a bit */);
+               bad_core_relo(280, TRUNC_PARTIAL /* truncate original log a bit */);
        if (test__start_subtest("bad_core_relo_trunc_full"))
-               bad_core_relo(210, TRUNC_FULL  /* truncate also libbpf's message patch */);
+               bad_core_relo(220, TRUNC_FULL  /* truncate also libbpf's message patch */);
        if (test__start_subtest("bad_core_relo_subprog"))
                bad_core_relo_subprog();
        if (test__start_subtest("missing_map"))

diff --git a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c
index 0fa564a5cc5b3fdcd1ad7e98bd8b33cba42b1d54..9fe9c4a4e8f647b766dda2137a1992d171596d9e 100644 (file)
--- a/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c
+++ b/tools/testing/selftests/bpf/progs/cgrp_kfunc_failure.c
@@ -78,7 +78,7 @@ int BPF_PROG(cgrp_kfunc_acquire_fp, struct cgroup *cgrp, const char *path)
 }
 
 SEC("kretprobe/cgroup_destroy_locked")
-__failure __msg("reg type unsupported for arg#0 function")
+__failure __msg("calling kernel function bpf_cgroup_acquire is not allowed")
 int BPF_PROG(cgrp_kfunc_acquire_unsafe_kretprobe, struct cgroup *cgrp)
 {
        struct cgroup *acquired;

diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
index dcdea312708633f74546f235693abe78d5019f40..ad88a3796ddff28b28d19cf190a8867618d5cd50 100644 (file)
--- a/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
+++ b/tools/testing/selftests/bpf/progs/task_kfunc_failure.c
@@ -248,7 +248,7 @@ int BPF_PROG(task_kfunc_from_pid_no_null_check, struct task_struct *task, u64 cl
 }
 
 SEC("lsm/task_free")
-__failure __msg("reg type unsupported for arg#0 function")
+__failure __msg("R1 must be a rcu pointer")
 int BPF_PROG(task_kfunc_from_lsm_task_free, struct task_struct *task)
 {
        struct task_struct *acquired;