@@ -1659,6 +1659,7 @@ static int copy_verifier_state(struct bpf_verifier_state *dst_state,
dst_state->callback_unroll_depth = src->callback_unroll_depth;
dst_state->used_as_loop_entry = src->used_as_loop_entry;
dst_state->may_goto_depth = src->may_goto_depth;
+ dst_state->loop_entry = src->loop_entry;
for (i = 0; i <= src->curframe; i++) {
dst = dst_state->frame[i];
if (!dst) {
@@ -19230,6 +19231,8 @@ static int do_check(struct bpf_verifier_env *env)
return err;
break;
} else {
+ if (WARN_ON_ONCE(env->cur_state->loop_entry))
+ env->cur_state->loop_entry = NULL;
do_print_state = true;
continue;
}
The bpf_verifier_state.loop_entry state should be copied by copy_verifier_state(). Otherwise, .loop_entry values from unrelated states would poison env->cur_state. Additionally, env->stack should not contain any states with .loop_entry != NULL. The states in env->stack are yet to be verified, while .loop_entry is set for states that reached an equivalent state. This means that env->cur_state->loop_entry should always be NULL after pop_stack(). See the selftest in the next commit for an example of the program that is not safe yet is accepted by verifier w/o this fix. This change has some verification performance impact for selftests: Program Insns (DIFF) States (DIFF) ---------------------------- -------------- ------------- arena_htab_llvm -291 (-40.59%) -20 (-35.09%) arena_htab_asm -152 (-25.46%) -10 (-21.28%) arena_list_del -30 (-9.71%) -9 (-39.13%) checkpoint_states_deletion -5 (-0.03%) -1 (-0.12%) iter_nested_deeply_iters -26 (-4.38%) -4 (-5.97%) iter_subprog_check_stacksafe -14 (-9.03%) -1 (-6.67%) iter_subprog_iters -91 (-8.32%) -5 (-5.68%) loop_state_deps2 +246 (+51.36%) +17 (+36.96%) open_coded_iter -4 (-6.35%) -1 (-14.29%) on_event -320 (-2.59%) -80 (-18.14%) big_alloc2 +7 (+0.24%) +1 (+0.65%) max_words -8 (-8.70%) -1 (-12.50%) cond_break2 -6 (-5.31%) +0 (+0.00%) And significant negative impact for sched_ext: Program Insns (DIFF) States (DIFF) ---------------------- -------------------- ------------------ lavd_cpu_offline +4 (+0.09%) -1 (-0.32%) lavd_cpu_online +4 (+0.09%) -1 (-0.32%) lavd_enqueue -4 (-0.08%) +0 (+0.00%) lavd_init +7684 (+109.40%) +649 (+133.26%) layered_dispatch -937 (-8.16%) -86 (-10.14%) layered_dump +992580 (+13375.29%) +30497 (+4478.27%) layered_enqueue -2733 (-20.91%) -260 (-22.07%) layered_runnable -435 (-13.52%) -38 (-12.88%) refresh_layer_cpumasks +658273 (+3992.68%) +63600 (+3593.22%) rustland_init -22 (-4.42%) -4 (-9.76%) rustland_init -22 (-4.42%) -4 (-9.76%) rusty_init +1917 (+12.72%) +175 (+22.76%) rusty_init_task +135 (+0.32%) +10 (+0.45%) rusty_select_cpu +75128 (+3580.93%) +5807 (+3208.29%) rusty_set_cpumask -15799 (-78.00%) -1349 (-81.02%) central_dispatch +2051 (+322.48%) +164 (+260.32%) nest_init +179 (+28.14%) +13 (+21.67%) qmap_dispatch +1187 (+49.60%) +57 (+29.08%) qmap_dump +85 (+36.48%) +8 (+36.36%) qmap_init +1069 (+6.53%) +66 (+10.95%) Note 'layered_dump' program, which now hits 1M instructions limit. This impact would be mitigated in the next patch. Signed-off-by: Eduard Zingerman <eddyz87@gmail.com> --- kernel/bpf/verifier.c | 3 +++ 1 file changed, 3 insertions(+)