Re: [PATCH bpf-next v1 02/14] bpf, riscv: Fix stack-passed arguments for indirect trampolines

On 2026/8/22 7:34, Kumar Kartikeya Dwivedi wrote:
> store_args() reads stack-passed arguments relative to FP assuming the
> trampoline was entered through the fentry call from a traced function. In
> that path, the trampoline pushes the parent frame before establishing its
> final FP, so the incoming stack arguments start at FP + 16.
> 
> An indirect trampoline for a struct_ops callback is called through a
> function pointer. Its prologue allocates only the trampoline frame and sets
> FP to the incoming SP. The RISC-V ABI places the first stack argument at
> that incoming SP, so the arguments start at FP, not FP + 16. Every
> stack-passed argument of a callback with more than eight argument slots is
> therefore read two slots late.
> 
> Pass the prologue-dependent offset to store_args(), using zero for a direct
> struct_ops trampoline and 16 for the fentry path.
> 
> Fixes: 6801b0aef79d ("riscv, bpf: Add 12-argument support for RV64 bpf trampoline")
> Cc: Björn Töpel <[email protected]>
> Cc: Pu Lehui <[email protected]>
> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
> ---
>   arch/riscv/net/bpf_jit_comp64.c | 14 ++++++++++----
>   1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 47c7bf431ba8..b1084f30f3ce 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -857,7 +857,8 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type old_t,
>   	return ret;
>   }
>   
> -static void store_args(int nr_arg_slots, int args_off, struct rv_jit_context *ctx)
> +static void store_args(int nr_arg_slots, int args_off, int stack_args_off,
> +		       struct rv_jit_context *ctx)
>   {
>   	int i;
>   
> @@ -865,8 +866,8 @@ static void store_args(int nr_arg_slots, int args_off, struct rv_jit_context *ct
>   		if (i < RV_MAX_REG_ARGS) {
>   			emit_sd(RV_REG_FP, -args_off, RV_REG_A0 + i, ctx);
>   		} else {
> -			/* skip slots for T0 and FP of traced function */
> -			emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
> +			emit_ld(RV_REG_T1, stack_args_off +
> +				(i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
>   			emit_sd(RV_REG_FP, -args_off, RV_REG_T1, ctx);
>   		}
>   		args_off -= 8;
> @@ -1152,7 +1153,12 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
>   	func_meta = nr_arg_slots;
>   	emit_store_stack_imm64(RV_REG_T1, -func_meta_off, func_meta, ctx);
>   
> -	store_args(nr_arg_slots, args_off, ctx);
> +	/*
> +	 * A direct struct_ops call has its first stack argument at the incoming
> +	 * SP, which the trampoline keeps as FP. The fentry path pushes the
> +	 * parent frame first, so its incoming stack arguments start at FP + 16.
> +	 */
> +	store_args(nr_arg_slots, args_off, is_struct_ops ? 0 : 16, ctx);
>   
>   	if (bpf_fsession_cnt(tnodes)) {
>   		/* clear all session cookies' value */

Reviewed-by: Pu Lehui <[email protected]>