Re: [PATCH bpf-next] bpf: Fix bpf_loop() depth check to use 32-bit max for nr_loops

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]

> bpf: Fix bpf_loop() depth check to use 32-bit max for nr_loops
>
> bpf_loop() takes a u32 nr_loops argument. At runtime, nr_loops is
> passed as a u32 via the BPF_CALL_4 signature:
>
>     BPF_CALL_4(bpf_loop, u32, nr_loops, ...)
>
> However, the verifier compares callback_depth (u32) against
> reg_umax(), which returns the 64-bit upper bound of R1. If the upper
> 32 bits of R1 are set, reg_umax() can be U64_MAX, causing the depth
> check to remain ineffective and triggering excessive
> push_callback_call() invocations until the complexity limit is hit.
>
> Use reg_u32_max() instead, matching the 32-bit range actually used by
> bpf_loop() and avoiding unnecessary state exploration when R1 has a
> large 64-bit range.
>
> Signed-off-by: Ömer Mete Kaya <omermetekaya0@gmail.com>

The changelog justifies the switch to reg_u32_max() by saying the runtime
only ever uses the low 32 bits of R1 ("nr_loops is passed as a u32 via the
BPF_CALL_4 signature", "matching the 32-bit range actually used by
bpf_loop()").

That holds for the out-of-line helper in kernel/bpf/bpf_iter.c, but not
for the inlined variant. When loop_inline_state.fit_for_inline is set
(flags known zero and a single callback subprog), which is the common case,
bpf_optimize_bpf_loop() replaces the call with inline_bpf_loop() in
kernel/bpf/fixups.c. That code works on the full 64-bit R1:

    insn_buf[cnt++] = BPF_JMP_IMM(BPF_JLE, BPF_REG_1, BPF_MAX_LOOPS, 2);
    insn_buf[cnt++] = BPF_MOV32_IMM(BPF_REG_0, -E2BIG);
    ...
    insn_buf[cnt++] = BPF_MOV64_REG(reg_loop_max, BPF_REG_1);
    ...
    insn_buf[cnt++] = BPF_JMP_REG(BPF_JGE, reg_loop_cnt, reg_loop_max, 5);

For example, R1 = 0x100000005 runs the callback 5 times through the helper
but returns -E2BIG with zero iterations when inlined.

The patch is still sound for both variants. Any R1 with upper bits set
fails the 64-bit R1 <= BPF_MAX_LOOPS check in the inlined code, and any R1
that passes it equals (u32)R1 <= cnum32_umax(r32). However, the changelog
does not make that argument, and its wording implies there is a single
32-bit runtime semantic.

For a change that reduces how many callback iterations the verifier
simulates, the soundness argument for the inlined path should be in the
commit message. A selftest where R1 has unknown upper 32 bits and bounded
low 32 bits would also help, for example a JMP32 bound on a 64-bit value.

Could the changelog be updated to cover the inlined bpf_loop() case?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36358690326