Re: [PATCH bpf-next v1 11/18] selftests/bpf: Check liveness merge of masks with different widths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: merge_keeps_whole_frame_read test is vacuous on some 64-bit archs
--

commit 1299c778ee1ba2ffa8d447fa3ce55a6d2aec24cd
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Check liveness merge of masks with different widths

This commit adds a selftest to verify that merge_instances() correctly
widens liveness masks of different widths when a function instance is
analyzed through different call sites.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_live_stack.c b/tools/testing/selftests/bpf/progs/verifier_live_stack.c
> index f8758eb62dac5..a94365decd5f1 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_live_stack.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_live_stack.c
> @@ -2861,3 +2861,52 @@ __naked void narrow_store_defines_nothing(void)

[ ... ]

> +SEC("socket")
> +__log_level(2)
> +__msg("stack use/def subprog#{{[0-9]+}} merge_read_all_callee (d2,cs{{[0-9]+}}):")
> +__msg("(79) r0 = *(u64 *)(r1 +0){{.*}}; use: fp0-8..-512")
> +__naked void merge_keeps_whole_frame_read(void)
> +{
> +	asm volatile (
> +	"r1 = 0;"
> +	"*(u64 *)(r10 - 8) = r1;"
> +	"*(u64 *)(r10 - 16) = r1;"
> +	"*(u64 *)(r10 - 264) = r1;"

[Severity: Low]
Is this test effectively vacuous on 64-bit architectures with a 512-byte stack
limit?

The precise 8-byte read at offset -264 touches half-slot 66. This means the
mask requires 66 bits, which on a 64-bit architecture occupies 2 words.

The second pass performs an unknown-offset read, forcing a whole-frame mask.
For a 512-byte stack limit, this covers 128 half-slots (128 bits), which
also occupies 2 words.

Because both masks occupy 2 words, the actual widening logic in
widen_frame_masks() is silently skipped, missing the intended test coverage.

While later patches in this series might raise the stack limit for some
architectures, the test remains mathematically vacuous on 64-bit architectures
that retain the 512-byte limit (e.g., RISC-V, s390x, or when JIT is off).

Would changing the precise read to an offset <= 256 (e.g., -248) correctly
produce a 1-word mask and test the intended logic universally?

> +	"call %[bpf_get_prandom_u32];"
> +	"r0 &= 8;"
> +	"r1 = r10;"
> +	"r1 += -16;"
> +	"r1 += r0;"
> +	"call merge_read_all_mid;"
> +	"r1 = r10;"
> +	"r1 += -264;"
> +	"call merge_read_all_mid;"
> +	"r0 = 0;"
> +	"exit;"
> +	:: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923191139.2816206-1-memxor@gmail.com?part=11