From: Michael Ellerman Date: Fri, 19 Oct 2018 05:19:10 +0000 (+1100) Subject: powerpc: Fix stack protector crashes on CPU hotplug X-Git-Tag: v4.20-rc1~24^2~21 X-Git-Url: https://git.kernel.dk/?a=commitdiff_plain;h=b6aeddea74b08518289fc86545297cf18a0b53a7;p=linux-2.6-block.git powerpc: Fix stack protector crashes on CPU hotplug 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 Signed-off-by: Michael Ellerman Reviewed-by: Christophe Leroy --- diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c index 8e3a5da24d59..3f15edf25a0d 100644 --- a/arch/powerpc/kernel/smp.c +++ b/arch/powerpc/kernel/smp.c @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -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 */