All of lore.kernel.org
 help / color / mirror / Atom feed
From: KaFai Wan <kafai.wan@linux.dev>
To: "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Eduard Zingerman" <eddyz87@gmail.com>,
	"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Song Liu" <song@kernel.org>,
	"Yonghong Song" <yonghong.song@linux.dev>,
	"Jiri Olsa" <jolsa@kernel.org>,
	"Emil Tsalapatis" <emil@etsalapatis.com>,
	"Ihor Solodrai" <ihor.solodrai@linux.dev>,
	"Björn Töpel" <bjorn@kernel.org>, "Pu Lehui" <pulehui@huawei.com>,
	"Puranjay Mohan" <puranjay@kernel.org>,
	"Paul Walmsley" <pjw@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	bpf@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf 1/1] bpf, riscv: Fix stack arguments of struct_ops trampolines
Date: Fri, 02 Oct 2026 09:13:45 +0800	[thread overview]
Message-ID: <5382c8744bac2a1d17fd96f76aa6a5245cf8a94e.camel@linux.dev> (raw)
In-Reply-To: <20261001154038.2265210-1-kafai.wan@linux.dev>

On Thu, 2026-10-01 at 23:40 +0800, KaFai Wan wrote:

I ran the tests yesterday and didn’t notice that Pu Lehui had already sent the same patch. Please
ignore this. Sorry for the noise.

> When a struct_ops trampoline takes more than 8 arguments (up to 12), the
> 9th and beyond are passed on the stack. store_args() copies them into the
> trampoline frame using a hard-coded FP + 16 base:
> 
> 	emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
> 
> That offset only holds for fentry trampolines, where the traced function's
> T0/FP are saved at FP - 8/FP - 16 and its stack arguments start at FP + 16.
> A struct_ops trampoline is called directly, so its stack arguments start at
> FP + 0 and store_args() reads the wrong words. BPF programs then see garbage
> for arg9 and beyond, which fails the selftest:
> 
>   struct_ops_multi_args/test_trampoline_stack_args:FAIL
> 
> Pass the stack argument base offset to store_args(): 0 for struct_ops
> trampolines, 16 otherwise.
> 
> Fixes: 6801b0aef79d ("riscv, bpf: Add 12-argument support for RV64 bpf trampoline")
> Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
> ---
>  arch/riscv/net/bpf_jit_comp64.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 01fe66774f02..4ae6674f58ed 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -875,7 +875,7 @@ 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_base, struct rv_jit_context
> *ctx)
>  {
>  	int i;
>  
> @@ -883,8 +883,7 @@ 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_base + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP,
> ctx);
>  			emit_sd(RV_REG_FP, -args_off, RV_REG_T1, ctx);
>  		}
>  		args_off -= 8;
> @@ -1179,7 +1178,8 @@ 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);
> +	/* skip slots for T0 and FP of traced function */
> +	store_args(nr_arg_slots, args_off, is_struct_ops ? 0 : 16, ctx);
>  
>  	if (bpf_fsession_cnt(tnodes)) {
>  		/* clear all session cookies' value */

-- 
Thanks,
KaFai

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: KaFai Wan <kafai.wan@linux.dev>
To: "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Eduard Zingerman" <eddyz87@gmail.com>,
	"Kumar Kartikeya Dwivedi" <memxor@gmail.com>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Song Liu" <song@kernel.org>,
	"Yonghong Song" <yonghong.song@linux.dev>,
	"Jiri Olsa" <jolsa@kernel.org>,
	"Emil Tsalapatis" <emil@etsalapatis.com>,
	"Ihor Solodrai" <ihor.solodrai@linux.dev>,
	"Björn Töpel" <bjorn@kernel.org>, "Pu Lehui" <pulehui@huawei.com>,
	"Puranjay Mohan" <puranjay@kernel.org>,
	"Paul Walmsley" <pjw@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	bpf@vger.kernel.org, linux-riscv@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf 1/1] bpf, riscv: Fix stack arguments of struct_ops trampolines
Date: Fri, 02 Oct 2026 09:13:45 +0800	[thread overview]
Message-ID: <5382c8744bac2a1d17fd96f76aa6a5245cf8a94e.camel@linux.dev> (raw)
In-Reply-To: <20261001154038.2265210-1-kafai.wan@linux.dev>

On Thu, 2026-10-01 at 23:40 +0800, KaFai Wan wrote:

I ran the tests yesterday and didn’t notice that Pu Lehui had already sent the same patch. Please
ignore this. Sorry for the noise.

> When a struct_ops trampoline takes more than 8 arguments (up to 12), the
> 9th and beyond are passed on the stack. store_args() copies them into the
> trampoline frame using a hard-coded FP + 16 base:
> 
> 	emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
> 
> That offset only holds for fentry trampolines, where the traced function's
> T0/FP are saved at FP - 8/FP - 16 and its stack arguments start at FP + 16.
> A struct_ops trampoline is called directly, so its stack arguments start at
> FP + 0 and store_args() reads the wrong words. BPF programs then see garbage
> for arg9 and beyond, which fails the selftest:
> 
>   struct_ops_multi_args/test_trampoline_stack_args:FAIL
> 
> Pass the stack argument base offset to store_args(): 0 for struct_ops
> trampolines, 16 otherwise.
> 
> Fixes: 6801b0aef79d ("riscv, bpf: Add 12-argument support for RV64 bpf trampoline")
> Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
> ---
>  arch/riscv/net/bpf_jit_comp64.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 01fe66774f02..4ae6674f58ed 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -875,7 +875,7 @@ 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_base, struct rv_jit_context
> *ctx)
>  {
>  	int i;
>  
> @@ -883,8 +883,7 @@ 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_base + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP,
> ctx);
>  			emit_sd(RV_REG_FP, -args_off, RV_REG_T1, ctx);
>  		}
>  		args_off -= 8;
> @@ -1179,7 +1178,8 @@ 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);
> +	/* skip slots for T0 and FP of traced function */
> +	store_args(nr_arg_slots, args_off, is_struct_ops ? 0 : 16, ctx);
>  
>  	if (bpf_fsession_cnt(tnodes)) {
>  		/* clear all session cookies' value */

-- 
Thanks,
KaFai

  reply	other threads:[~2026-10-02  1:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01 15:40 [PATCH bpf 1/1] bpf, riscv: Fix stack arguments of struct_ops trampolines KaFai Wan
2026-10-01 15:40 ` KaFai Wan
2026-10-02  1:13 ` KaFai Wan [this message]
2026-10-02  1:13   ` KaFai Wan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5382c8744bac2a1d17fd96f76aa6a5245cf8a94e.camel@linux.dev \
    --to=kafai.wan@linux.dev \
    --cc=alex@ghiti.fr \
    --cc=andrii@kernel.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=ast@kernel.org \
    --cc=bjorn@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=pulehui@huawei.com \
    --cc=puranjay@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.