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
| ||
|
Date: Fri, 24 Jan 2020 16:46:24 +1100 From: Michael Ellerman <mpe@...erman.id.au> To: Christophe Leroy <christophe.leroy@....fr>, Benjamin Herrenschmidt <benh@...nel.crashing.org>, Paul Mackerras <paulus@...ba.org> Cc: linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org, Segher Boessenkool <segher@...nel.crashing.org> Subject: Re: [PATCH 1/2] powerpc/irq: don't use current_stack_pointer() in check_stack_overflow() Christophe Leroy <christophe.leroy@....fr> writes: > current_stack_pointer() doesn't return the stack pointer, but the > caller's stack frame. See commit bfe9a2cfe91a ("powerpc: Reimplement > __get_SP() as a function not a define") and commit acf620ecf56c > ("powerpc: Rename __get_SP() to current_stack_pointer()") for details. > > The purpose of check_stack_overflow() is to verify that the stack has > not overflowed. > > To really know whether the stack pointer is still within boundaries, > the check must be done directly on the value of r1. > > Signed-off-by: Christophe Leroy <christophe.leroy@....fr> > --- > arch/powerpc/kernel/irq.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index bb34005ff9d2..4d468d835558 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -599,9 +599,8 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > static inline void check_stack_overflow(void) > { > #ifdef CONFIG_DEBUG_STACKOVERFLOW > - long sp; > - > - sp = current_stack_pointer() & (THREAD_SIZE-1); > + register unsigned long r1 asm("r1"); > + long sp = r1 & (THREAD_SIZE - 1); This appears to work but seems to be "unsupported" by GCC, and clang actually complains about it: /linux/arch/powerpc/kernel/irq.c:603:12: error: variable 'r1' is uninitialized when used here [-Werror,-Wuninitialized] long sp = r1 & (THREAD_SIZE - 1); ^~ The GCC docs say: The only supported use for this feature is to specify registers for input and output operands when calling Extended asm (see Extended Asm). https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Local-Register-Variables.html#Local-Register-Variables If I do this it seems to work, but feels a little dicey: asm ("" : "=r" (r1)); sp = r1 & (THREAD_SIZE - 1); Generated code looks OK ish: clang: sp = r1 & (THREAD_SIZE - 1); 22e0: a0 04 24 78 clrldi r4,r1,50 if (unlikely(sp < 2048)) { 22e4: ff 07 24 28 cmpldi r4,2047 22e8: 58 00 81 40 ble 2340 <do_IRQ+0xe0> gcc: if (unlikely(sp < 2048)) { 2eb4: 00 38 28 70 andi. r8,r1,14336 ... 2ecc: 24 00 82 40 bne c000000000002ef0 <do_IRQ+0xa0> cheers
Powered by blists - more mailing lists