Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1347967 > unrolled thread
| Started by | Borislav Petkov <bp@alien8.de> |
|---|---|
| First post | 2016-03-02 12:30 +0100 |
| Last post | 2016-03-02 17:30 +0100 |
| Articles | 14 on this page of 34 — 5 participants |
Back to article view | Back to linux.kernel
[RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 12:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 17:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Mika Penttilä <mika.penttila@nextfour.com> - 2016-03-02 17:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 18:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Mika Penttilä <mika.penttila@nextfour.com> - 2016-03-02 18:50 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Mika Penttilä <mika.penttila@nextfour.com> - 2016-03-02 17:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 17:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 19:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 19:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 19:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 19:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 21:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 21:50 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 22:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 22:50 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 23:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 23:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 23:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-02 23:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 23:50 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Yinghai Lu <yinghai@kernel.org> - 2016-03-03 01:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Yinghai Lu <yinghai@kernel.org> - 2016-03-03 02:10 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Yinghai Lu <yinghai@kernel.org> - 2016-03-03 04:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-03 13:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-03 16:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-03 17:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-03 21:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-03 22:00 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack "H. Peter Anvin" <hpa@zytor.com> - 2016-03-03 22:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-03 22:40 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Yinghai Lu <yinghai@kernel.org> - 2016-03-04 02:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Yinghai Lu <yinghai@kernel.org> - 2016-03-04 03:30 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Borislav Petkov <bp@alien8.de> - 2016-03-02 23:20 +0100
Re: [RFC PATCH] x86: Make sure verify_cpu has a good stack Brian Gerst <brgerst@gmail.com> - 2016-03-02 17:30 +0100
Page 2 of 2 — ← Prev page 1 [2]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-03-03 01:20 +0100 |
| Message-ID | <r8p9M-7js-7@gated-at.bofh.it> |
| In reply to | #1348576 |
On Wed, Mar 2, 2016 at 2:32 PM, H. Peter Anvin <hpa@zytor.com> 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. Why do we need to call verify_cpu in arch/x86/kernel/head_64.S aka the vmlinux again ? Is that already called in arch/x86/boot/compressed/head_64.S? Or Tom is using 64bit bootloader that use vmlinux instead of bzImage? Thanks Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-03-03 02:10 +0100 |
| Message-ID | <r8pWa-7WC-7@gated-at.bofh.it> |
| In reply to | #1348636 |
On Wed, Mar 2, 2016 at 4:13 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Wed, Mar 2, 2016 at 2:32 PM, H. Peter Anvin <hpa@zytor.com> 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. > > Why do we need to call verify_cpu in arch/x86/kernel/head_64.S aka the > vmlinux again ? > > Is that already called in arch/x86/boot/compressed/head_64.S? that calling is from startup_32, so may add another calling in startup_64, so can avoid calling from arch/x86/kernel/head_64.S > > Or Tom is using 64bit bootloader that use vmlinux instead of bzImage?
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-03-03 04:00 +0100 |
| Message-ID | <r8rEC-pu-13@gated-at.bofh.it> |
| In reply to | #1348662 |
On Wed, Mar 2, 2016 at 5:00 PM, Yinghai Lu <yinghai@kernel.org> wrote: > On Wed, Mar 2, 2016 at 4:13 PM, Yinghai Lu <yinghai@kernel.org> wrote: >> On Wed, Mar 2, 2016 at 2:32 PM, H. Peter Anvin <hpa@zytor.com> 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. >> >> Why do we need to call verify_cpu in arch/x86/kernel/head_64.S aka the >> vmlinux again ? >> >> Is that already called in arch/x86/boot/compressed/head_64.S? > > that calling is from startup_32, so may add another calling in startup_64, > so can avoid calling from arch/x86/kernel/head_64.S > at the same time, the "call verify_cpu" in arch/x86/kernel/head_64.s::secondary_startup_64() is not needed. As APs already go through arch/x86/realmode/rm/trampoline_64.S::trampoline_start(), and it already call verify_cpu in 16bit mode. Thanks Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-03 13:30 +0100 |
| Message-ID | <r8Aye-745-13@gated-at.bofh.it> |
| In reply to | #1348576 |
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@suse.de>
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@amd.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Mika Penttilä <mika.penttila@nextfour.com>
---
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
--
2.3.5
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | "H. Peter Anvin" <hpa@zytor.com> |
|---|---|
| Date | 2016-03-03 16:30 +0100 |
| Message-ID | <r8Dmq-ux-23@gated-at.bofh.it> |
| In reply to | #1349092 |
On March 3, 2016 4:28:36 AM PST, Borislav Petkov <bp@alien8.de> 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@suse.de>
>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@amd.com>
>Signed-off-by: Borislav Petkov <bp@suse.de>
>Cc: Brian Gerst <brgerst@gmail.com>
>Cc: "H. Peter Anvin" <hpa@zytor.com>
>Cc: Mika Penttilä <mika.penttila@nextfour.com>
>---
> 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.
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-03 17:40 +0100 |
| Message-ID | <r8Es9-1q6-3@gated-at.bofh.it> |
| In reply to | #1349284 |
On Thu, Mar 03, 2016 at 07:26:06AM -0800, H. Peter Anvin wrote:
> Why -8?
GLOBAL(stack_start)
.quad init_thread_union+THREAD_SIZE-8
^^^
But I don't see why it needed the -8 then. It came with a conglomerate
dump in 2002:
commit af53c7a2c81399b805b6d4eff887401a5e50feef
Author: Andi Kleen <ak@muc.de>
Date: Fri Apr 19 20:23:17 2002 -0700
[PATCH] x86-64 architecture specific sync for 2.5.8
- /* Setup the first kernel stack (this instruction is modified by smpboot) */
- .byte 0x48, 0xb8 /* movq *init_rsp,%rax */
-init_rsp:
- .quad init_thread_union+THREAD_SIZE
- movq %rax, %rsp
...
-
- /* SMP bootup changes this */
+ /* SMP bootup changes these two */
.globl initial_code
initial_code:
.quad x86_64_start_kernel
+ .globl init_rsp
+init_rsp:
+ .quad init_thread_union+THREAD_SIZE-8
+
---
But since we decrement first and then copy to stack ptr when we push, I
don't see why we need the -8.
Do you have a better clue?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | "H. Peter Anvin" <hpa@zytor.com> |
|---|---|
| Date | 2016-03-03 21:30 +0100 |
| Message-ID | <r8I2K-3U3-7@gated-at.bofh.it> |
| In reply to | #1349346 |
On 03/03/16 08:29, Borislav Petkov wrote: > On Thu, Mar 03, 2016 at 07:26:06AM -0800, H. Peter Anvin wrote: >> Why -8? > > GLOBAL(stack_start) > .quad init_thread_union+THREAD_SIZE-8 > ^^^ > > But I don't see why it needed the -8 then. It came with a conglomerate > dump in 2002: > > commit af53c7a2c81399b805b6d4eff887401a5e50feef > Author: Andi Kleen <ak@muc.de> > Date: Fri Apr 19 20:23:17 2002 -0700 > > [PATCH] x86-64 architecture specific sync for 2.5.8 > > > - /* Setup the first kernel stack (this instruction is modified by smpboot) */ > - .byte 0x48, 0xb8 /* movq *init_rsp,%rax */ > -init_rsp: > - .quad init_thread_union+THREAD_SIZE > - movq %rax, %rsp > > ... > > - > - /* SMP bootup changes this */ > + /* SMP bootup changes these two */ > .globl initial_code > initial_code: > .quad x86_64_start_kernel > + .globl init_rsp > +init_rsp: > + .quad init_thread_union+THREAD_SIZE-8 > + > --- > > But since we decrement first and then copy to stack ptr when we push, I > don't see why we need the -8. > > Do you have a better clue? > The only thing I can think of is that the -8 creates a null pointer that terminates a stack trace. -hpa
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-03 22:00 +0100 |
| Message-ID | <r8IvM-49H-13@gated-at.bofh.it> |
| In reply to | #1349555 |
On Thu, Mar 03, 2016 at 12:22:06PM -0800, H. Peter Anvin wrote:
> The only thing I can think of is that the -8 creates a null pointer that
> terminates a stack trace.
Probably not needed anymore as print_context_stack()->valid_stack_ptr()
in dumpstack.c look at the stack boundaries instead of checking for
NULL...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | "H. Peter Anvin" <hpa@zytor.com> |
|---|---|
| Date | 2016-03-03 22:30 +0100 |
| Message-ID | <r8IYO-4Fu-19@gated-at.bofh.it> |
| In reply to | #1349569 |
On 03/03/16 12:54, Borislav Petkov wrote: > On Thu, Mar 03, 2016 at 12:22:06PM -0800, H. Peter Anvin wrote: >> The only thing I can think of is that the -8 creates a null pointer that >> terminates a stack trace. > > Probably not needed anymore as print_context_stack()->valid_stack_ptr() > in dumpstack.c look at the stack boundaries instead of checking for > NULL... > Sure, but it could still affect kgdb or what not. That's the only reason I can see for -8 though. -hpa
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-03 22:40 +0100 |
| Message-ID | <r8J8u-4LK-7@gated-at.bofh.it> |
| In reply to | #1349592 |
On Thu, Mar 03, 2016 at 01:22:59PM -0800, H. Peter Anvin wrote:
> reason I can see for -8 though.
... and keeping the -8 won't hurt us or anything. I'll add a small
comment about it.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-03-04 02:20 +0100 |
| Message-ID | <r8Mzn-7ld-3@gated-at.bofh.it> |
| In reply to | #1349092 |
On Thu, Mar 3, 2016 at 4:28 AM, Borislav Petkov <bp@alien8.de> wrote:
>
> 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.
Even more than that. For AP verify_cpu already get called in trampoline.
arch/x86/realmode/rm/trampoline_64.S::trampoline_start().
So you remove verify_cpu calling in secondary_startup_64.
Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-03-04 03:30 +0100 |
| Message-ID | <r8NF7-8b8-7@gated-at.bofh.it> |
| In reply to | #1349092 |
On Thu, Mar 3, 2016 at 4:28 AM, Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
> Date: Sun, 28 Feb 2016 21:35:44 +0100
> Subject: [PATCH -v2] x86/asm: Make sure verify_cpu() has a good stack
> 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.
>
> Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Mika Penttilä <mika.penttila@nextfour.com>
> ---
> 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
> --
> 2.3.5
I would suggest moving down verify_cpu calling after offset is
calcuated, like following instead of adding __end_init_task.
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 22fbf9d..e8c8085 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -64,9 +64,6 @@ startup_64:
* tables and then reload them.
*/
- /* Sanitize CPU configuration */
- call verify_cpu
-
/*
* Compute the delta between the address I am compiled to run at and the
* address I am actually running at.
@@ -74,6 +71,14 @@ startup_64:
leaq _text(%rip), %rbp
subq $_text - __START_KERNEL_map, %rbp
+ /* Setup stack for verify_cpu() */
+ movq stack_start(%rip), %rsp
+ subq $__START_KERNEL_map, %rsp
+ addq %rbp, %rsp
+
+ /* Sanitize CPU configuration */
+ call verify_cpu
+
/* Is the address not 2M aligned? */
testl $~PMD_PAGE_MASK, %ebp
jnz bad_address
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2016-03-02 23:20 +0100 |
| Message-ID | <r8nhE-5Zk-33@gated-at.bofh.it> |
| In reply to | #1348494 |
On Wed, Mar 02, 2016 at 01:54:50PM -0800, H. Peter Anvin wrote:
> A relocating bootloader is one that doesn't load the kernel at
> CONFIG_PHYSICAL_ADDRESS. The EFI stub is one example.
>
> __START_KERNEL_map is not relocated. On x86-64 we do relocation by
> pointing the page tables at a different address.
>
> So I really think we need this to be a leaq, so we take a nonstandard
> load address into consideration.
Hmm, but __START_KERNEL_map is a simple macro:
#define __START_KERNEL_map _AC(0xffffffff80000000, UL)
Ok, I think you want to do something like this for stack_start too:
/*
* Compute the delta between the address I am compiled to run at and the
* address I am actually running at.
*/
leaq _text(%rip), %rbp
subq $_text - __START_KERNEL_map, %rbp
...
in the normal case %rbp is 0, of course.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
[toc] | [prev] | [next] | [standalone]
| From | Brian Gerst <brgerst@gmail.com> |
|---|---|
| Date | 2016-03-02 17:30 +0100 |
| Message-ID | <r8hOW-1I2-5@gated-at.bofh.it> |
| In reply to | #1347967 |
On Wed, Mar 2, 2016 at 6:20 AM, Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
>
> 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@amd.com>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
> arch/x86/kernel/head_64.S | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index 22fbf9df61bb..d60a044c2fdc 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -64,6 +64,10 @@ startup_64:
> * tables and then reload them.
> */
>
> + /* Setup a stack for verify_cpu */
> + movq stack_start - __START_KERNEL_map, %rsp
This should be: movq stack_start(%rip), %rsp
> + subq $__START_KERNEL_map, %rsp
It would be better to add the offset to the initializer for
stack_start instead of adjusting it at runtime. That would require
moving the existing load of stack_start from the common path to the
secondary startup, which probably isn't a bad thing as it wouldn't
depend on the trampoline stack anymore.
--
Brian Gerst
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web