Re: [PATCH bpf-next 1/7] bpf, arm64: Fix stack-passed arguments for indirect trampolines
Xu Kuohai <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 8/11/2026 3:09 AM, Puranjay Mohan wrote:
> save_args() reads stack-passed arguments relative to FP assuming the
> trampoline is entered through the fentry call from a traced function, in
> which case both the parent frame (FP/x9) and the traced function frame
> (FP/LR) are saved before FP is set, so the arguments start at FP + 32.
>
> An indirect trampoline for a struct_ops callback is entered through a
> function pointer (blr), so only the FP/LR frame is pushed and the
> arguments start at FP + 16, not FP + 32. Every stack-passed argument of
> a struct_ops callback with more than eight argument slots is read two
> slots off.
>
> This went unnoticed because no struct_ops member passed arguments on the
> stack until bpf_testmod_ops3::test_arena_stack, added by
> commit 2d4de9a493a0 ("selftests/bpf: Test stack-passed struct_ops arena arguments").
> That member covers this on arm64 once the JIT gains arena argument
> support later in this series. Pass is_struct_ops into save_args() and
> pick the offset accordingly, mirroring the x86 fix.
>
> Fixes: 9014cf56f13d ("bpf, arm64: Support up to 12 function arguments")
> Signed-off-by: Puranjay Mohan <[email protected]>
> ---
> arch/arm64/net/bpf_jit_comp.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index d14d297ebb967..4af5a98b84e19 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c
> @@ -2509,9 +2509,8 @@ static void clear_garbage(struct jit_ctx *ctx, int reg, int effective_bytes)
> }
>
> static void save_args(struct jit_ctx *ctx, int bargs_off, int oargs_off,
> - const struct btf_func_model *m,
> - const struct arg_aux *a,
> - bool for_call_origin)
> + const struct btf_func_model *m, const struct arg_aux *a,
> + bool for_call_origin, bool is_struct_ops)
> {
> int i;
> int reg;
> @@ -2531,7 +2530,15 @@ static void save_args(struct jit_ctx *ctx, int bargs_off, int oargs_off,
> bargs_off += 8;
> }
>
> - soff = 32; /* on stack arguments start from FP + 32 */
> + /*
> + * On-stack arguments start above the frame(s) pushed by the trampoline
> + * prologue. Entered through the fentry call from a traced function, the
> + * prologue saves both the parent (FP/x9) and the traced function
> + * (FP/LR) frames, so the arguments start at FP + 32. A struct_ops
> + * callback is called indirectly and only the FP/LR frame is saved, so
> + * they start at FP + 16.
> + */
> + soff = is_struct_ops ? 16 : 32;
> doff = (for_call_origin ? oargs_off : bargs_off);
>
> /* save on stack arguments */
> @@ -2721,7 +2728,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
> store_func_meta(ctx, func_meta, func_meta_off);
>
> /* save args for bpf */
> - save_args(ctx, bargs_off, oargs_off, m, a, false);
> + save_args(ctx, bargs_off, oargs_off, m, a, false, is_struct_ops);
>
> /* save callee saved registers */
> emit(A64_STR64I(A64_R(19), A64_SP, regs_off), ctx);
> @@ -2770,7 +2777,7 @@ static int prepare_trampoline(struct jit_ctx *ctx, struct bpf_tramp_image *im,
>
> if (flags & BPF_TRAMP_F_CALL_ORIG) {
> /* save args for original func */
> - save_args(ctx, bargs_off, oargs_off, m, a, true);
> + save_args(ctx, bargs_off, oargs_off, m, a, true, is_struct_ops);
> /* call original func */
> emit(A64_LDR64I(A64_R(10), A64_SP, retaddr_off), ctx);
> emit(A64_ADR(A64_LR, AARCH64_INSN_SIZE * 2), ctx);
Reviewed-by: Xu Kuohai <[email protected]>