Re: [PATCH v2 4/4] openrisc: Add Kretprobes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] openrisc: kretprobe trampoline leaves regs->sp and regs->sr uninitialized
--

commit a5e8b19fd4705325db24ad7195d2d69f741fc493
Author: Sahil Siddiq <sahilcdq0@gmail.com>

openrisc: Add Kretprobes

This commit adds Kretprobes support for OpenRISC by building on top of the
generic rethook framework. It implements the necessary trampoline routines to
intercept function returns and route them through the kretprobe handler.

> diff --git a/arch/openrisc/kernel/rethook_trampoline.S b/arch/openrisc/kernel/rethook_trampoline.S
> new file mode 100644
> index 0000000000000..ac4ca0ecc6694
> --- /dev/null
> +++ b/arch/openrisc/kernel/rethook_trampoline.S
[ ... ]
> +	.text
> +
> +	.macro save_all_base_regs
> +	l.sw	PT_GPR2(r1),r2
> +	l.sw	PT_GPR3(r1),r3

[Severity: High]
Does this macro need to save the stack pointer (r1) to the PT_SP offset and
the status register to the PT_SR offset? It appears they are omitted from the
base register save list.

[ ... ]
> +SYM_CODE_START(arch_rethook_trampoline)
> +	/* Allocate enough space on stack for pt_regs */
> +	l.addi	r1,r1,-PT_SIZE
> +	save_all_base_regs
> +
> +	l.addi	r3,r1,0	/* pt_regs */
> +	l.jal	arch_rethook_trampoline_callback

[Severity: High]
Since save_all_base_regs skips saving r1 (sp) and sr, will the pt_regs
passed into arch_rethook_trampoline_callback contain uninitialized stack
garbage for regs->sp and regs->sr?

Tracing handlers like BPF or ftrace rely on functions like
kernel_stack_pointer() and user_mode() which read these fields. Could these
uninitialized fields lead to broken stack unwinding or incorrect privilege
evaluation when the kretprobe handlers run?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930203129.51048-1-sahilcdq0@gmail.com?part=4