linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc: Fix stack protector crashes on CPU hotplug
@ 2018-10-19  5:59 Michael Ellerman
  2018-10-19  6:20 ` Christophe LEROY
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Michael Ellerman @ 2018-10-19  5:59 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: abdhalee, joel

Recently in commit 7241d26e8175 ("powerpc/64: properly initialise
the stackprotector canary on SMP.") we fixed a crash with stack
protector on SMP by initialising the stack canary in
cpu_idle_thread_init().

But this can also causes crashes, when a CPU comes back online after
being offline:

  Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: pnv_smp_cpu_kill_self+0x2a0/0x2b0
  CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.19.0-rc3-gcc-7.3.1-00168-g4ffe713b7587 #94
  Call Trace:
    dump_stack+0xb0/0xf4 (unreliable)
    panic+0x144/0x328
    __stack_chk_fail+0x2c/0x30
    pnv_smp_cpu_kill_self+0x2a0/0x2b0
    cpu_die+0x48/0x70
    arch_cpu_idle_dead+0x20/0x40
    do_idle+0x274/0x390
    cpu_startup_entry+0x38/0x50
    start_secondary+0x5e4/0x600
    start_secondary_prolog+0x10/0x14

Looking at the stack we see that the canary value in the stack frame
doesn't match the canary in the task/paca. That is because we have
reinitialised the task/paca value, but then the CPU coming online has
returned into a function using the old canary value. That causes the
comparison to fail.

Instead we can call boot_init_stack_canary() from start_secondary()
which never returns. This is essentially what the generic code does in
cpu_startup_entry() under #ifdef X86, we should make that non-x86
specific in a future patch.

Fixes: 7241d26e8175 ("powerpc/64: properly initialise the stackprotector canary on SMP.")
Reported-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/kernel/smp.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 8e3a5da24d59..951c476faffc 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -61,6 +61,7 @@
 #include <asm/asm-prototypes.h>
 #include <asm/cpu_has_feature.h>
 #include <asm/ftrace.h>
+#include <asm/stackprotector.h>
 
 #ifdef DEBUG
 #include <asm/udbg.h>
@@ -1014,16 +1015,9 @@ static void cpu_idle_thread_init(unsigned int cpu, struct task_struct *idle)
 {
 	struct thread_info *ti = task_thread_info(idle);
 
-#ifdef CONFIG_STACKPROTECTOR
-	idle->stack_canary = get_random_canary();
-#endif
-
 #ifdef CONFIG_PPC64
 	paca_ptrs[cpu]->__current = idle;
 	paca_ptrs[cpu]->kstack = (unsigned long)ti + THREAD_SIZE - STACK_FRAME_OVERHEAD;
-#ifdef CONFIG_STACKPROTECTOR
-	paca_ptrs[cpu]->canary = idle->stack_canary;
-#endif
 #endif
 	ti->cpu = cpu;
 	secondary_ti = current_set[cpu] = ti;
@@ -1316,6 +1310,8 @@ void start_secondary(void *unused)
 	notify_cpu_starting(cpu);
 	set_cpu_online(cpu, true);
 
+	boot_init_stack_canary();
+
 	local_irq_enable();
 
 	/* We can enable ftrace for secondary cpus now */
-- 
2.17.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-10-22 13:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-19  5:59 [PATCH] powerpc: Fix stack protector crashes on CPU hotplug Michael Ellerman
2018-10-19  6:20 ` Christophe LEROY
2018-10-21  8:37 ` Michael Ellerman
2018-10-22  9:40 ` Michael Ellerman
2018-10-22 13:40 ` [PATCH] " Abdul Haleem

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).