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

On 4/29/26 20:49, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Missing compiler barrier in the inline assembly landing pad allows LLVM to hoist the 'call_happened' load, leading to an infinite recursion loop and kernel panic.
> --
> 
> commit 7a012e4a738e7d37b40994fe21bcf400bb1dd252
> Author: Yazhou Tang <[email protected]>
> 
> selftests/bpf: Add test for large offset bpf-to-bpf call
> 
> This commit adds a selftest to verify verifier and JIT behavior when handling
> bpf-to-bpf calls with relative jump offsets exceeding the s16 boundary. It uses
> an inline assembly block with a .rept directive to generate a large dummy
> subprogram, forcing the verifier to process a jump exceeding the s16 range.
> 
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_call_large_imm.c b/tools/testing/selftests/bpf/progs/verifier_call_large_imm.c
>> new file mode 100644
>> index 000000000000..da52a1c16d05
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/verifier_call_large_imm.c
> 
> [ ... ]
> 
>> +SEC("syscall")
>> +__success __retval(3)
>> +int call_large_imm_test(void *ctx)
>> +{
>> +	/*
>> +	 * Landing pad to handle call error on kernel without the fix,
>> +	 * preventing kernel panic.
>> +	 */
>> +	asm volatile (
>> +	"r0 = 0;"
>> +	".rept 32768;"
>> +	"r0 += 0;"
>> +	".endr;"
>> +	::: "r0");
>> +
>> +	/*
>> +	 * The call_happened variable is 1 only when the call insn wrongly
>> +	 * go back to the landing pad above.
>> +	 */
>> +	if (call_happened == 1) {
> 
> Could this lack of a memory clobber in the inline assembly allow the compiler
> to hoist the load of call_happened to before the inline assembly block?
> 
> Because call_happened is a plain global variable and the asm volatile block
> does not include a "memory" clobber, LLVM is structurally permitted to optimize
> the code by hoisting the load.
> 
> If the load is hoisted and a truncated backwards jump lands inside the
> assembly block on an unpatched kernel, the if condition would evaluate a stale
> register value (0) instead of re-reading memory.
> 
> Will this bypass the termination check, re-execute the target_subprog call,
> and create an infinite recursion loop that overflows the kernel stack?

Yes. I should use __clobber_all in the inline asm.

I will send v9 asap.

> 
>> +		/* A volatile variable is used here to prevent optimization. */
>> +		volatile int flag = -1;
>> +		return flag;
>> +	}
>> +
>> +	call_happened = 1;
>> +
>> +	padding_subprog();
>> +
>> +	return target_subprog();
>> +}
>