From afbf75616dbf2b2e30fd359de1259b49c337d683 Mon Sep 17 00:00:00 2001 From: Eduard Zingerman Date: Fri, 19 Dec 2025 11:31:31 -0800 Subject: [PATCH 1/3] bpf: calls to bpf_loop() should have an SCC and accumulate backedges This is a correctness fix for the verification of BPF programs that work with callback-calling functions. The problem is the same as the issue fixed by series [1] for iterator-based loops: some of the states created while processing the callback function body might have incomplete read or precision marks. An example of an unsafe program that is accepted without this fix can be found in patch #2. There is some impact on verification performance: File Program Insns (A) Insns (B) Insns (DIFF) ------------------------------- -------------------- --------- --------- ----------------- pyperf600_bpf_loop.bpf.o on_event 3429 8259 +4830 (+140.86%) setget_sockopt.bpf.o skops_sockopt 4478 6043 +1565 (+34.95%) setget_sockopt.bpf.o socket_post_create 958 1271 +313 (+32.67%) strobemeta_bpf_loop.bpf.o on_event 2689 6312 +3623 (+134.73%) test_tcp_custom_syncookie.bpf.o tcp_custom_syncookie 9217 27385 +18168 (+197.11%) xdp_synproxy_kern.bpf.o syncookie_tc 6809 13773 +6964 (+102.28%) xdp_synproxy_kern.bpf.o syncookie_xdp 6819 13783 +6964 (+102.13%) Total progs: 4161 Old success: 2508 New success: 2507 total_insns diff min : 0.00 % total_insns diff max : 197.11 % total_insns abs max old : 806,540 total_insns abs max new : 806,540 0 .. 5 %: 4148 5 .. 15 %: 5 30 .. 40 %: 2 40 .. 50 %: 1 100 .. 110 %: 2 130 .. 140 %: 1 140 .. 150 %: 1 195 .. 200 %: 1 [1] https://lore.kernel.org/bpf/174968344350.3524559.14906547029551737094.git-patchwork-notify@kernel.org/ --- b4-submit-tracking --- { "series": { "revision": 1, "change-id": "20251219-scc-for-callbacks-d6d94faa2e43", "prefixes": [] } } From 8a402437b550f2198895b9d397d0db17074bad12 Mon Sep 17 00:00:00 2001 From: Eduard Zingerman Date: Wed, 26 Nov 2025 15:19:25 -0800 Subject: [PATCH 2/3] bpf: bpf_scc_visit instance and backedges accumulation for bpf_loop() Calls like bpf_loop() or bpf_for_each_map_elem() introduce loops that are not explicitly present in the control-flow graph. The verifier processes such calls by repeatedly interpreting the callback function body within the same verification path (until the current state converges with a previous state). Such loops require a bpf_scc_visit instance in order to allow the accumulation of the state graph backedges. Otherwise, certain checkpoint states created within the bodies of such loops will have incomplete precision marks. See the next patch for an example of a program that leads to the verifier accepting an unsafe program. Fixes: 96c6aa4c63af ("bpf: compute SCCs in program control flow graph") Fixes: c9e31900b54c ("bpf: propagate read/precision marks over state graph backedges") Reported-by: Breno Leitao Signed-off-by: Eduard Zingerman --- kernel/bpf/verifier.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index d6b8a77fbe3bf..65914c8fc9493 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19826,8 +19826,10 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx) } } if (bpf_calls_callback(env, insn_idx)) { - if (states_equal(env, &sl->state, cur, RANGE_WITHIN)) + if (states_equal(env, &sl->state, cur, RANGE_WITHIN)) { + loop = true; goto hit; + } goto skip_inf_loop_check; } /* attempt to detect infinite loop to avoid unnecessary doomed work */ @@ -25061,15 +25063,18 @@ static int compute_scc(struct bpf_verifier_env *env) } /* * Assign SCC number only if component has two or more elements, - * or if component has a self reference. + * or if component has a self reference, or if instruction is a + * callback calling function (implicit loop). */ - assign_scc = stack[stack_sz - 1] != w; - for (j = 0; j < succ->cnt; ++j) { + assign_scc = stack[stack_sz - 1] != w; /* two or more elements? */ + for (j = 0; j < succ->cnt; ++j) { /* self reference? */ if (succ->items[j] == w) { assign_scc = true; break; } } + if (bpf_calls_callback(env, w)) /* implicit loop? */ + assign_scc = true; /* Pop component elements from stack */ do { t = stack[--stack_sz]; From 1b0046c0d2311893aeaa71704e3fe5204f7545e5 Mon Sep 17 00:00:00 2001 From: Eduard Zingerman Date: Wed, 26 Nov 2025 15:40:53 -0800 Subject: [PATCH 3/3] selftests/bpf: test cases for bpf_loop SCC and state graph backedges Test for state graph backedges accumulation for SCCs formed by bpf_loop(). Equivalent to the following C program: int main(void) { 1: fp[-8] = bpf_get_prandom_u32(); 2: fp[-16] = -32; // used in a memory access below 3: bpf_loop(7, loop_cb4, fp, 0); 4: return 0; } int loop_cb4(int i, void *ctx) { 5: if (unlikely(ctx[-8] > bpf_get_prandom_u32())) 6: *(u64 *)(fp + ctx[-16]) = 42; // aligned access expected 7: if (unlikely(fp[-8] > bpf_get_prandom_u32())) 8: ctx[-16] = -31; // makes said access unaligned 9: return 0; } If state graph backedges are not accumulated properly at the SCC formed by loop_cb4() call from bpf_loop(), the state {ctx[-16]=-32} injected at instruction 9 on verification path 1,2,3,5,7,9,4 would be considered fully verified and would lack precision mark for ctx[-16]. This would lead to early pruning of verification path 1,2,3,5,7,8,9 in state {ctx[-16]=-31}, which in turn leads to the incorrect assumption that the above program is safe. Signed-off-by: Eduard Zingerman --- tools/testing/selftests/bpf/progs/iters.c | 75 +++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tools/testing/selftests/bpf/progs/iters.c b/tools/testing/selftests/bpf/progs/iters.c index 7dd92a303bf6b..69061f0309579 100644 --- a/tools/testing/selftests/bpf/progs/iters.c +++ b/tools/testing/selftests/bpf/progs/iters.c @@ -1926,4 +1926,79 @@ static int loop1_wrapper(void) ); } +/* + * This is similar to a test case absent_mark_in_the_middle_state(), + * but adapted for use with bpf_loop(). + */ +SEC("raw_tp") +__flag(BPF_F_TEST_STATE_FREQ) +__failure __msg("math between fp pointer and register with unbounded min value is not allowed") +__naked void absent_mark_in_the_middle_state4(void) +{ + /* + * Equivalent to a C program below: + * + * int main(void) { + * fp[-8] = bpf_get_prandom_u32(); + * fp[-16] = -32; // used in a memory access below + * bpf_loop(7, loop_cb4, fp, 0); + * return 0; + * } + * + * int loop_cb4(int i, void *ctx) { + * if (unlikely(ctx[-8] > bpf_get_prandom_u32())) + * *(u64 *)(fp + ctx[-16]) = 42; // aligned access expected + * if (unlikely(fp[-8] > bpf_get_prandom_u32())) + * ctx[-16] = -31; // makes said access unaligned + * return 0; + * } + */ + asm volatile ( + "call %[bpf_get_prandom_u32];" + "r8 = r0;" + "*(u64 *)(r10 - 8) = r0;" + "*(u64 *)(r10 - 16) = -32;" + "r1 = 7;" + "r2 = loop_cb4 ll;" + "r3 = r10;" + "r4 = 0;" + "call %[bpf_loop];" + "r0 = 0;" + "exit;" + : + : __imm(bpf_loop), + __imm(bpf_get_prandom_u32) + : __clobber_all + ); +} + +__used __naked +static void loop_cb4(void) +{ + asm volatile ( + "r9 = r2;" + "r8 = *(u64 *)(r9 - 8);" + "r6 = *(u64 *)(r9 - 16);" + "call %[bpf_get_prandom_u32];" + "if r0 > r8 goto use_fp16_%=;" + "1:" + "call %[bpf_get_prandom_u32];" + "if r0 > r8 goto update_fp16_%=;" + "2:" + "r0 = 0;" + "exit;" + "use_fp16_%=:" + "r1 = r10;" + "r1 += r6;" + "*(u64 *)(r1 + 0) = 42;" + "goto 1b;" + "update_fp16_%=:" + "*(u64 *)(r9 - 16) = -31;" + "goto 2b;" + : + : __imm(bpf_get_prandom_u32) + : __clobber_all + ); +} + char _license[] SEC("license") = "GPL";