Re: [PATCH bpf-next v2 22/23] bpf, arm64: Add JIT support for stack arguments

Re: [PATCH bpf-next v2 22/23] bpf, arm64: Add JIT support for stack arguments

[Yonghong Song]

On 5/9/26 4:15 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] In `emit_stack_arg_load()`, `bpf_off` is not negated, causing a negative index that fails array bounds checks and results in the JIT embedding an invalid instruction (BRK), leading to a kernel panic.
> --
>
> commit 6ccf042549a3902757c6335313c905e99fb41292
> Author: Puranjay Mohan <puranjay@kernel.org>
>
> bpf, arm64: Add JIT support for stack arguments
>
> This commit implements stack argument passing for BPF-to-BPF and kfunc
> calls with more than 5 parameters on arm64, following the AAPCS64 calling
> convention.
>
>> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
>> index 085e650662e3d..9b9d2501d2d36 100644
>> --- a/arch/arm64/net/bpf_jit_comp.c
>> +++ b/arch/arm64/net/bpf_jit_comp.c
> [ ... ]
>
>> @@ -1191,6 +1207,41 @@ static int add_exception_handler(const struct bpf_insn *insn,
>>   	return 0;
>>   }
>>   
>> +static const u8 stack_arg_reg[] = { A64_R(5), A64_R(6), A64_R(7) };
>> +
>> +#define NR_STACK_ARG_REGS	ARRAY_SIZE(stack_arg_reg)
>> +
>> +static void emit_stack_arg_load(u8 dst, s16 bpf_off, struct jit_ctx *ctx)
>> +{
>> +	int idx = bpf_off / sizeof(u64) - 1;
> Does this missing negation for bpf_off lead to an invalid instruction being
> emitted?

For stack argument load, the offset is positive number and it is multiple of 8.
So it should be okay here.

>
> Since BPF stack arguments use negative offsets from BPF_REG_PARAMS (such as
> -8 for arg 6), the calculation bpf_off / sizeof(u64) - 1 will result in a
> negative idx.
>
> Because NR_STACK_ARG_REGS uses ARRAY_SIZE, it evaluates to an unsigned
> size_t. Comparing a negative idx to it will result in a false condition,
> falling through to the else block.
>
> In the else block, the negative idx generates a negative offset for
> A64_LDR64I. Since A64_LDR64I expects an unsigned 12-bit scaled immediate,
> could this cause the instruction encoder to fail and embed an
> AARCH64_BREAK_FAULT into the JIT image?
>
>> +
>> +	if (idx < NR_STACK_ARG_REGS)
>> +		emit(A64_MOV(1, dst, stack_arg_reg[idx]), ctx);
>> +	else
>> +		emit(A64_LDR64I(dst, A64_FP, (idx - NR_STACK_ARG_REGS) * sizeof(u64) + 16), ctx);
>> +}
>> +
>> +static void emit_stack_arg_store(u8 src_a64, s16 bpf_off, struct jit_ctx *ctx)
>> +{
>> +	int idx = -bpf_off / sizeof(u64) - 1;
> For comparison, emit_stack_arg_store() correctly negates bpf_off here.
>
>> +
>> +	if (idx < NR_STACK_ARG_REGS)