Re: [PATCH bpf-next 2/2] selftests/bpf: Test for empty intersection of tnum and u64

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] The test relies on the verifier treating the return value of `bpf_get_prandom_u32()` as a completely unbounded 64-bit scalar. If the verifier is improved to track its true 32-bit bounds, the test will silently stop testing the intended logic.
- [Medium] The inline comments in `empty_tnum_u64_intersection` incorrectly describe the verifier state at the wrong instruction boundaries and use incorrect terminology.
--

commit f07d8c6131ab63ef38acb708d393fc328146ef7c
Author: Paul Chaignon <paul.chaignon@gmail.com>

selftests/bpf: Test for empty intersection of tnum and u64

This commit adds a verifier selftest for a scenario where the tnum and u64
values don't intersect on a branch. It uses bpf_get_prandom_u32() and
conditional jumps to test if the verifier correctly deduces that the branch
is dead code.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> index c1ae013dee29c..151c7a479a6b8 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c

[ ... ]

> @@ -2184,4 +2184,29 @@ __naked void tnums_equal_impossible_constant(void *ctx)
>  	: __clobber_all);
>  }
>  
> +SEC("socket")
> +__description("dead branch: tnum and u64 don't intersect on true branch")
> +__success
> +__naked void empty_tnum_u64_intersection(void *ctx)
> +{
> +	asm volatile("									\
> +	call %[bpf_get_prandom_u32];							\
> +	r6 = r0;									\

Since bpf_get_prandom_u32() returns a u32, its runtime value is strictly
within [0, 0xffffffff]. If a future verifier update bounds prandom_u32() to
this range, the verifier will accurately determine that the jump
(r6 s< 0xe00000002) is always taken and prune the fallthrough path as dead
code.

Would this cause the test to silently pass while no longer providing any test
coverage for the tnum intersection logic? Could a truly unknown 64-bit value
be constructed instead to prevent this test fragility?

> +	r1 = 0xe00000002 ll;								\
> +	r2 = 0xf00000000 ll;								\
> +	if r6 s< r1 goto +2;								\
> +	/* On fallthrough, r6's u64=[0xe00000002;0xf00000000] */			\

Are these inline comments describing the verifier state accurately? The upper
bound of 0xf00000000 does not seem to be established until the fallthrough of
the next instruction (if r6 s> r2 goto +1).

> +	/*                      tnum=(0xe00000000; 0x1ffffffff) */			\
> +	if r6 s> r2 goto +1;								\
> +	/* On fallthrough, r6's tnum=(0xe00000001; 0x100000000) */			\

Does this tnum state actually describe the fallthrough path? It appears to
describe the mathematically deduced state on the jump target (true branch) of
the w6 == 1 check, making the term "fallthrough" incorrect here.

> +	/* It doesn't intersect with the u64 so the condition is always false */	\
> +	if w6 == 1 goto +1;								\

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260415160728.657270-1-harishankar.vishwanathan@gmail.com?part=2