Re: [PATCH bpf-next v2 06/11] selftests/bpf: Test trampoline args around a 16-byte aligned stack arg

[Yonghong Song <yonghong.song@linux.dev>]

On 10/13/26 2:43 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] bpf: endianness assumption causes selftest failure on aarch64 big-endian
> --
>
> commit df1efd6b9c0e934bad453d07e6efe674bda8e1e6
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> selftests/bpf: Test trampoline args around a 16-byte aligned stack arg
>
> This commit adds tests for bpf_testmod functions taking a 16-byte aligned
> struct containing an __int128. It checks that the BPF trampoline packs
> these arguments correctly according to the BPF calling convention.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> index 8086586d3e196..a2da219e14769 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_struct.c
> [ ... ]
>> @@ -141,6 +143,20 @@ static void test_int128_args(void)
>>   	ASSERT_EQ(skel->bss->t_c, 3, "t:c");
>>   	ASSERT_EQ(skel->bss->t_ret, 6, "t ret");
>>   
>> +	ASSERT_EQ(skel->bss->s_g, 7, "s:g");
>> +	ASSERT_EQ(skel->bss->s_h_lo, 8, "s:h.lo");
>> +	ASSERT_EQ(skel->bss->s_h_hi, 9, "s:h.hi");
> [Severity: Low]
> Does this code assume a little-endian memory layout for the __int128
> argument?
>
> The test passes a 16-byte struct containing ((__int128)9 << 64) | 8.
> On big-endian architectures like aarch64 (when configured with
> CONFIG_CPU_BIG_ENDIAN), the most significant 64 bits (9) are placed at the
> lowest address, and the least significant bits (8) at the higher address.
> This hardcoded expectation for the little-endian layout appears to cause the
> test_int128_args selftest to fail its assertions on big-endian systems.

Ok, will fix due to possible arm64 big endian.

>
>> +	ASSERT_EQ(skel->bss->s_i, 10, "s:i");
>> +	ASSERT_EQ(skel->bss->s_ret, 385, "s ret");
> [ ... ]
>> diff --git a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
>> index 4638dfec1f38e..5520ee500c01d 100644
>> --- a/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
>> +++ b/tools/testing/selftests/bpf/progs/tracing_struct_int128.c
> [ ... ]
>> @@ -15,4 +15,49 @@ int test_int128_arg_fexit(unsigned long long *ctx)
>>   	return 0;
>>   }
>>   
>> +/*
>> + * The arguments come packed in the BPF convention whatever holes the
>> + * kernel convention left between them: an eightbyte each, and two for the
>> + * struct holding an __int128.
>> + */
>> +long s_g, s_h_lo, s_h_hi, s_i, s_ret;
>> +
>> +SEC("fentry/bpf_testmod_test_int128_stack")
>> +int test_int128_stack_fentry(unsigned long long *ctx)
>> +{
>> +	s_g = ctx[6];
>> +	s_h_lo = ctx[7];
>> +	s_h_hi = ctx[8];
> [Severity: Low]
> Is this direct array indexing in test_int128_stack_fentry() subject to the
> same endianness assumption?
>
> When the BPF trampoline projects the struct onto the u64 ctx[] array on a
> big-endian system, ctx[7] receives 9 and ctx[8] receives 8, rather than the
> little-endian projection expected here.

Ok, will fix due to possible arm64 big endian.

>
>> +	s_i = ctx[9];
>> +	return 0;
>> +}