Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] openrisc: missing NOKPROBE annotation on arch_rethook_trampoline() --
commit 20e73d01661b40600429433415de2e9c63032d8f Author: Sahil Siddiq <[email protected]> openrisc: Add Kretprobes This commit adds Kretprobes support for OpenRISC on top of the generic rethook framework, providing arch_rethook_prepare(), arch_rethook_trampoline_callback(), and the assembly return trampoline arch_rethook_trampoline. > diff --git a/arch/openrisc/kernel/rethook_trampoline.S > b/arch/openrisc/kernel/rethook_trampoline.S > new file mode 100644 > index 0000000000000..5e8fd5a652222 > --- /dev/null > +++ b/arch/openrisc/kernel/rethook_trampoline.S > @@ -0,0 +1,111 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Author: Sahil Siddiq <[email protected]> > + */ > + > +#include <linux/linkage.h> > + > +#include <asm/asm.h> [ ... ] > +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 > + l.nop > + > + /* use the result as the return-address */ > + l.addi r9,r11,0 > + > + restore_all_base_regs > + l.addi r1,r1,PT_SIZE > + > + l.jr r9 > + l.nop > +SYM_CODE_END(arch_rethook_trampoline) [Severity: High] Should arch_rethook_trampoline be marked with _ASM_NOKPROBE() here (or NOKPROBE_SYMBOL() in arch/openrisc/kernel/rethook.c)? Although arch/openrisc/kernel/rethook_trampoline.S includes <asm/asm.h> and arch/openrisc/kernel/rethook.c marks arch_rethook_trampoline_callback() and arch_rethook_prepare() with NOKPROBE_SYMBOL(), arch_rethook_trampoline is placed in .text without a kprobe blacklist annotation. Because arch_rethook_trampoline is not blacklisted, registering a kretprobe or kprobe on arch_rethook_trampoline is permitted. When a kretprobed function returns to arch_rethook_trampoline with r9 holding the address of arch_rethook_trampoline, a kretprobe on arch_rethook_trampoline triggers: pre_handler_kretprobe() rethook_hook() arch_rethook_prepare() rhn->ret_addr = regs->gpr9; which pushes a rethook_node with ret_addr set to arch_rethook_trampoline onto current->rethooks. When arch_rethook_trampoline_callback() then calls rethook_trampoline_handler(), __rethook_find_ret_addr() skips nodes whose ret_addr equals arch_rethook_trampoline: kernel/trace/rethook.c:__rethook_find_ret_addr() { ... while (node) { rh = container_of(node, struct rethook_node, llist); if (rh->ret_addr != (unsigned long)arch_rethook_trampoline) { *cur = node; return rh->ret_addr; } node = node->next; } return 0; } Can this corrupt the current->rethooks shadow stack or trigger the BUG_ON(1) in rethook_trampoline_handler() when a probe is attached to arch_rethook_trampoline? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
