Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752324AbdL0Tsy (ORCPT ); Wed, 27 Dec 2017 14:48:54 -0500 Received: from mail-pg0-f53.google.com ([74.125.83.53]:33157 "EHLO mail-pg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751203AbdL0Tsx (ORCPT ); Wed, 27 Dec 2017 14:48:53 -0500 X-Google-Smtp-Source: ACJfBosmIWujqOhxBrwP3PmA84GyPb7Co8jI9oHS4CeOzp/xCFQyjelkqzKs6XgVSuGSLHDQF8AkVA== Date: Wed, 27 Dec 2017 11:48:50 -0800 (PST) From: Linus Torvalds X-X-Sender: torvalds@i7.lan To: Alexandru Chirvasitu , Ingo Molnar , Thomas Gleixner cc: Andy Lutomirski , kernel list , Borislav Petkov , Brian Gerst , Denys Vlasenko , "H. Peter Anvin" , Josh Poimboeuf , Peter Zijlstra , Steven Rostedt Subject: [PATCH] x86-32: fix kexec with stack canary (CONFIG_CC_STACKPROTECTOR) Message-ID: User-Agent: Alpine 2.21 (LFD 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3921 Lines: 95 From: Linus Torvalds Date: Wed, 27 Dec 2017 11:41:30 -0800 Subject: [PATCH] x86-32: fix kexec with stack canary (CONFIG_CC_STACKPROTECTOR) Commit e802a51ede91 ("x86/idt: Consolidate IDT invalidation") cleaned up and unified the IDT invalidation that existed in a couple of places. It changed no actual real code. Despite not changing any actual real code, it _did_ change code generation: by implementing the common idt_invalidate() function in archx86/kernel/idt.c, it made the use of the function in arch/x86/kernel/machine_kexec_32.c be a real function call rather than an (accidental) inlining of the function. That, in turn, exposed two issues: - in load_segments(), we had incorrectly reset all the segment registers, which then made the stack canary load (which gcc does using offset of %gs) cause a trap. Instead of %gs pointing to the stack canary, it will be the normal zero-based kernel segment, and the stack canary load will take a page fault at address 0x14. - to make this even harder to debug, we had invalidated the GDT just before calling idt_invalidate(), which meant that the fault happened with an invalid GDT, which in turn causes a triple fault and immediate reboot. Fix this by (a) not reloading the special segments in load_segments(). We currently don't do any percpu accesses (which would require %fs on x86-32) in this area, but there's no reason to think that we might not want to do them, and like %gs, it's pointless to break it. (b) doing idt_invalidate() before invalidating the GDT, to keep things at least _slightly_ more debuggable for a bit longer. Without a IDT, traps will not work. Without a GDT, traps also will not work, but neither will any segment loads etc. So in a very real sense, the GDT is even more core than the IDT. Reported-and-tested-by: Alexandru Chirvasitu Fixes: e802a51ede91 ("x86/idt: Consolidate IDT invalidation") Cc: Thomas Gleixner Cc: Andy Lutomirski Cc: Peter Anvin Cc: Ingo Molnar Signed-off-by: Linus Torvalds --- I wrote "Reported-and-tested-by: Alexandru" because while this isn't exactly the same patch as anything Alexandru tested, it's pretty close, and I'm pretty sure this version will fix his issues too. I decided to try to just do the minimal changes: the GDT invalidation last (because of the debugging) and _only_ removing the resetting of fs/gs rather than removing load_segments() entirely. I think making idt_invalidate() be inline would be a good thing as well, and I do think that all those "phys_to_virt(0)" things are garbage, but I also think they are independent issues, so I didn't touch any of that. I'm assuming I'll get this patch back through the x86 tree, and will not be applying it to my own git tree unless the x86 people ask me to. Comments? arch/x86/kernel/machine_kexec_32.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/x86/kernel/machine_kexec_32.c b/arch/x86/kernel/machine_kexec_32.c index 00bc751c861c..edfede768688 100644 --- a/arch/x86/kernel/machine_kexec_32.c +++ b/arch/x86/kernel/machine_kexec_32.c @@ -48,8 +48,6 @@ static void load_segments(void) "\tmovl $"STR(__KERNEL_DS)",%%eax\n" "\tmovl %%eax,%%ds\n" "\tmovl %%eax,%%es\n" - "\tmovl %%eax,%%fs\n" - "\tmovl %%eax,%%gs\n" "\tmovl %%eax,%%ss\n" : : : "eax", "memory"); #undef STR @@ -232,8 +230,8 @@ void machine_kexec(struct kimage *image) * The gdt & idt are now invalid. * If you want to load them you must set up your own idt & gdt. */ - set_gdt(phys_to_virt(0), 0); idt_invalidate(phys_to_virt(0)); + set_gdt(phys_to_virt(0), 0); /* now call it */ image->start = relocate_kernel_ptr((unsigned long)image->head, -- 2.15.1.391.gc6612a551