Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack

[H. Peter Anvin]

On March 3, 2016 4:28:36 AM PST, Borislav Petkov <bp@xxxxxxxxx> wrote:
>On Wed, Mar 02, 2016 at 02:32:54PM -0800, H. Peter Anvin wrote:
>> I'm trying to think of any reason why we couldn't simply have a
>symbol
>> at the top of the initial stack? Then a simple leaq would suffice;
>> this is for the BSP after all.
>
>How about something like this:
>
>---
>From: Borislav Petkov <bp@xxxxxxx>
>Date: Sun, 28 Feb 2016 21:35:44 +0100
>Subject: [PATCH -v2] x86/asm: Make sure verify_cpu() has a good stack
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>04633df0c43d ("x86/cpu: Call verify_cpu() after having entered long
>mode too")
>added the call to verify_cpu() for sanitizing CPU configuration.
>
>The latter uses the stack minimally and it can happen that we land in
>startup_64() directly from a 64-bit bootloader. Then we want to use our
>own, known good stack.
>
>Do that.
>
>APs don't need this as the trampoline sets up a stack for them.
>
>Reported-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
>Signed-off-by: Borislav Petkov <bp@xxxxxxx>
>Cc: Brian Gerst <brgerst@xxxxxxxxx>
>Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>Cc: Mika Penttilà <mika.penttila@xxxxxxxxxxxx>
>---
> arch/x86/kernel/head_64.S         | 3 +++
> include/asm-generic/vmlinux.lds.h | 4 +++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
>index 22fbf9df61bb..968d6408b887 100644
>--- a/arch/x86/kernel/head_64.S
>+++ b/arch/x86/kernel/head_64.S
>@@ -64,6 +64,9 @@ startup_64:
> 	 * tables and then reload them.
> 	 */
> 
>+	/* Setup stack for verify_cpu(). */
>+	leaq	(__end_init_task - 8)(%rip), %rsp
>+
> 	/* Sanitize CPU configuration */
> 	call verify_cpu
> 
>diff --git a/include/asm-generic/vmlinux.lds.h
>b/include/asm-generic/vmlinux.lds.h
>index 772c784ba763..cba2a26628fc 100644
>--- a/include/asm-generic/vmlinux.lds.h
>+++ b/include/asm-generic/vmlinux.lds.h
>@@ -246,7 +246,9 @@
> 
> #define INIT_TASK_DATA(align)						\
> 	. = ALIGN(align);						\
>-	*(.data..init_task)
>+	VMLINUX_SYMBOL(__start_init_task) = .;				\
>+	*(.data..init_task)						\
>+	VMLINUX_SYMBOL(__end_init_task) = .;
> 
> /*
>  * Read only Data

Why -8?
-- 
Sent from my Android device with K-9 Mail. Please excuse brevity and formatting.