From c16c6c4683cc57dd2736750da12e04f1f74a45e1 Mon Sep 17 00:00:00 2001 From: Alexei Starovoitov Date: Sat, 31 Oct 2026 21:55:42 +0000 Subject: [PATCH] bpf: End bpf_loop() when the time of may_goto is up When the time of a prog is up all loops with may_goto end, but bpf_loop() goes on. It calls its callback up to 1 << 23 times. The loops in the callback make no iteration, but the rest of the callback runs, and that can be another bpf_loop(). Two test progs with bpf_loop(1 << 23, cb) and some work in cb return after 4 sec and 10 sec. With this patch they return after 250 ms. bpf_loop() and bpf_for_each_map_elem() end their loop when the callback returns 1. So return 1 in the first insns of the callback when the time is up: r0 = address of the count r0 = &(void __percpu *)(r0) r0 = *(u64 *)(r0 +0) if r0 != 0x0 goto pc+2 r0 = 1 exit The verifier has seen what follows: the loop of the helper can end after each call of the callback. A function that BPF code calls too, or that is the callback of another helper or kfunc, stays as it is: there the return value has another meaning. The callback of bpf_user_ringbuf_drain() stays as it is too. The helper takes a sample out of the buffer for each call, so a call that does nothing loses the sample. Its loop ends after 128K samples. This patch does not limit bpf_loop() itself. Only may_goto counts and reads the clock. So nested bpf_loop() are not stopped when the prog has no may_goto, or has none in the callbacks: bpf_loop(1 << 20, cb) with bpf_loop(4096, cb2) in cb takes 5 sec. Nested open coded iterators have the same problem. A follow up will fix these holes. A call of such a callback executes 4 more insns. The cost on x86-64 (Zen 5) in nsec: before after callback makes 1 iteration, inlined bpf_loop() 1.16 1.54 callback makes 1 iteration, bpf_loop() 6.52 6.74 callback without may_goto, inlined bpf_loop() 1.16 1.16 callback without may_goto, bpf_loop() 4.95 4.95 callback without may_goto, nested bpf_loop() 1.13 1.38 Signed-off-by: Alexei Starovoitov Link: https://patch.msgid.link/20261031215543.682284-9-alexei.starovoitov@gmail.com --- include/linux/bpf_verifier.h | 4 ++++ kernel/bpf/fixups.c | 26 +++++++++++++++++++++----- kernel/bpf/verifier.c | 7 +++++++ 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 228a3f6889c2..b43eb97bcd44 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -969,6 +969,10 @@ struct bpf_subprog_info { bool is_cb: 1; bool is_async_cb: 1; bool is_exception_cb: 1; + /* bpf_loop() or bpf_for_each_map_elem() call it in a loop */ + bool is_loop_cb: 1; + /* BPF code, another helper or a kfunc call it */ + bool is_called: 1; bool args_cached: 1; /* true if the return value is passed in the R0:R2 register pair */ bool ret_reg_pair: 1; diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index 66b9d1d9e961..a656634fa9e4 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -2771,9 +2771,12 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) int stack_slots = subprogs[i].stack_extra / 8; /* an invocation begins, see bpf_timed_may_goto_save() for async callbacks */ bool main_prog = tmg && !i; + /* only a helper calls it, in a loop that ends when it returns 1 */ + bool loop_cb = tmg && i && subprogs[i].is_loop_cb && !subprogs[i].is_called && + !subprogs[i].is_async_cb; int cnt = 0; - if (timed_may_goto ? !main_prog : !stack_slots) + if (timed_may_goto ? !main_prog && !loop_cb : !stack_slots) continue; if (stack_slots > 1) { verifier_bug(env, "stack_slots supports may_goto only"); @@ -2793,10 +2796,23 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[cnt++] = ld_tmg[0]; insn_buf[cnt++] = ld_tmg[1]; insn_buf[cnt++] = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0); - insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, count_off, - BPF_MAX_TIMED_LOOPS); - insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, timestamp_off, 0); - insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, owner_off, 0); + if (main_prog) { + insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, count_off, + BPF_MAX_TIMED_LOOPS); + insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, timestamp_off, 0); + insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, owner_off, 0); + } else { + /* + * The time is up, but the helper would call the function again and + * again. End its loop. The verifier has seen that: the loop can end + * after each call. + */ + insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, + count_off); + insn_buf[cnt++] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2); + insn_buf[cnt++] = BPF_MOV64_IMM(BPF_REG_0, 1); + insn_buf[cnt++] = BPF_EXIT_INSN(); + } } else { /* Add ST insn to subprog prologue to init extra stack */ insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_FP, -stack_depth, diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index fb96c61ebec0..5e4700fda66b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11508,6 +11508,12 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins if (err) return err; + if (!bpf_pseudo_kfunc_call(insn) && + (insn->imm == BPF_FUNC_loop || insn->imm == BPF_FUNC_for_each_map_elem)) + env->subprog_info[subprog].is_loop_cb = true; + else + env->subprog_info[subprog].is_called = true; + /* for callback functions enqueue entry to callback and * proceed with next instruction within current frame. */ @@ -11544,6 +11550,7 @@ static int check_static_func_call(struct bpf_verifier_env *env, int subprog, int err; caller = state->frame[state->curframe]; + env->subprog_info[subprog].is_called = true; /* * Track caller's total stack arg count (incoming + max outgoing).