Re: [PATCH bpf v2 2/2] selftests/bpf: Cover frame changes in bounded loops

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-09-18 at 03:13 +0200, Kumar Kartikeya Dwivedi wrote:
> Add two finite loops whose progress is represented only by changing the
> frame number of a stack pointer. Each loop first reads zero from the
> caller's stack, switches to the same offset in the callee's stack, and
> exits after reading one on its next iteration.
> 
> Keep the pointer in a register in one test and spill it in the other to
> exercise both regsafe() and stacksafe() during exact state comparison.
> Force frequent checkpoints so the tests exercise infinite-loop detection,
> and check that both programs return one when run.
> 
> Without the frameno comparison in regs_exact(), both programs are
> rejected with an "infinite loop detected" diagnostic instead of loading
> successfully.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Tested-by: Eduard Zingerman <eddyz87@gmail.com>

>  .../selftests/bpf/progs/verifier_loops1.c     | 70 +++++++++++++++++++
>  1 file changed, 70 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/progs/verifier_loops1.c b/tools/testing/selftests/bpf/progs/verifier_loops1.c
> index d248ce877f14..ada7e1105a95 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_loops1.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_loops1.c
> @@ -303,4 +303,74 @@ __naked void maybe_exit_scc_bug1(void)
>  	::: __clobber_all);
>  }
>  
> +/*
> + * The loop reads zero from the caller's stack on its first iteration and
> + * one from the callee's stack on its second iteration. At the loop header,
> + * only the frame number of the pointer in r1 changes.
> + */
> +static __naked __noinline __used
> +void loop_stack_frames_reg(void)
> +{
> +	asm volatile (
> +	"*(u64 *)(r10 - 8) = 1;"
> +	"1: r0 = *(u64 *)(r1 + 0);"

Nit: please use formatting as below:

  "1:"
	"r0 = *(u64 *)(r1 + 0);"

> +	"if r0 != 0 goto 2f;"
> +	"r1 = r10;"
> +	"r1 += -8;"
> +	"goto 1b;"
> +	"2: exit;"
> +	::: __clobber_all);
> +}
> +
> +SEC("xdp")
> +__description("bounded loop changing stack frame in a register")
> +__success __retval(1)
> +__flag(BPF_F_TEST_STATE_FREQ)
> +__naked void bounded_loop_stack_frames_reg(void)
> +{
> +	asm volatile (
> +	"*(u64 *)(r10 - 8) = 0;"
> +	"r1 = r10;"
> +	"r1 += -8;"
> +	"call loop_stack_frames_reg;"
> +	"exit;"
> +	::: __clobber_all);
> +}

--- 8< ---

> +/*
> + * Exercise the same frame change through a spilled pointer. All registers
> + * at the loop header are identical, while fp-16 points to a different frame.
> + */
> +static __naked __noinline __used
> +void loop_stack_frames_spill(void)
> +{
> +	asm volatile (
> +	"*(u64 *)(r10 - 8) = 1;"
> +	"*(u64 *)(r10 - 16) = r1;"
> +	"1: r1 = *(u64 *)(r10 - 16);"
> +	"r0 = *(u64 *)(r1 + 0);"
> +	"if r0 != 0 goto 2f;"
> +	"r1 = r10;"
> +	"r1 += -8;"
> +	"*(u64 *)(r10 - 16) = r1;"
> +	"goto 1b;"
> +	"2: exit;"
> +	::: __clobber_all);
> +}
> +
> +SEC("xdp")
> +__description("bounded loop changing stack frame in a spill")
> +__success __retval(1)
> +__flag(BPF_F_TEST_STATE_FREQ)
> +__naked void bounded_loop_stack_frames_spill(void)
> +{
> +	asm volatile (
> +	"*(u64 *)(r10 - 8) = 0;"
> +	"r1 = r10;"
> +	"r1 += -8;"
> +	"call loop_stack_frames_spill;"
> +	"exit;"
> +	::: __clobber_all);
> +}
> +
>  char _license[] SEC("license") = "GPL";

I think this test is redundant, existing tests should already cover
the chain stack_safe -> regsafe.

--- >8 ---