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

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Wed Sep 23, 2026 at 9:26 PM CEST, sashiko-bot wrote:
> 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?
>

Plausible, will investigate.

>> +	"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);
>> +}