lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <YWtBDXJQRLN2T1c6@shell.armlinux.org.uk> Date: Sat, 16 Oct 2021 22:15:57 +0100 From: "Russell King (Oracle)" <linux@...linux.org.uk> To: Masami Hiramatsu <mhiramat@...nel.org> Cc: Steven Rostedt <rostedt@...dmis.org>, "Naveen N . Rao" <naveen.n.rao@...ux.vnet.ibm.com>, Ananth N Mavinakayanahalli <ananth@...ux.ibm.com>, Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org, Sven Schnelle <svens@...ux.ibm.com>, Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, linux-arm-kernel@...ts.infradead.org Subject: Re: [PATCH 08/10] ARM: kprobes: Make a frame pointer on __kretprobe_trampoline On Fri, Oct 15, 2021 at 09:51:56PM +0900, Masami Hiramatsu wrote: > Currently kretprobe on ARM just fills r0-r11 of pt_regs, but > that is not enough for the stacktrace. Moreover, from the user > kretprobe handler, stacktrace needs a frame pointer on the > __kretprobe_trampoline. > > This adds a frame pointer on __kretprobe_trampoline for both gcc > and clang case. Those have different frame pointer so we need > different but similar stack on pt_regs. > > Gcc makes the frame pointer (fp) to point the 'pc' address of > the {fp, ip (=sp), lr, pc}, this means {r11, r13, r14, r15}. > Thus if we save the r11 (fp) on pt_regs->r12, we can make this > set on the end of pt_regs. > > On the other hand, Clang makes the frame pointer to point the > 'fp' address of {fp, lr} on stack. Since the next to the > pt_regs->lr is pt_regs->sp, I reused the pair of pt_regs->fp > and pt_regs->ip. > So this stores the 'lr' on pt_regs->ip and make the fp to point > pt_regs->fp. > > For both cases, saves __kretprobe_trampoline address to > pt_regs->lr, so that the stack tracer can identify this frame > pointer has been made by the __kretprobe_trampoline. > > Note that if the CONFIG_FRAME_POINTER is not set, this keeps > fp as is. > > Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org> > Reviewed-by: Nick Desaulniers <ndesaulniers@...gle.com> > --- > arch/arm/probes/kprobes/core.c | 29 ++++++++++++++++++++++++----- > 1 file changed, 24 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c > index 95f23b47ba27..7cbd65a22769 100644 > --- a/arch/arm/probes/kprobes/core.c > +++ b/arch/arm/probes/kprobes/core.c > @@ -368,16 +368,35 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, > /* > * 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. > + * give a view of registers r0-r11, sp, lr, and pc to the user > + * return-handler. This is not a complete pt_regs structure, but that > + * should be enough for stacktrace from the return handler with or > + * without pt_regs. > */ > void __naked __kprobes __kretprobe_trampoline(void) > { > __asm__ __volatile__ ( > - "sub sp, sp, #16 \n\t" > + "ldr lr, =__kretprobe_trampoline \n\t" > + "stmdb sp!, {sp, lr, pc} \n\t" I think you really do not want to do that. >From DDI0406C: "ARM deprecates the use of instructions with the base register in the list and ! specified. If the base register is not the lowest-numbered register in the list, such an instruction stores an UNKNOWN value for the base register." However, it doesn't say what value is stored if the base register is the lowest-numbered register in the list. The pseudocode given shows that it is the original value. However, DDI0100E: "Operand restrictions If <Rn> is specified as <registers> and base register writeback is specified: • If <Rn> is the lowest-numbered register specified in <register_list>, the original value of <Rn> is stored. • Otherwise, the stored value of <Rn> is UNPREDICTABLE." So I guess it might be okay... but it seems a bit dodgy to rely on this behaviour. > +#ifdef CONFIG_FRAME_POINTER > + /* __kretprobe_trampoline makes a framepointer on pt_regs. */ > +#ifdef CONFIG_CC_IS_CLANG > + /* In clang case, pt_regs->ip = lr. */ > + "stmdb sp!, {lr} \n\t" > "stmdb sp!, {r0 - r11} \n\t" This can be simplified to: "stmdb sp!, {r0 - r11, lr} \n\t" Also, note the value we store for "fp" is __kretprobe_trampoline. > + /* fp points regs->r11 (fp) */ > + "add fp, sp, #44 \n\t" > +#else /* !CONFIG_CC_IS_CLANG */ > + /* In gcc case, pt_regs->ip = fp. */ > + "stmdb sp!, {fp} \n\t" > + "stmdb sp!, {r0 - r11} \n\t" This can be simplified to: "stmdb sp!, {r0 - r12} \n\t" since fp is r12. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists