Re: [PATCH bpf 08/11] selftests/bpf: Test for mixed arena/nonarena code paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: verifier tests for mixed-type CFG paths are vulnerable to compiler optimizations
--

commit 2865d7b4af7e52310b9ec10118abb32164639af3
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: Test for mixed arena/nonarena code paths

Add a selftest to confirm the verifier rejects ALU operations
that return arena or non-arena results depending on code path.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> index 332322c9b54cf..1ce9c4b784816 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
> @@ -561,6 +561,55 @@ int arena_ptr_add_arena_ptr(void *ctx)
>  	return 0;
>  }
>  
> +SEC("syscall")
> +__failure __msg("same insn cannot be used with and without arena pointer")
> +int mixed_arena_scalar_alu64_scalar_first(void *ctx)
> +{
> +	volatile register __u64 reg asm("r3");
> +	__u32 pick_arena = bpf_get_prandom_u32();
> +
> +	reg = 1ULL << 32;
> +
> +	if (pick_arena) {
> +		asm volatile (
> +			"r9 = %[arena] ll;"
> +			"%[reg] = 0;"
> +			"%[reg] = addr_space_cast(%[reg], 0x0, 0x1);"
> +			: [reg] "=r"(reg)
> +			: __imm_addr(arena)
> +			: "r9"
> +		);
> +	}
> +
> +	reg += 1;
> +
> +	return 0;

[Severity: Medium]
Are these tests vulnerable to compiler optimizations?

Since these tests rely on the control flow converging to evaluate a single
instruction with mixed types (SCALAR_VALUE and PTR_TO_ARENA), could compiler
optimizations like Clang's tail duplication duplicate the reg += 1; return 0;
block into both branches?

If the tail block is duplicated, the verifier would evaluate two separate
addition instructions, each with a uniform type, bypassing the mixed-type
check entirely and causing the test to fail to produce the expected
__failure __msg(...).

Should these functions be written using __naked and explicit assembly (like
mixed_arena_stack_atomic) to guarantee the exact instruction sharing
required for this test?

> +}
> +
> +SEC("syscall")
> +__failure __msg("same insn cannot be used with and without arena pointer")
> +int mixed_arena_scalar_alu64_arena_first(void *ctx)
> +{
> +	volatile register __u64 reg asm("r3");
> +	__u32 pick_scalar = bpf_get_prandom_u32();
> +
> +	asm volatile (
> +		"r9 = %[arena] ll;"
> +		"%[reg] = 0;"
> +		"%[reg] = addr_space_cast(%[reg], 0x0, 0x1);"
> +		: [reg] "=r"(reg)
> +		: __imm_addr(arena)
> +		: "r9"
> +	);
> +
> +	if (pick_scalar)
> +		reg = 1ULL << 32;
> +
> +	reg += 1;
> +
> +	return 0;

[Severity: Medium]
Similarly, could the tail addition here be duplicated into both execution
paths by the compiler, invalidating the test's mixed-type validation logic?

> +}

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