Re: [PATCH bpf-next v7 3/3] selftests/bpf: Add test for large offset bpf-to-bpf call

On 4/22/26 5:02 AM, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [Medium] The selftest causes a kernel panic on unfixed kernels (when JIT is disabled), which disrupts CI pipelines and bisection workflows.
> --
> 
> commit c0593c5f6291ee1a846f9c17858890ded3cd8ea4
> Author: Yazhou Tang <[email protected]>
> 
> selftests/bpf: Add test for large offset bpf-to-bpf call
> 
> This commit adds a BPF selftest to verify that the verifier and JIT correctly
> handle bpf-to-bpf calls with relative jump offsets exceeding the s16 boundary.
> It uses a large inline assembly block to force the verifier to process a jump
> exceeding the standard limit.
> 
>> - Before the fix, the verifier rewrites the call instruction with a truncated
>>    offset (here 32768 -> -32768) and lets it pass. When the program is executed,
>>    the call instruction causes a kernel panic due to an invalid jump target.
> 
> Is it safe to introduce a test that causes a kernel panic on unpatched kernels?
> 
> While reproducing the bug is the test's goal, crashing the kernel is highly
> disruptive for CI systems and git bisect workflows that run the testsuite on
> older or unpatched kernel trees.

This point is valid. Although the upstream CI typically runs with JIT enabled,
causing a kernel panic when JIT is disabled is definitely something we should avoid.

>> diff --git a/tools/testing/selftests/bpf/progs/call_large_imm.c b/tools/testing/selftests/bpf/progs/call_large_imm.c
>> new file mode 100644
>> index 0000000000000..7cdf1e6d80d8d
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/call_large_imm.c
> 
> [ ... ]
> 
>> +static __attribute__((noinline)) void padding_subprog(void)
>> +{
>> +	asm volatile ("					\
>> +		r0 = 0;					\
>> +		.rept 32765;				\
>> +		r0 += 0;				\
>> +		.endr;					\
>> +	" ::: "r0");
>> +}
>> +
>> +static __attribute__((noinline)) int target_subprog(void)
>> +{
>> +	/* A volatile variable is used here to prevent optimization. */
>> +	volatile int magic_ret = 3;
>> +	return magic_ret;
>> +}
>> +
>> +SEC("syscall")
>> +int call_large_imm_test(void *ctx)
>> +{
>> +	if (zero)
>> +		padding_subprog();
>> +	return target_subprog();
>> +}
> 
> Could the test be redesigned to fail gracefully?
> 
> If the large padding block were placed before the call instruction, the
> call site would be at a large positive offset. A truncated negative jump
> (-32768) would then land safely inside the valid padding instructions instead
> of jumping out of bounds of the program.
> 
> The execution would then fall through and return an incorrect value, causing
> ASSERT_EQ(opts.retval, 3) in the runner to cleanly fail without bringing down
> the system.
> 

This approach is indeed more elegant and safer. I will redesign the selftest
as suggested for next version.