Re: [PATCH bpf-next 2/5] selftests/bpf: libarena: Fix can-loop zero variable definition

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `barrier_var()` clobber in open-coded loops allows compiler Loop Strength Reduction, leading to verifier failures.
--

commit 3447132bcf1b724653f0483db116e1292c49b493
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: libarena: Fix can-loop zero variable definition

This patch modifies the zero variable to be a regular volatile instead of
a const volatile, ensuring it remains imprecise in BPF can_loop constructs.
Because this change can cause the compiler to optimize array accesses into
pointer increments (which breaks verification), barrier_var() is added to
several loops.

> diff --git a/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c b/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c
> index b45a306816c0e..5628f09870121 100644
> --- a/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c
> +++ b/tools/testing/selftests/bpf/libarena/selftests/test_buddy.bpf.c
[ ... ]
> @@ -171,7 +171,8 @@ __weak int test_buddy_alignment(void)
>  SEC("syscall")
>  __weak int test_buddy_alignment(void)
>  {
> -	int ret, i;
> +	int ret;
> +	u32 i;
>  
>  	ret = buddy_init(&buddy);
>  	if (ret)
> @@ -179,6 +180,7 @@ __weak int test_buddy_alignment(void)
>  
>  	/* Allocate various sizes and check alignment */
>  	for (i = zero; i < 17 && can_loop; i++) {
> +		barrier_var(i);
>  		ptrs[i] = buddy_alloc(&buddy, alignment_sizes[i]);

[Severity: Medium]
The commit message notes that modifying the zero variable definition requires
using barrier_var() to prevent the compiler from optimizing array dereferences
into pointer increments, which causes verification failures.

Since zero is now globally volatile, do we need to add barrier_var() to other
loops that use zero for array dereferences to prevent loop strength reduction?

For instance, in test_buddy.bpf.c:test_buddy_alloc_multiple():

    for (j = zero; j < sz && can_loop; j++) {
        mem[j] = poison;
    }

And the outer loop in the same function:

    for (i = zero; i < SEGARRLEN && can_loop; i++) {
        sz = alloc_multiple_sizes[i % 9];
        ...
        segarr[i].block = mem;
    }

Similar tight loops exist in other files, such as spmc_common_init()
in test_parallel_spmc.bpf.c:

    for (i = zero; i < TEST_SPMC_MAX_VALUES && can_loop; i++)
        seen[i] = 0;

And asan_test_buddy_oob_single() in test_asan_buddy.bpf.c:

    for (i = zero; i < alloc_size && can_loop; i++) {
        mem[i] = 0xba;
    }

Will these un-barriered loops also suffer from the same verification failures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618085626.19633-1-emil@etsalapatis.com?part=2