2020-04-28 15:26:48

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH v3 12/75] x86/boot/compressed/64: Switch to __KERNEL_CS after GDT is loaded

From: Joerg Roedel <[email protected]>

When the pre-decompression code loads its first GDT in startup_64, it is
still running on the CS value of the previous GDT. In the case of SEV-ES
this is the EFI GDT.

To make exception handling work (especially IRET) the CPU needs to
switch to a CS value in the current GDT, so jump to __KERNEL_CS after
the first GDT is loaded.

Signed-off-by: Joerg Roedel <[email protected]>
---
arch/x86/boot/compressed/head_64.S | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 4f7e6b84be07..6b11060c3a0f 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -393,6 +393,14 @@ SYM_CODE_START(startup_64)
addq %rax, 2(%rax)
lgdt (%rax)

+ /* Reload CS so IRET returns to a CS actually in the GDT */
+ pushq $__KERNEL_CS
+ leaq .Lon_kernel_cs(%rip), %rax
+ pushq %rax
+ lretq
+
+.Lon_kernel_cs:
+
/*
* paging_prepare() sets up the trampoline and checks if we need to
* enable 5-level paging.
--
2.17.1


2020-05-04 10:54:11

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 12/75] x86/boot/compressed/64: Switch to __KERNEL_CS after GDT is loaded

On Tue, Apr 28, 2020 at 05:16:22PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> When the pre-decompression code loads its first GDT in startup_64, it is
> still running on the CS value of the previous GDT. In the case of SEV-ES
> this is the EFI GDT.
>
> To make exception handling work (especially IRET) the CPU needs to
> switch to a CS value in the current GDT, so jump to __KERNEL_CS after
> the first GDT is loaded.
>
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> arch/x86/boot/compressed/head_64.S | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 4f7e6b84be07..6b11060c3a0f 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -393,6 +393,14 @@ SYM_CODE_START(startup_64)
> addq %rax, 2(%rax)
> lgdt (%rax)
>
> + /* Reload CS so IRET returns to a CS actually in the GDT */
> + pushq $__KERNEL_CS
> + leaq .Lon_kernel_cs(%rip), %rax
> + pushq %rax
> + lretq
> +
> +.Lon_kernel_cs:
> +
> /*
> * paging_prepare() sets up the trampoline and checks if we need to
> * enable 5-level paging.
> --

So I'm thinking I should take this one even now on the grounds that
it sanitizes CS to something known-good than what was there before and
who knows what set it and loaded the kernel...?

And that is a good thing in itself.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2020-05-04 11:31:37

by Jörg Rödel

[permalink] [raw]
Subject: Re: [PATCH v3 12/75] x86/boot/compressed/64: Switch to __KERNEL_CS after GDT is loaded

On Mon, May 04, 2020 at 12:41:29PM +0200, Borislav Petkov wrote:
> On Tue, Apr 28, 2020 at 05:16:22PM +0200, Joerg Roedel wrote:
> > + /* Reload CS so IRET returns to a CS actually in the GDT */
> > + pushq $__KERNEL_CS
> > + leaq .Lon_kernel_cs(%rip), %rax
> > + pushq %rax
> > + lretq
> > +
> > +.Lon_kernel_cs:
> > +
> > /*
> > * paging_prepare() sets up the trampoline and checks if we need to
> > * enable 5-level paging.
> > --
>
> So I'm thinking I should take this one even now on the grounds that
> it sanitizes CS to something known-good than what was there before and
> who knows what set it and loaded the kernel...?
>
> And that is a good thing in itself.

Right, sure. CS is basically undefined at this point and depends on what
loaded the kernel (EFI, legacy boot code, some container runtime...), so
setting it to something known is definitly good.

Regards,

Joerg

Subject: [tip: x86/boot] x86/boot/compressed/64: Switch to __KERNEL_CS after GDT is loaded

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: 34bb49229f19399a5b45c323afb5749f31f7876c
Gitweb: https://git.kernel.org/tip/34bb49229f19399a5b45c323afb5749f31f7876c
Author: Joerg Roedel <[email protected]>
AuthorDate: Tue, 28 Apr 2020 17:16:22 +02:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Mon, 04 May 2020 19:53:08 +02:00

x86/boot/compressed/64: Switch to __KERNEL_CS after GDT is loaded

When the pre-decompression code loads its first GDT in startup_64(), it
is still running on the CS value of the previous GDT. In the case of
SEV-ES, this is the EFI GDT but it can be anything depending on what has
loaded the kernel (boot loader, container runtime, etc.)

To make exception handling work (especially IRET) the CPU needs to
switch to a CS value in the current GDT, so jump to __KERNEL_CS after
the first GDT is loaded. This is prudent also as a general sanitization
of CS to a known good value.

[ bp: Massage commit message. ]

Signed-off-by: Joerg Roedel <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/boot/compressed/head_64.S | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 4f7e6b8..6b11060 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -393,6 +393,14 @@ SYM_CODE_START(startup_64)
addq %rax, 2(%rax)
lgdt (%rax)

+ /* Reload CS so IRET returns to a CS actually in the GDT */
+ pushq $__KERNEL_CS
+ leaq .Lon_kernel_cs(%rip), %rax
+ pushq %rax
+ lretq
+
+.Lon_kernel_cs:
+
/*
* paging_prepare() sets up the trampoline and checks if we need to
* enable 5-level paging.