Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

bpf: fix control-flow graph checking in privileged mode

When BPF program is verified in privileged mode, BPF verifier allows
bounded loops. This means that from CFG point of view there are
definitely some back-edges. Original commit adjusted check_cfg() logic
to not detect back-edges in control flow graph if they are resulting
from conditional jumps, which the idea that subsequent full BPF
verification process will determine whether such loops are bounded or
not, and either accept or reject the BPF program. At least that's my
reading of the intent.

Unfortunately, the implementation of this idea doesn't work correctly in
all possible situations. Conditional jump might not result in immediate
back-edge, but just a few unconditional instructions later we can arrive
at back-edge. In such situations check_cfg() would reject BPF program
even in privileged mode, despite it might be bounded loop. Next patch
adds one simple program demonstrating such scenario.

To keep things simple, instead of trying to detect back edges in
privileged mode, just assume every back edge is valid and let subsequent
BPF verification prove or reject bounded loops.

Note a few test changes. For unknown reason, we have a few tests that
are specified to detect a back-edge in a privileged mode, but looking at
their code it seems like the right outcome is passing check_cfg() and
letting subsequent verification to make a decision about bounded or not
bounded looping.

Bounded recursion case is also interesting. The example should pass, as
recursion is limited to just a few levels and so we never reach maximum
number of nested frames and never exhaust maximum stack depth. But the
way that max stack depth logic works today it falsely detects this as
exceeding max nested frame count. This patch series doesn't attempt to
fix this orthogonal problem, so we just adjust expected verifier failure.

Suggested-by: Alexei Starovoitov <ast@kernel.org>
Fixes: 2589726d12a1 ("bpf: introduce bounded loops")
Reported-by: Hao Sun <sunhao.th@gmail.com>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Link: https://lore.kernel.org/r/20231110061412.2995786-1-andrii@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

authored by

Andrii Nakryiko and committed by
Alexei Starovoitov
10e14e96 8c74b27f

+17 -21
+8 -15
kernel/bpf/verifier.c
··· 15403 15403 * w - next instruction 15404 15404 * e - edge 15405 15405 */ 15406 - static int push_insn(int t, int w, int e, struct bpf_verifier_env *env, 15407 - bool loop_ok) 15406 + static int push_insn(int t, int w, int e, struct bpf_verifier_env *env) 15408 15407 { 15409 15408 int *insn_stack = env->cfg.insn_stack; 15410 15409 int *insn_state = env->cfg.insn_state; ··· 15435 15436 insn_stack[env->cfg.cur_stack++] = w; 15436 15437 return KEEP_EXPLORING; 15437 15438 } else if ((insn_state[w] & 0xF0) == DISCOVERED) { 15438 - if (loop_ok && env->bpf_capable) 15439 + if (env->bpf_capable) 15439 15440 return DONE_EXPLORING; 15440 15441 verbose_linfo(env, t, "%d: ", t); 15441 15442 verbose_linfo(env, w, "%d: ", w); ··· 15458 15459 int ret, insn_sz; 15459 15460 15460 15461 insn_sz = bpf_is_ldimm64(&insns[t]) ? 2 : 1; 15461 - ret = push_insn(t, t + insn_sz, FALLTHROUGH, env, false); 15462 + ret = push_insn(t, t + insn_sz, FALLTHROUGH, env); 15462 15463 if (ret) 15463 15464 return ret; 15464 15465 ··· 15468 15469 15469 15470 if (visit_callee) { 15470 15471 mark_prune_point(env, t); 15471 - ret = push_insn(t, t + insns[t].imm + 1, BRANCH, env, 15472 - /* It's ok to allow recursion from CFG point of 15473 - * view. __check_func_call() will do the actual 15474 - * check. 15475 - */ 15476 - bpf_pseudo_func(insns + t)); 15472 + ret = push_insn(t, t + insns[t].imm + 1, BRANCH, env); 15477 15473 } 15478 15474 return ret; 15479 15475 } ··· 15490 15496 if (BPF_CLASS(insn->code) != BPF_JMP && 15491 15497 BPF_CLASS(insn->code) != BPF_JMP32) { 15492 15498 insn_sz = bpf_is_ldimm64(insn) ? 2 : 1; 15493 - return push_insn(t, t + insn_sz, FALLTHROUGH, env, false); 15499 + return push_insn(t, t + insn_sz, FALLTHROUGH, env); 15494 15500 } 15495 15501 15496 15502 switch (BPF_OP(insn->code)) { ··· 15537 15543 off = insn->imm; 15538 15544 15539 15545 /* unconditional jump with single edge */ 15540 - ret = push_insn(t, t + off + 1, FALLTHROUGH, env, 15541 - true); 15546 + ret = push_insn(t, t + off + 1, FALLTHROUGH, env); 15542 15547 if (ret) 15543 15548 return ret; 15544 15549 ··· 15550 15557 /* conditional jump with two edges */ 15551 15558 mark_prune_point(env, t); 15552 15559 15553 - ret = push_insn(t, t + 1, FALLTHROUGH, env, true); 15560 + ret = push_insn(t, t + 1, FALLTHROUGH, env); 15554 15561 if (ret) 15555 15562 return ret; 15556 15563 15557 - return push_insn(t, t + insn->off + 1, BRANCH, env, true); 15564 + return push_insn(t, t + insn->off + 1, BRANCH, env); 15558 15565 } 15559 15566 } 15560 15567
+6 -3
tools/testing/selftests/bpf/progs/verifier_loops1.c
··· 75 75 " ::: __clobber_all); 76 76 } 77 77 78 - SEC("tracepoint") 78 + SEC("socket") 79 79 __description("bounded loop, start in the middle") 80 - __failure __msg("back-edge") 80 + __success 81 + __failure_unpriv __msg_unpriv("back-edge") 81 82 __naked void loop_start_in_the_middle(void) 82 83 { 83 84 asm volatile (" \ ··· 137 136 138 137 SEC("tracepoint") 139 138 __description("bounded recursion") 140 - __failure __msg("back-edge") 139 + __failure 140 + /* verifier limitation in detecting max stack depth */ 141 + __msg("the call stack of 8 frames is too deep !") 141 142 __naked void bounded_recursion(void) 142 143 { 143 144 asm volatile (" \
+3 -3
tools/testing/selftests/bpf/verifier/calls.c
··· 442 442 BPF_EXIT_INSN(), 443 443 }, 444 444 .prog_type = BPF_PROG_TYPE_TRACEPOINT, 445 - .errstr = "back-edge from insn 0 to 0", 445 + .errstr = "the call stack of 9 frames is too deep", 446 446 .result = REJECT, 447 447 }, 448 448 { ··· 799 799 BPF_EXIT_INSN(), 800 800 }, 801 801 .prog_type = BPF_PROG_TYPE_TRACEPOINT, 802 - .errstr = "back-edge", 802 + .errstr = "the call stack of 9 frames is too deep", 803 803 .result = REJECT, 804 804 }, 805 805 { ··· 811 811 BPF_EXIT_INSN(), 812 812 }, 813 813 .prog_type = BPF_PROG_TYPE_TRACEPOINT, 814 - .errstr = "back-edge", 814 + .errstr = "the call stack of 9 frames is too deep", 815 815 .result = REJECT, 816 816 }, 817 817 {