bpf: Validate global subprogs lazily

Slightly change BPF verifier logic around eagerness and order of global
subprog validation. Instead of going over every global subprog eagerly
and validating it before main (entry) BPF program is verified, turn it
around. Validate main program first, mark subprogs that were called from
main program for later verification, but otherwise assume it is valid.
Afterwards, go over marked global subprogs and validate those,
potentially marking some more global functions as being called. Continue
this process until all (transitively) callable global subprogs are
validated. It's a BFS traversal at its heart and will always converge.

This is an important change because it allows to feature-gate some
subprograms that might not be verifiable on some older kernel, depending
on supported set of features.

E.g., at some point, global functions were allowed to accept a pointer
to memory, which size is identified by user-provided type.
Unfortunately, older kernels don't support this feature. With BPF CO-RE
approach, the natural way would be to still compile BPF object file once
and guard calls to this global subprog with some CO-RE check or using
.rodata variables. That's what people do to guard usage of new helpers
or kfuncs, and any other new BPF-side feature that might be missing on
old kernels.

That's currently impossible to do with global subprogs, unfortunately,
because they are eagerly and unconditionally validated. This patch set
aims to change this, so that in the future when global funcs gain new
features, those can be guarded using BPF CO-RE techniques in the same
fashion as any other new kernel feature.

Two selftests had to be adjusted in sync with these changes.

test_global_func12 relied on eager global subprog validation failing
before main program failure is detected (unknown return value). Fix by
making sure that main program is always valid.

verifier_subprog_precision's parent_stack_slot_precise subtest relied on
verifier checkpointing heuristic to do a checkpoint at instruction #5,
but that's no longer true because we don't have enough jumps validated
before reaching insn #5 due to global subprogs being validated later.

Other than that, no changes, as one would expect.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20231124035937.403208-3-andrii@kernel.org

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 258ba232e30215d6ba05feec560b5f2f34bb799a..eb447b0a942313ac5d361b34d52c6ea6b674f8a7 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1347,6 +1347,8 @@ static inline bool bpf_prog_has_trampoline(const struct bpf_prog *prog)
 struct bpf_func_info_aux {
        u16 linkage;
        bool unreliable;
+       bool called : 1;
+       bool verified : 1;
 };
 
 enum bpf_jit_poke_reason {

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a2939ebf26385e34fc9398b0360451c2303af72b..8e7b6072e3f4dbabc1fdc4f0e48362db62e1d5ed 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -434,6 +434,11 @@ static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
        return btf_type_name(env->prog->aux->btf, info->type_id);
 }
 
+static struct bpf_func_info_aux *subprog_aux(const struct bpf_verifier_env *env, int subprog)
+{
+       return &env->prog->aux->func_info_aux[subprog];
+}
+
 static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 {
        return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK);
@@ -9290,6 +9295,8 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 
                verbose(env, "Func#%d ('%s') is global and assumed valid.\n",
                        subprog, sub_name);
+               /* mark global subprog for verifying after main prog */
+               subprog_aux(env, subprog)->called = true;
                clear_caller_saved_regs(env, caller->regs);
 
                /* All global functions return a 64-bit SCALAR_VALUE */
@@ -19873,8 +19880,11 @@ out:
        return ret;
 }
 
-/* Verify all global functions in a BPF program one by one based on their BTF.
- * All global functions must pass verification. Otherwise the whole program is rejected.
+/* Lazily verify all global functions based on their BTF, if they are called
+ * from main BPF program or any of subprograms transitively.
+ * BPF global subprogs called from dead code are not validated.
+ * All callable global functions must pass verification.
+ * Otherwise the whole program is rejected.
  * Consider:
  * int bar(int);
  * int foo(int f)
@@ -19893,14 +19903,26 @@ out:
 static int do_check_subprogs(struct bpf_verifier_env *env)
 {
        struct bpf_prog_aux *aux = env->prog->aux;
-       int i, ret;
+       struct bpf_func_info_aux *sub_aux;
+       int i, ret, new_cnt;
 
        if (!aux->func_info)
                return 0;
 
+       /* exception callback is presumed to be always called */
+       if (env->exception_callback_subprog)
+               subprog_aux(env, env->exception_callback_subprog)->called = true;
+
+again:
+       new_cnt = 0;
        for (i = 1; i < env->subprog_cnt; i++) {
-               if (aux->func_info_aux[i].linkage != BTF_FUNC_GLOBAL)
+               if (!subprog_is_global(env, i))
+                       continue;
+
+               sub_aux = subprog_aux(env, i);
+               if (!sub_aux->called || sub_aux->verified)
                        continue;
+
                env->insn_idx = env->subprog_info[i].start;
                WARN_ON_ONCE(env->insn_idx == 0);
                ret = do_check_common(env, i, env->exception_callback_subprog == i);
@@ -19910,7 +19932,21 @@ static int do_check_subprogs(struct bpf_verifier_env *env)
                        verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
                                i, subprog_name(env, i));
                }
+
+               /* We verified new global subprog, it might have called some
+                * more global subprogs that we haven't verified yet, so we
+                * need to do another pass over subprogs to verify those.
+                */
+               sub_aux->verified = true;
+               new_cnt++;
        }
+
+       /* We can't loop forever as we verify at least one global subprog on
+        * each pass.
+        */
+       if (new_cnt)
+               goto again;
+
        return 0;
 }
 
@@ -20556,8 +20592,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
        if (ret < 0)
                goto skip_full_check;
 
-       ret = do_check_subprogs(env);
-       ret = ret ?: do_check_main(env);
+       ret = do_check_main(env);
+       ret = ret ?: do_check_subprogs(env);
 
        if (ret == 0 && bpf_prog_is_offloaded(env->prog->aux))
                ret = bpf_prog_offload_finalize(env);

diff --git a/tools/testing/selftests/bpf/progs/test_global_func12.c b/tools/testing/selftests/bpf/progs/test_global_func12.c
index 7f159d83c6f673f78f3b7c89e9ce7e1cfffdeef4..6e03d42519a675398011c76e069f674baf4307a6 100644 (file)
--- a/tools/testing/selftests/bpf/progs/test_global_func12.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func12.c
@@ -19,5 +19,7 @@ int global_func12(struct __sk_buff *skb)
 {
        const struct S s = {.x = skb->len };
 
-       return foo(&s);
+       foo(&s);
+
+       return 1;
 }

diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
index f61d623b1ce8dfe1c4b1355c036e2b74e71d7227..b5efcaeaa1ae97dcd9c0ed233a9d08114ff8994c 100644 (file)
--- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
@@ -370,12 +370,10 @@ __naked int parent_stack_slot_precise(void)
 SEC("?raw_tp")
 __success __log_level(2)
 __msg("9: (0f) r1 += r6")
-__msg("mark_precise: frame0: last_idx 9 first_idx 6")
+__msg("mark_precise: frame0: last_idx 9 first_idx 0")
 __msg("mark_precise: frame0: regs=r6 stack= before 8: (bf) r1 = r7")
 __msg("mark_precise: frame0: regs=r6 stack= before 7: (27) r6 *= 4")
 __msg("mark_precise: frame0: regs=r6 stack= before 6: (79) r6 = *(u64 *)(r10 -8)")
-__msg("mark_precise: frame0: parent state regs= stack=-8:")
-__msg("mark_precise: frame0: last_idx 5 first_idx 0")
 __msg("mark_precise: frame0: regs= stack=-8 before 5: (85) call pc+6")
 __msg("mark_precise: frame0: regs= stack=-8 before 4: (b7) r1 = 0")
 __msg("mark_precise: frame0: regs= stack=-8 before 3: (7b) *(u64 *)(r10 -8) = r6")