From mboxrd@z Thu Jan 1 00:00:00 1970 From: Masami Hiramatsu Date: Sun, 21 Mar 2021 12:54:36 +0000 Subject: Re: [PATCH -tip v3 08/11] kprobes: Setup instruction pointer in __kretprobe_trampoline_handler Message-Id: <20210321215436.2e0447bf4fc43471bcecc243@kernel.org> List-Id: References: <161615650355.306069.17260992641363840330.stgit@devnote2> <161615659174.306069.12736134222759644948.stgit@devnote2> In-Reply-To: <161615659174.306069.12736134222759644948.stgit@devnote2> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Masami Hiramatsu Cc: Steven Rostedt , Ingo Molnar , X86 ML , Daniel Xu , linux-kernel@vger.kernel.org, bpf@vger.kernel.org, kuba@kernel.org, mingo@redhat.com, ast@kernel.org, tglx@linutronix.de, kernel-team@fb.com, yhs@fb.com, Josh Poimboeuf , linux-ia64@vger.kernel.org On Fri, 19 Mar 2021 21:23:12 +0900 Masami Hiramatsu wrote: > To simplify the stacktrace with pt_regs from kretprobe handler, > set the correct return address to the instruction pointer in > the pt_regs before calling kretprobe handlers. > Oops, now I also find this breaks kretprobe for arm. It seems not enough registers stores. I need to fix the arm kprobe code too. In arch/arm/include/uapi/asm/ptrace.h, --- #define ARM_pc uregs[15] --- And in arch/arm/probes/kprobes/core.c, --- /* * When a retprobed function returns, trampoline_handler() is called, * calling the kretprobe's handler. We construct a struct pt_regs to * give a view of registers r0-r11 to the user return-handler. This is * not a complete pt_regs structure, but that should be plenty sufficient * for kretprobe handlers which should normally be interested in r0 only * anyway. */ void __naked __kprobes kretprobe_trampoline(void) { __asm__ __volatile__ ( "stmdb sp!, {r0 - r11} \n\t" --- So, changing regs->ARM_pc will break a stack entry. I need to expand it to r15. Thanks, > Suggested-by: Josh Poimboeuf > Signed-off-by: Masami Hiramatsu > --- > Changes in v3: > - Cast the correct_ret_addr to unsigned long. > --- > kernel/kprobes.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index cf19edc038e4..4ce3e6f5d28d 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1914,6 +1914,9 @@ unsigned long __kretprobe_trampoline_handler(struct pt_regs *regs, > BUG_ON(1); > } > > + /* Set the instruction pointer to the correct address */ > + instruction_pointer_set(regs, (unsigned long)correct_ret_addr); > + > /* Run them. */ > first = current->kretprobe_instances.first; > while (first) { > -- Masami Hiramatsu