Re: [PATCH] riscv: probes: save original sp in rethook trampoline

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Martin Kaiser <martin@kaiser.cx>
Cc: Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: probes: save original sp in rethook trampoline
Date: Wed, 1 Jul 2026 07:33:35 +0900	[thread overview]
Message-ID: <20260701073335.548d8f0b435b1a5fb4e41a69@kernel.org> (raw)
In-Reply-To: <20260630194010.1824039-1-martin@kaiser.cx>

On Tue, 30 Jun 2026 21:40:03 +0200
Martin Kaiser <martin@kaiser.cx> wrote:

> Reading a word from the stack in a kretprobe crashes a risc-v kernel.
> 
> $ cd /sys/kernel/tracing/
> $ echo 'r n_tty_write $stack0' > dynamic_events
> $ echo 1 > events/kprobes/enable
> Unable to handle kernel paging request at virtual address 0000000200000128
> ...
> [<ffffffff80016d16>] regs_get_kernel_stack_nth+0x26/0x38
> [<ffffffff80177196>] process_fetch_insn+0x3ee/0x760
> [<ffffffff80177836>] kretprobe_trace_func+0x116/0x1f0
> [<ffffffff8017795a>] kretprobe_dispatcher+0x4a/0x58
> [<ffffffff8013572e>] kretprobe_rethook_handler+0x5e/0x90
> [<ffffffff80180838>] rethook_trampoline_handler+0x70/0x108
> [<ffffffff8001ba32>] arch_rethook_trampoline_callback+0x12/0x1c
> [<ffffffff8001ba84>] arch_rethook_trampoline+0x48/0x94
> [<ffffffff8067872a>] tty_write+0x1a/0x30
> 
> In regs_get_kernel_stack_nth, regs->sp contains an arbitrary value.
> 
> arch_rethook_trampoline saves the registers from the probed function in a
> struct pt_regs. sp is not saved. Instead, sp is decremented for
> arch_rethook_trampoline's local stack.
> 
> Fix this crash and save the original sp along with the other registers.
> Use a0 as a temporary register, it is overwritten anyway.

Good catch!

Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

I would like this to be handled by the RISC-V maintainers.

Thank you,

> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  arch/riscv/kernel/probes/rethook_trampoline.S | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/riscv/kernel/probes/rethook_trampoline.S b/arch/riscv/kernel/probes/rethook_trampoline.S
> index f2cd83d9b0f0..c3aa8d8cf5af 100644
> --- a/arch/riscv/kernel/probes/rethook_trampoline.S
> +++ b/arch/riscv/kernel/probes/rethook_trampoline.S
> @@ -41,6 +41,9 @@
>  	REG_S x29, PT_T4(sp)
>  	REG_S x30, PT_T5(sp)
>  	REG_S x31, PT_T6(sp)
> +	/* save original sp */
> +	addi a0, sp, PT_SIZE_ON_STACK
> +	REG_S a0, PT_SP(sp)
>  	.endm
>  
>  	.macro restore_all_base_regs
> -- 
> 2.43.7
> 

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>