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

[sashiko-bot@kernel.org]

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?

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)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507212942.1122000-1-yonghong.song@linux.dev?part=22