[PATCH v2 0/5] Couple of bugfixes to sev-es series

[PATCH v2 0/5] Couple of bugfixes to sev-es series

[Arvind Sankar]

With the SEV-ES series, the kernel command line is no longer guaranteed
to be mapped on entry into the main kernel. This fixes that, and a
stackprotector issue that cropped up on head64.c.

The first three patches are preparatory cleanups. Patch 4 fixes the
mapping issue and patch 5 disables stack protector for head code.

Changes from v1:
- Add comments suggested by Joerg
- Split out cmdline into cmdline.h and use it
- Leave add_identity_map() as [start,end)

v1: https://lore.kernel.org/lkml/20201007195351.776555-1-nivedita@alum.mit.edu/

Arvind Sankar (5):
  x86/boot: Initialize boot_params in startup code
  x86/boot: Split out command-line related declarations
  x86/boot/64: Show original faulting address in case of error
  x86/boot/64: Explicitly map boot_params and command line
  x86/head/64: Disable stack protection for head$(BITS).o

 arch/x86/boot/compressed/acpi.c               |  1 +
 arch/x86/boot/compressed/cmdline.c            |  1 +
 arch/x86/boot/compressed/cmdline.h            | 13 +++++++
 .../boot/compressed/early_serial_console.c    |  1 +
 arch/x86/boot/compressed/head_32.S            | 12 ++++---
 arch/x86/boot/compressed/head_64.S            | 35 +++++++------------
 arch/x86/boot/compressed/ident_map_64.c       | 22 +++++++++---
 arch/x86/boot/compressed/kaslr.c              |  7 +---
 arch/x86/boot/compressed/misc.c               | 10 +-----
 arch/x86/boot/compressed/misc.h               |  4 ---
 arch/x86/boot/compressed/pgtable_64.c         |  7 ++--
 arch/x86/kernel/Makefile                      |  2 ++
 12 files changed, 58 insertions(+), 57 deletions(-)
 create mode 100644 arch/x86/boot/compressed/cmdline.h

-- 
2.26.2

[PATCH v2 1/5] x86/boot: Initialize boot_params in startup code

[Arvind Sankar]

Save the boot_params pointer passed in by the bootloader in
startup_32/64. This avoids having to initialize it in two different
places in C code, and having to preserve SI through the early assembly
code.

Move boot_params from .bss to .data, since .bss will be cleared after
the kernel is relocated.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/boot/compressed/head_32.S    | 12 +++++----
 arch/x86/boot/compressed/head_64.S    | 35 +++++++++------------------
 arch/x86/boot/compressed/misc.c       | 10 +-------
 arch/x86/boot/compressed/pgtable_64.c |  5 +---
 4 files changed, 21 insertions(+), 41 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 659fad53ca82..9b5c88d8b290 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -113,6 +113,9 @@ SYM_FUNC_START(startup_32)
 	addl    BP_init_size(%esi), %ebx
 	subl    $_end@GOTOFF, %ebx
 
+	/* Initialize boot_params */
+	movl	%esi, boot_params@GOTOFF(%edx)
+
 	/* Set up the stack */
 	leal	boot_stack_end@GOTOFF(%ebx), %esp
 
@@ -124,7 +127,6 @@ SYM_FUNC_START(startup_32)
  * Copy the compressed kernel to the end of our buffer
  * where decompression in place becomes safe.
  */
-	pushl	%esi
 	leal	(_bss@GOTOFF-4)(%edx), %esi
 	leal	(_bss@GOTOFF-4)(%ebx), %edi
 	movl	$(_bss - startup_32), %ecx
@@ -132,7 +134,6 @@ SYM_FUNC_START(startup_32)
 	std
 	rep	movsl
 	cld
-	popl	%esi
 
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
@@ -187,14 +188,12 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushl	%eax			/* input_data */
 	leal	boot_heap@GOTOFF(%ebx), %eax
 	pushl	%eax			/* heap area */
-	pushl	%esi			/* real mode pointer */
 	call	extract_kernel		/* returns kernel location in %eax */
-	addl	$24, %esp
 
 /*
  * Jump to the extracted kernel.
  */
-	xorl	%ebx, %ebx
+	movl	boot_params@GOTOFF(%ebx), %esi
 	jmp	*%eax
 SYM_FUNC_END(.Lrelocated)
 
@@ -209,6 +208,9 @@ SYM_DATA_START_LOCAL(gdt)
 	.quad	0x00cf92000000ffff	/* __KERNEL_DS */
 SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)
 
+/* boot_params must be in .data because .bss is cleared after the kernel is relocated */
+SYM_DATA(boot_params, .long 0)
+
 #ifdef CONFIG_EFI_STUB
 SYM_DATA(image_offset, .long 0)
 #endif
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1c80f1738fd9..521b41ca14fe 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -375,6 +375,9 @@ SYM_CODE_START(startup_64)
 	subl	$ rva(_end), %ebx
 	addq	%rbp, %rbx
 
+	/* Initialize boot_params */
+	movq	%rsi, boot_params(%rip)
+
 	/* Set up the stack */
 	leaq	rva(boot_stack_end)(%rbx), %rsp
 
@@ -429,14 +432,8 @@ SYM_CODE_START(startup_64)
 	 *   - Address of the trampoline is returned in RAX.
 	 *   - Non zero RDX means trampoline needs to enable 5-level
 	 *     paging.
-	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
-	movq	%rsi, %rdi		/* real mode address */
 	call	paging_prepare
-	popq	%rsi
 
 	/* Save the trampoline address in RCX */
 	movq	%rax, %rcx
@@ -461,14 +458,9 @@ trampoline_return:
 	 *
 	 * RDI is address of the page table to use instead of page table
 	 * in trampoline memory (if required).
-	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
 	leaq	rva(top_pgtable)(%rbx), %rdi
 	call	cleanup_trampoline
-	popq	%rsi
 
 	/* Zero EFLAGS */
 	pushq	$0
@@ -478,7 +470,6 @@ trampoline_return:
  * Copy the compressed kernel to the end of our buffer
  * where decompression in place becomes safe.
  */
-	pushq	%rsi
 	leaq	(_bss-8)(%rip), %rsi
 	leaq	rva(_bss-8)(%rbx), %rdi
 	movl	$(_bss - startup_32), %ecx
@@ -486,7 +477,6 @@ trampoline_return:
 	std
 	rep	movsq
 	cld
-	popq	%rsi
 
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
@@ -541,28 +531,24 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
  * handler. Then load stage2 IDT and switch to the kernel's own
  * page-table.
  */
-	pushq	%rsi
 	call	set_sev_encryption_mask
 	call	load_stage2_idt
 	call	initialize_identity_maps
-	popq	%rsi
 
 /*
  * Do the extraction, and jump to the new kernel..
  */
-	pushq	%rsi			/* Save the real mode argument */
-	movq	%rsi, %rdi		/* real mode address */
-	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
-	leaq	input_data(%rip), %rdx  /* input_data */
-	movl	input_len(%rip), %ecx	/* input_len */
-	movq	%rbp, %r8		/* output target address */
-	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
+	leaq	boot_heap(%rip), %rdi	/* malloc area for uncompression */
+	leaq	input_data(%rip), %rsi  /* input_data */
+	movl	input_len(%rip), %edx	/* input_len */
+	movq	%rbp, %rcx		/* output target address */
+	movl	output_len(%rip), %r8d	/* decompressed length, end of relocs */
 	call	extract_kernel		/* returns kernel location in %rax */
-	popq	%rsi
 
 /*
  * Jump to the decompressed kernel.
  */
+	movq	boot_params(%rip), %rsi
 	jmp	*%rax
 SYM_FUNC_END(.Lrelocated)
 
@@ -691,6 +677,9 @@ SYM_DATA_START(boot_idt)
 	.endr
 SYM_DATA_END_LABEL(boot_idt, SYM_L_GLOBAL, boot_idt_end)
 
+/* boot_params must be in .data because .bss is cleared after the kernel is relocated */
+SYM_DATA(boot_params, .quad 0)
+
 #ifdef CONFIG_EFI_STUB
 SYM_DATA(image_offset, .long 0)
 #endif
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 267e7f93050e..279631650bd8 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -39,11 +39,6 @@
 /* Functions used by the included decompressor code below. */
 void *memmove(void *dest, const void *src, size_t n);
 
-/*
- * This is set up by the setup-routine at boot-time
- */
-struct boot_params *boot_params;
-
 memptr free_mem_ptr;
 memptr free_mem_end_ptr;
 
@@ -338,7 +333,7 @@ static void parse_elf(void *output)
  *             |-------uncompressed kernel image---------|
  *
  */
-asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
+asmlinkage __visible void *extract_kernel(memptr heap,
 				  unsigned char *input_data,
 				  unsigned long input_len,
 				  unsigned char *output,
@@ -348,9 +343,6 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 	unsigned long virt_addr = LOAD_PHYSICAL_ADDR;
 	unsigned long needed_size;
 
-	/* Retain x86 boot parameters pointer passed from startup_32/64. */
-	boot_params = rmode;
-
 	/* Clear flags intended for solely in-kernel use. */
 	boot_params->hdr.loadflags &= ~KASLR_FLAG;
 
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 7d0394f4ebf9..0fb948c0c8b4 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -98,13 +98,10 @@ static unsigned long find_trampoline_placement(void)
 	return bios_start - TRAMPOLINE_32BIT_SIZE;
 }
 
-struct paging_config paging_prepare(void *rmode)
+struct paging_config paging_prepare(void)
 {
 	struct paging_config paging_config = {};
 
-	/* Initialize boot_params. Required for cmdline_find_option_bool(). */
-	boot_params = rmode;
-
 	/*
 	 * Check if LA57 is desired and supported.
 	 *
-- 
2.26.2

[PATCH v2 2/5] x86/boot: Split out command-line related declarations

[Arvind Sankar]

Move prototypes for command-line related functions into a new header
file to split it out from misc.h.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/acpi.c                 |  1 +
 arch/x86/boot/compressed/cmdline.c              |  1 +
 arch/x86/boot/compressed/cmdline.h              | 13 +++++++++++++
 arch/x86/boot/compressed/early_serial_console.c |  1 +
 arch/x86/boot/compressed/kaslr.c                |  7 +------
 arch/x86/boot/compressed/misc.h                 |  4 ----
 arch/x86/boot/compressed/pgtable_64.c           |  2 +-
 7 files changed, 18 insertions(+), 11 deletions(-)
 create mode 100644 arch/x86/boot/compressed/cmdline.h

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 8bcbcee54aa1..9097108c37e1 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -3,6 +3,7 @@
 #include "misc.h"
 #include "error.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #include <linux/numa.h>
 #include <linux/efi.h>
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index f1add5d85da9..20f2e6d8b891 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "misc.h"
+#include "cmdline.h"
 
 static unsigned long fs;
 static inline void set_fs(unsigned long seg)
diff --git a/arch/x86/boot/compressed/cmdline.h b/arch/x86/boot/compressed/cmdline.h
new file mode 100644
index 000000000000..72800770bd60
--- /dev/null
+++ b/arch/x86/boot/compressed/cmdline.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef BOOT_COMPRESSED_CMDLINE_H
+#define BOOT_COMPRESSED_CMDLINE_H
+
+#define _SETUP
+#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
+#undef _SETUP
+
+unsigned long get_cmd_line_ptr(void);
+int cmdline_find_option(const char *option, char *buffer, int bufsize);
+int cmdline_find_option_bool(const char *option);
+
+#endif /* BOOT_COMPRESSED_CMDLINE_H */
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 261e81fb9582..64a1f557e122 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,4 +1,5 @@
 #include "misc.h"
+#include "cmdline.h"
 
 int early_serial_base;
 
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b59547ce5b19..489fabc051d7 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -22,6 +22,7 @@
 #include "misc.h"
 #include "error.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #include <generated/compile.h>
 #include <linux/module.h>
@@ -36,12 +37,6 @@
 #define STATIC
 #include <linux/decompress/mm.h>
 
-#define _SETUP
-#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
-#undef _SETUP
-
-extern unsigned long get_cmd_line_ptr(void);
-
 /* Simplified build-specific string for starting entropy. */
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 6d31f1b4c4d1..e3e2f312c025 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -69,10 +69,6 @@ static inline void debug_puthex(unsigned long value)
 
 #endif
 
-/* cmdline.c */
-int cmdline_find_option(const char *option, char *buffer, int bufsize);
-int cmdline_find_option_bool(const char *option);
-
 struct mem_vector {
 	u64 start;
 	u64 size;
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 0fb948c0c8b4..46d761f7faa6 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -4,6 +4,7 @@
 #include <asm/efi.h>
 #include "pgtable.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
 #define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
@@ -26,7 +27,6 @@ static char trampoline_save[TRAMPOLINE_32BIT_SIZE];
 unsigned long *trampoline_32bit __section(.data);
 
 extern struct boot_params *boot_params;
-int cmdline_find_option_bool(const char *option);
 
 static unsigned long find_trampoline_placement(void)
 {
-- 
2.26.2

[PATCH v2 3/5] x86/boot/64: Show original faulting address in case of error

[Arvind Sankar]

This makes the #PF handler print the original CR2 value in case of
error, instead of after aligning to PMD_SIZE.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/ident_map_64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index 063a60edcf99..fd957b2625e9 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -327,9 +327,6 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
 
 	ghcb_fault = sev_es_check_ghcb_fault(address);
 
-	address   &= PMD_MASK;
-	end        = address + PMD_SIZE;
-
 	/*
 	 * Check for unexpected error codes. Unexpected are:
 	 *	- Faults on present pages
@@ -345,5 +342,8 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
 	 * Error code is sane - now identity map the 2M region around
 	 * the faulting address.
 	 */
+	address   &= PMD_MASK;
+	end        = address + PMD_SIZE;
+
 	add_identity_map(address, end);
 }
-- 
2.26.2

[PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

Commits

  ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
  8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")

set up a new page table in the decompressor stub, but without explicit
mappings for boot_params and the kernel command line, relying on the #PF
handler instead.

This is fragile, as boot_params and the command line mappings are
required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
disabled, a QEMU/OVMF boot never accesses the command line in the
decompressor stub, and so it never gets mapped. The main kernel accesses
it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
crash.

Fix this by adding back the explicit mapping of boot_params and the
command line.

Note: the changes also removed the explicit mapping of the main kernel,
with the result that .bss and .brk may not be in the identity mapping,
but those don't get accessed by the main kernel before it switches to
its own page tables.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/ident_map_64.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index fd957b2625e9..a3613857c532 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -21,6 +21,7 @@
 
 #include "error.h"
 #include "misc.h"
+#include "cmdline.h"
 
 /* These actually do the work of building the kernel identity maps. */
 #include <linux/pgtable.h>
@@ -109,6 +110,8 @@ static void add_identity_map(unsigned long start, unsigned long end)
 /* Locates and clears a region for a new top level page table. */
 void initialize_identity_maps(void)
 {
+	unsigned long cmdline;
+
 	/* Exclude the encryption mask from __PHYSICAL_MASK */
 	physical_mask &= ~sme_me_mask;
 
@@ -149,10 +152,19 @@ void initialize_identity_maps(void)
 	}
 
 	/*
-	 * New page-table is set up - map the kernel image and load it
-	 * into cr3.
+	 * New page-table is set up - map the kernel image, boot_params and the
+	 * command line.
+	 * The uncompressed kernel requires boot_params and the command line to
+	 * be mapped in the identity mapping.
+	 * Map them explicitly here in case the compressed kernel does not
+	 * touch them, or does not touch all the pages covering them.
 	 */
 	add_identity_map((unsigned long)_head, (unsigned long)_end);
+	add_identity_map((unsigned long)boot_params, (unsigned long)(boot_params + 1));
+	cmdline = get_cmd_line_ptr();
+	add_identity_map(cmdline, cmdline + COMMAND_LINE_SIZE);
+
+	/* Load the new page-table. */
 	write_cr3(top_level_pgt);
 }
 
-- 
2.26.2

[PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Arvind Sankar]

On 64-bit, the startup_64_setup_env() function added in
  866b556efa12 ("x86/head/64: Install startup GDT")
has stack protection enabled because of set_bringup_idt_handler().

At this point, %gs is not yet initialized, and this doesn't cause a
crash only because the #PF handler from the decompressor stub is still
installed and handles the page fault.

Disable stack protection for the whole file, and do it on 32-bit as
well to avoid surprises.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/kernel/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 04ceea8f4a89..68608bd892c0 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -47,6 +47,8 @@ endif
 # non-deterministic coverage.
 KCOV_INSTRUMENT		:= n
 
+CFLAGS_head$(BITS).o	+= -fno-stack-protector
+
 CFLAGS_irq.o := -I $(srctree)/$(src)/../include/asm/trace
 
 obj-y			:= process_$(BITS).o signal.o
-- 
2.26.2

Re: [PATCH v2 3/5] x86/boot/64: Show original faulting address in case of error

[Joerg Roedel]

On Thu, Oct 08, 2020 at 03:16:21PM -0400, Arvind Sankar wrote:
> This makes the #PF handler print the original CR2 value in case of
> error, instead of after aligning to PMD_SIZE.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/ident_map_64.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
> index 063a60edcf99..fd957b2625e9 100644
> --- a/arch/x86/boot/compressed/ident_map_64.c
> +++ b/arch/x86/boot/compressed/ident_map_64.c
> @@ -327,9 +327,6 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
>  
>  	ghcb_fault = sev_es_check_ghcb_fault(address);
>  
> -	address   &= PMD_MASK;
> -	end        = address + PMD_SIZE;
> -
>  	/*
>  	 * Check for unexpected error codes. Unexpected are:
>  	 *	- Faults on present pages
> @@ -345,5 +342,8 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
>  	 * Error code is sane - now identity map the 2M region around
>  	 * the faulting address.
>  	 */
> +	address   &= PMD_MASK;
> +	end        = address + PMD_SIZE;
> +
>  	add_identity_map(address, end);
>  }

Reviewed-by: Joerg Roedel <jroedel@suse.de>

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Joerg Roedel]

On Thu, Oct 08, 2020 at 03:16:22PM -0400, Arvind Sankar wrote:
> Commits
> 
>   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
>   8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")
> 
> set up a new page table in the decompressor stub, but without explicit
> mappings for boot_params and the kernel command line, relying on the #PF
> handler instead.
> 
> This is fragile, as boot_params and the command line mappings are
> required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
> disabled, a QEMU/OVMF boot never accesses the command line in the
> decompressor stub, and so it never gets mapped. The main kernel accesses
> it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
> crash.
> 
> Fix this by adding back the explicit mapping of boot_params and the
> command line.
> 
> Note: the changes also removed the explicit mapping of the main kernel,
> with the result that .bss and .brk may not be in the identity mapping,
> but those don't get accessed by the main kernel before it switches to
> its own page tables.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/ident_map_64.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
> index fd957b2625e9..a3613857c532 100644
> --- a/arch/x86/boot/compressed/ident_map_64.c
> +++ b/arch/x86/boot/compressed/ident_map_64.c
> @@ -21,6 +21,7 @@
>  
>  #include "error.h"
>  #include "misc.h"
> +#include "cmdline.h"
>  
>  /* These actually do the work of building the kernel identity maps. */
>  #include <linux/pgtable.h>
> @@ -109,6 +110,8 @@ static void add_identity_map(unsigned long start, unsigned long end)
>  /* Locates and clears a region for a new top level page table. */
>  void initialize_identity_maps(void)
>  {
> +	unsigned long cmdline;
> +
>  	/* Exclude the encryption mask from __PHYSICAL_MASK */
>  	physical_mask &= ~sme_me_mask;
>  
> @@ -149,10 +152,19 @@ void initialize_identity_maps(void)
>  	}
>  
>  	/*
> -	 * New page-table is set up - map the kernel image and load it
> -	 * into cr3.
> +	 * New page-table is set up - map the kernel image, boot_params and the
> +	 * command line.
> +	 * The uncompressed kernel requires boot_params and the command line to
> +	 * be mapped in the identity mapping.
> +	 * Map them explicitly here in case the compressed kernel does not
> +	 * touch them, or does not touch all the pages covering them.
>  	 */
>  	add_identity_map((unsigned long)_head, (unsigned long)_end);
> +	add_identity_map((unsigned long)boot_params, (unsigned long)(boot_params + 1));
> +	cmdline = get_cmd_line_ptr();
> +	add_identity_map(cmdline, cmdline + COMMAND_LINE_SIZE);
> +
> +	/* Load the new page-table. */
>  	write_cr3(top_level_pgt);
>  }

Reviewed-by: Joerg Roedel <jroedel@suse.de>

Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Joerg Roedel]

On Thu, Oct 08, 2020 at 03:16:23PM -0400, Arvind Sankar wrote:
> On 64-bit, the startup_64_setup_env() function added in
>   866b556efa12 ("x86/head/64: Install startup GDT")
> has stack protection enabled because of set_bringup_idt_handler().
> 
> At this point, %gs is not yet initialized, and this doesn't cause a
> crash only because the #PF handler from the decompressor stub is still
> installed and handles the page fault.
> 
> Disable stack protection for the whole file, and do it on 32-bit as
> well to avoid surprises.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/kernel/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 04ceea8f4a89..68608bd892c0 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -47,6 +47,8 @@ endif
>  # non-deterministic coverage.
>  KCOV_INSTRUMENT		:= n
>  
> +CFLAGS_head$(BITS).o	+= -fno-stack-protector
> +
>  CFLAGS_irq.o := -I $(srctree)/$(src)/../include/asm/trace
>  
>  obj-y			:= process_$(BITS).o signal.o
> -- 
> 2.26.2

Reviewed-by: Joerg Roedel <jroedel@suse.de>

[PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Arvind Sankar]

Commit
  ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
started using a new set of pagetables even without KASLR.

After that commit, initialize_identity_maps() is called before the
5-level paging variables are setup in choose_random_location(), which
will not work if 5-level paging is actually enabled.

Fix this by moving the initialization of __pgtable_l5_enabled,
pgdir_shift and ptrs_per_p4d into cleanup_trampoline(), which is called
immediately after the finalization of whether the kernel is executing
with 4- or 5-level paging. This will be earlier than anything that might
require those variables, and keeps the 4- vs 5-level paging code all in
one place.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/ident_map_64.c |  6 ------
 arch/x86/boot/compressed/kaslr.c        |  8 --------
 arch/x86/boot/compressed/pgtable_64.c   | 16 ++++++++++++++++
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index a3613857c532..505d6299b76e 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -34,12 +34,6 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
-#ifdef CONFIG_X86_5LEVEL
-unsigned int __pgtable_l5_enabled;
-unsigned int pgdir_shift = 39;
-unsigned int ptrs_per_p4d = 1;
-#endif
-
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 489fabc051d7..9eabd8bc7673 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -835,14 +835,6 @@ void choose_random_location(unsigned long input,
 		return;
 	}
 
-#ifdef CONFIG_X86_5LEVEL
-	if (__read_cr4() & X86_CR4_LA57) {
-		__pgtable_l5_enabled = 1;
-		pgdir_shift = 48;
-		ptrs_per_p4d = 512;
-	}
-#endif
-
 	boot_params->hdr.loadflags |= KASLR_FLAG;
 
 	if (IS_ENABLED(CONFIG_X86_32))
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 46d761f7faa6..0976c2d2ab2f 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -9,6 +9,13 @@
 #define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
 #define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
 
+#ifdef CONFIG_X86_5LEVEL
+/* __pgtable_l5_enabled needs to be in .data to avoid being cleared along with .bss */
+unsigned int __section(.data) __pgtable_l5_enabled;
+unsigned int __section(.data) pgdir_shift = 39;
+unsigned int __section(.data) ptrs_per_p4d = 1;
+#endif
+
 struct paging_config {
 	unsigned long trampoline_start;
 	unsigned long l5_required;
@@ -195,4 +202,13 @@ void cleanup_trampoline(void *pgtable)
 
 	/* Restore trampoline memory */
 	memcpy(trampoline_32bit, trampoline_save, TRAMPOLINE_32BIT_SIZE);
+
+	/* Initialize variables for 5-level paging */
+#ifdef CONFIG_X86_5LEVEL
+	if (__read_cr4() & X86_CR4_LA57) {
+		__pgtable_l5_enabled = 1;
+		pgdir_shift = 48;
+		ptrs_per_p4d = 512;
+	}
+#endif
 }
-- 
2.26.2

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Arvind Sankar]

On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> Commit
>   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> started using a new set of pagetables even without KASLR.
> 
> After that commit, initialize_identity_maps() is called before the
> 5-level paging variables are setup in choose_random_location(), which
> will not work if 5-level paging is actually enabled.

Note that I don't have hardware that supports 5-level paging, so this
is not actually tested with 5-level, but based on code inspection, it
shouldn't work.

> 
> Fix this by moving the initialization of __pgtable_l5_enabled,
> pgdir_shift and ptrs_per_p4d into cleanup_trampoline(), which is called
> immediately after the finalization of whether the kernel is executing
> with 4- or 5-level paging. This will be earlier than anything that might
> require those variables, and keeps the 4- vs 5-level paging code all in
> one place.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/ident_map_64.c |  6 ------
>  arch/x86/boot/compressed/kaslr.c        |  8 --------
>  arch/x86/boot/compressed/pgtable_64.c   | 16 ++++++++++++++++
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
> index a3613857c532..505d6299b76e 100644
> --- a/arch/x86/boot/compressed/ident_map_64.c
> +++ b/arch/x86/boot/compressed/ident_map_64.c
> @@ -34,12 +34,6 @@
>  #define __PAGE_OFFSET __PAGE_OFFSET_BASE
>  #include "../../mm/ident_map.c"
>  
> -#ifdef CONFIG_X86_5LEVEL
> -unsigned int __pgtable_l5_enabled;
> -unsigned int pgdir_shift = 39;
> -unsigned int ptrs_per_p4d = 1;
> -#endif
> -
>  /* Used by PAGE_KERN* macros: */
>  pteval_t __default_kernel_pte_mask __read_mostly = ~0;
>  
> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
> index 489fabc051d7..9eabd8bc7673 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -835,14 +835,6 @@ void choose_random_location(unsigned long input,
>  		return;
>  	}
>  
> -#ifdef CONFIG_X86_5LEVEL
> -	if (__read_cr4() & X86_CR4_LA57) {
> -		__pgtable_l5_enabled = 1;
> -		pgdir_shift = 48;
> -		ptrs_per_p4d = 512;
> -	}
> -#endif
> -
>  	boot_params->hdr.loadflags |= KASLR_FLAG;
>  
>  	if (IS_ENABLED(CONFIG_X86_32))
> diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> index 46d761f7faa6..0976c2d2ab2f 100644
> --- a/arch/x86/boot/compressed/pgtable_64.c
> +++ b/arch/x86/boot/compressed/pgtable_64.c
> @@ -9,6 +9,13 @@
>  #define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
>  #define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
>  
> +#ifdef CONFIG_X86_5LEVEL
> +/* __pgtable_l5_enabled needs to be in .data to avoid being cleared along with .bss */
> +unsigned int __section(.data) __pgtable_l5_enabled;
> +unsigned int __section(.data) pgdir_shift = 39;
> +unsigned int __section(.data) ptrs_per_p4d = 1;
> +#endif
> +
>  struct paging_config {
>  	unsigned long trampoline_start;
>  	unsigned long l5_required;
> @@ -195,4 +202,13 @@ void cleanup_trampoline(void *pgtable)
>  
>  	/* Restore trampoline memory */
>  	memcpy(trampoline_32bit, trampoline_save, TRAMPOLINE_32BIT_SIZE);
> +
> +	/* Initialize variables for 5-level paging */
> +#ifdef CONFIG_X86_5LEVEL
> +	if (__read_cr4() & X86_CR4_LA57) {
> +		__pgtable_l5_enabled = 1;
> +		pgdir_shift = 48;
> +		ptrs_per_p4d = 512;
> +	}
> +#endif
>  }
> -- 
> 2.26.2
> 

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Kirill A. Shutemov]

On Sat, Oct 10, 2020 at 03:26:24PM -0400, Arvind Sankar wrote:
> On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> > Commit
> >   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> > started using a new set of pagetables even without KASLR.
> > 
> > After that commit, initialize_identity_maps() is called before the
> > 5-level paging variables are setup in choose_random_location(), which
> > will not work if 5-level paging is actually enabled.
> 
> Note that I don't have hardware that supports 5-level paging, so this
> is not actually tested with 5-level, but based on code inspection, it
> shouldn't work.

qemu supports it. -cpu "qemu64,+la57"

-- 
 Kirill A. Shutemov

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Arvind Sankar]

On Mon, Oct 12, 2020 at 05:08:30PM +0300, Kirill A. Shutemov wrote:
> On Sat, Oct 10, 2020 at 03:26:24PM -0400, Arvind Sankar wrote:
> > On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> > > Commit
> > >   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> > > started using a new set of pagetables even without KASLR.
> > > 
> > > After that commit, initialize_identity_maps() is called before the
> > > 5-level paging variables are setup in choose_random_location(), which
> > > will not work if 5-level paging is actually enabled.
> > 
> > Note that I don't have hardware that supports 5-level paging, so this
> > is not actually tested with 5-level, but based on code inspection, it
> > shouldn't work.
> 
> qemu supports it. -cpu "qemu64,+la57"
> 
> -- 
>  Kirill A. Shutemov

Thanks! On QEMU, it does crash without this patch.

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Borislav Petkov]

On Mon, Oct 12, 2020 at 11:35:01AM -0400, Arvind Sankar wrote:
> > qemu supports it. -cpu "qemu64,+la57"
>
> Thanks! On QEMU, it does crash without this patch.

Works fine here. I gave this to qemu:

-cpu qemu64,+la57,vendor=GenuineIntel

it said:

qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:ECX.la57 [bit 16]

because host is AMD, probably, and I have

CONFIG_X86_5LEVEL=y

enabled for the guest kernel and tip:x86/seves booted fine.

What am I missing?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Kirill A. Shutemov]

On Tue, Oct 13, 2020 at 10:11:17AM +0200, Borislav Petkov wrote:
> On Mon, Oct 12, 2020 at 11:35:01AM -0400, Arvind Sankar wrote:
> > > qemu supports it. -cpu "qemu64,+la57"
> >
> > Thanks! On QEMU, it does crash without this patch.
> 
> Works fine here. I gave this to qemu:
> 
> -cpu qemu64,+la57,vendor=GenuineIntel
> 
> it said:
> 
> qemu-system-x86_64: warning: host doesn't support requested feature: CPUID.07H:ECX.la57 [bit 16]
> 
> because host is AMD, probably, and I have
> 
> CONFIG_X86_5LEVEL=y
> 
> enabled for the guest kernel and tip:x86/seves booted fine.
> 
> What am I missing?

With TCG or KVM? I use -machine "type=q35,accel=tcg".

-- 
 Kirill A. Shutemov

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Borislav Petkov]

On Tue, Oct 13, 2020 at 11:20:47AM +0300, Kirill A. Shutemov wrote:
> With TCG or KVM? I use -machine "type=q35,accel=tcg".

Thx, that triggered it - it was KVM before.

Btw, are 5level boxes shipping already or not yet? Because I've not seen
one yet. If you have access to the hw, I'd appreciate it if you could
test Arvind's patch ontop of

https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/seves

which does that early pagetable rework.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Joerg Roedel]

Hi Arvind,

On Sat, Oct 10, 2020 at 03:11:10PM -0400, Arvind Sankar wrote:
> Commit
>   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> started using a new set of pagetables even without KASLR.
> 
> After that commit, initialize_identity_maps() is called before the
> 5-level paging variables are setup in choose_random_location(), which
> will not work if 5-level paging is actually enabled.
> 
> Fix this by moving the initialization of __pgtable_l5_enabled,
> pgdir_shift and ptrs_per_p4d into cleanup_trampoline(), which is called
> immediately after the finalization of whether the kernel is executing
> with 4- or 5-level paging. This will be earlier than anything that might
> require those variables, and keeps the 4- vs 5-level paging code all in
> one place.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/ident_map_64.c |  6 ------
>  arch/x86/boot/compressed/kaslr.c        |  8 --------
>  arch/x86/boot/compressed/pgtable_64.c   | 16 ++++++++++++++++
>  3 files changed, 16 insertions(+), 14 deletions(-)

Thanks for fixing this! It is not only a fix but also a nice cleanup of
the 5level-paging initialization code.

Reviewed-by: Joerg Roedel <jroedel@suse.de>
Tested-by: Joerg Roedel <jroedel@suse.de>

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Kirill A. Shutemov]

On Tue, Oct 13, 2020 at 10:33:58AM +0200, Borislav Petkov wrote:
> On Tue, Oct 13, 2020 at 11:20:47AM +0300, Kirill A. Shutemov wrote:
> > With TCG or KVM? I use -machine "type=q35,accel=tcg".
> 
> Thx, that triggered it - it was KVM before.
> 
> Btw, are 5level boxes shipping already or not yet?

Not yet.

> Because I've not seen one yet. If you have access to the hw, I'd
> appreciate it if you could test Arvind's patch ontop of
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/seves
> 
> which does that early pagetable rework.

Okay, will do.

-- 
 Kirill A. Shutemov

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Borislav Petkov]

On Tue, Oct 13, 2020 at 12:12:28PM +0300, Kirill A. Shutemov wrote:
> Not yet.

Oh good, that makes this fix a lot less critical then.

> Okay, will do.

Thx!

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Kirill A. Shutemov]

On Tue, Oct 13, 2020 at 10:33:58AM +0200, Borislav Petkov wrote:
> On Tue, Oct 13, 2020 at 11:20:47AM +0300, Kirill A. Shutemov wrote:
> > With TCG or KVM? I use -machine "type=q35,accel=tcg".
> 
> Thx, that triggered it - it was KVM before.
> 
> Btw, are 5level boxes shipping already or not yet? Because I've not seen
> one yet. If you have access to the hw, I'd appreciate it if you could
> test Arvind's patch ontop of
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=x86/seves
> 
> which does that early pagetable rework.

Yes, the patch helps to fix 5-level paging boot:

Tested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [PATCH] x86/boot/64: Initialize 5-level paging variables earlier

[Borislav Petkov]

On Thu, Oct 15, 2020 at 04:52:09PM +0300, Kirill A. Shutemov wrote:
> Yes, the patch helps to fix 5-level paging boot:
> 
> Tested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Thx!

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Borislav Petkov]

On Thu, Oct 08, 2020 at 03:16:23PM -0400, Arvind Sankar wrote:
> On 64-bit, the startup_64_setup_env() function added in
>   866b556efa12 ("x86/head/64: Install startup GDT")
> has stack protection enabled because of set_bringup_idt_handler().

Where? I don't see it.

I have

CONFIG_STACKPROTECTOR=y
# CONFIG_STACKPROTECTOR_STRONG is not set

and a __stack_chk_fail call is nowhere to be found in the resulting
head64.s file.

startup_64_setup_env:
# arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
	leaq	startup_gdt(%rdi), %rax	#, tmp99
# arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
	subq	$_text, %rax	#, tmp101
	movq	%rax, startup_gdt_descr+2(%rip)	# tmp101, startup_gdt_descr.address
# ./arch/x86/include/asm/desc.h:209: 	asm volatile("lgdt %0"::"m" (*dtr));
#APP
# 209 "./arch/x86/include/asm/desc.h" 1
	lgdt startup_gdt_descr(%rip)		# startup_gdt_descr
# 0 "" 2
# arch/x86/kernel/head64.c:600: 	asm volatile("movl %%eax, %%ds\n"
#NO_APP
	movl	$24, %eax	#, tmp102
#APP
# 600 "arch/x86/kernel/head64.c" 1
	movl %eax, %ds
movl %eax, %ss
movl %eax, %es

# 0 "" 2
# arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
#NO_APP
	leaq	bringup_idt_table(%rdi), %r9	#, tmp105
	leaq	bringup_idt_descr(%rdi), %r8	#, tmp103
	leaq	vc_no_ghcb(%rdi), %rsi	#, tmp107
# arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
	subq	$_text, %r9	#, _11
	subq	$_text, %r8	#, _8
	subq	$_text, %rsi	#, tmp109
# arch/x86/kernel/head64.c:572: 		set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
	movq	%r9, %rdi	# _11,
	call	set_bringup_idt_handler.constprop.0	#
# arch/x86/kernel/head64.c:575: 	desc->address = (unsigned long)idt;
	movq	%r9, 2(%r8)	# _11, MEM[(struct desc_ptr *)_8].address
# ./arch/x86/include/asm/desc.h:214: 	asm volatile("lidt %0"::"m" (*dtr));
#APP
# 214 "./arch/x86/include/asm/desc.h" 1
	lidt (%r8)		# MEM[(const struct desc_ptr *)_8]
# 0 "" 2
# arch/x86/kernel/head64.c:605: }
#NO_APP
	ret

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Arvind Sankar]

On Fri, Oct 16, 2020 at 01:17:03PM +0200, Borislav Petkov wrote:
> On Thu, Oct 08, 2020 at 03:16:23PM -0400, Arvind Sankar wrote:
> > On 64-bit, the startup_64_setup_env() function added in
> >   866b556efa12 ("x86/head/64: Install startup GDT")
> > has stack protection enabled because of set_bringup_idt_handler().
> 
> Where? I don't see it.
> 
> I have
> 
> CONFIG_STACKPROTECTOR=y
> # CONFIG_STACKPROTECTOR_STRONG is not set

You need STACKPROTECTOR_STRONG -- I was testing with defconfig and the
option is enabled by default. You also need AMD_MEM_ENCRYPT enabled,
which it looks like you do.

> 
> and a __stack_chk_fail call is nowhere to be found in the resulting
> head64.s file.
> 
> startup_64_setup_env:
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	leaq	startup_gdt(%rdi), %rax	#, tmp99
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	subq	$_text, %rax	#, tmp101
> 	movq	%rax, startup_gdt_descr+2(%rip)	# tmp101, startup_gdt_descr.address
> # ./arch/x86/include/asm/desc.h:209: 	asm volatile("lgdt %0"::"m" (*dtr));
> #APP
> # 209 "./arch/x86/include/asm/desc.h" 1
> 	lgdt startup_gdt_descr(%rip)		# startup_gdt_descr
> # 0 "" 2
> # arch/x86/kernel/head64.c:600: 	asm volatile("movl %%eax, %%ds\n"
> #NO_APP
> 	movl	$24, %eax	#, tmp102
> #APP
> # 600 "arch/x86/kernel/head64.c" 1
> 	movl %eax, %ds
> movl %eax, %ss
> movl %eax, %es
> 
> # 0 "" 2
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> #NO_APP
> 	leaq	bringup_idt_table(%rdi), %r9	#, tmp105
> 	leaq	bringup_idt_descr(%rdi), %r8	#, tmp103
> 	leaq	vc_no_ghcb(%rdi), %rsi	#, tmp107
> # arch/x86/kernel/head64.c:91: 	return ptr - (void *)_text + (void *)physaddr;
> 	subq	$_text, %r9	#, _11
> 	subq	$_text, %r8	#, _8
> 	subq	$_text, %rsi	#, tmp109
> # arch/x86/kernel/head64.c:572: 		set_bringup_idt_handler(idt, X86_TRAP_VC, handler);
> 	movq	%r9, %rdi	# _11,
> 	call	set_bringup_idt_handler.constprop.0	#
> # arch/x86/kernel/head64.c:575: 	desc->address = (unsigned long)idt;
> 	movq	%r9, 2(%r8)	# _11, MEM[(struct desc_ptr *)_8].address
> # ./arch/x86/include/asm/desc.h:214: 	asm volatile("lidt %0"::"m" (*dtr));
> #APP
> # 214 "./arch/x86/include/asm/desc.h" 1
> 	lidt (%r8)		# MEM[(const struct desc_ptr *)_8]
> # 0 "" 2
> # arch/x86/kernel/head64.c:605: }
> #NO_APP
> 	ret
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Borislav Petkov]

On Fri, Oct 16, 2020 at 08:43:01AM -0400, Arvind Sankar wrote:
> You need STACKPROTECTOR_STRONG -- I was testing with defconfig and the
> option is enabled by default.

And you need to write those things in the commit messages.

Please, for the future, always make sure that all required ingredients
for triggering a bug are documented in the commit message, before
sending a fix. Jörg and I were both scratching heads on how you're
reproducing this.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 5/5] x86/head/64: Disable stack protection for head$(BITS).o

[Arvind Sankar]

On Fri, Oct 16, 2020 at 03:15:45PM +0200, Borislav Petkov wrote:
> On Fri, Oct 16, 2020 at 08:43:01AM -0400, Arvind Sankar wrote:
> > You need STACKPROTECTOR_STRONG -- I was testing with defconfig and the
> > option is enabled by default.
> 
> And you need to write those things in the commit messages.
> 
> Please, for the future, always make sure that all required ingredients
> for triggering a bug are documented in the commit message, before
> sending a fix. Jörg and I were both scratching heads on how you're
> reproducing this.
> 
> Thx.
> 

Sorry about that. This config option I didn't notice before since I
hadn't explicitly enabled the option, but I should have been more clear
about how I reproduced. I actually came across this from the other end,
was looking at the disassembly, saw the stack check call, and then spent
a while debugging why it did _not_ just always crash.

Thanks.

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Thu, Oct 08, 2020 at 03:16:22PM -0400, Arvind Sankar wrote:
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/ident_map_64.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)

Ok, just pushed your two fixes here:

https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc0%2b1-seves

Please rebase this one ontop and put the two cleanups last so that the
fixes can go into urgent while the two cleanups can follow later through
normal tip flow. And yes, pls do this in the future too: fixes should
be minimal so that they can be expedited first and normal cleanups and
features can be done later.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

On Fri, Oct 16, 2020 at 06:27:59PM +0200, Borislav Petkov wrote:
> On Thu, Oct 08, 2020 at 03:16:22PM -0400, Arvind Sankar wrote:
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > ---
> >  arch/x86/boot/compressed/ident_map_64.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> Ok, just pushed your two fixes here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/log/?h=rc0%2b1-seves
> 
> Please rebase this one ontop and put the two cleanups last so that the
> fixes can go into urgent while the two cleanups can follow later through
> normal tip flow. And yes, pls do this in the future too: fixes should
> be minimal so that they can be expedited first and normal cleanups and
> features can be done later.
> 

Just for clarity, by cleanups you mean patches 2 and 3? i.e. you want to
see 1, 4, 2, 3?

Thanks.

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Fri, Oct 16, 2020 at 12:47:55PM -0400, Arvind Sankar wrote:
> Just for clarity, by cleanups you mean patches 2 and 3? i.e. you want to
> see 1, 4, 2, 3?

It is important for:

[PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

to come first so that it goes in now.

The rest:

[PATCH v2 1/5] x86/boot: Initialize boot_params in startup code
[PATCH v2 2/5] x86/boot: Split out command-line related declarations
[PATCH v2 3/5] x86/boot/64: Show original faulting address in case of error

can come in any order and when ready.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

On Fri, Oct 16, 2020 at 07:07:42PM +0200, Borislav Petkov wrote:
> On Fri, Oct 16, 2020 at 12:47:55PM -0400, Arvind Sankar wrote:
> > Just for clarity, by cleanups you mean patches 2 and 3? i.e. you want to
> > see 1, 4, 2, 3?
> 
> It is important for:
> 
> [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line
> 
> to come first so that it goes in now.

This patch depends on 1 to initialize boot_params before
initialize_identity_maps() is called. You want me to rework it to avoid
that?

> 
> The rest:
> 
> [PATCH v2 1/5] x86/boot: Initialize boot_params in startup code
> [PATCH v2 2/5] x86/boot: Split out command-line related declarations
> [PATCH v2 3/5] x86/boot/64: Show original faulting address in case of error
> 
> can come in any order and when ready.
> 
> Thx.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Fri, Oct 16, 2020 at 01:20:58PM -0400, Arvind Sankar wrote:
> This patch depends on 1 to initialize boot_params before
> initialize_identity_maps() is called. You want me to rework it to avoid
> that?

Yes please. If it doesn't become too ugly, that is. If it does, then
we'll have to think of something else...

Thx.

-- 
Regards/Gruss,
    Boris.

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

[PATCH v3 1/4] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

Commits

  ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
  8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")

set up a new page table in the decompressor stub, but without explicit
mappings for boot_params and the kernel command line, relying on the #PF
handler instead.

This is fragile, as boot_params and the command line mappings are
required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
disabled, a QEMU/OVMF boot never accesses the command line in the
decompressor stub, and so it never gets mapped. The main kernel accesses
it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
crash.

Fix this by adding back the explicit mapping of boot_params and the
command line.

Note: the changes also removed the explicit mapping of the main kernel,
with the result that .bss and .brk may not be in the identity mapping,
but those don't get accessed by the main kernel before it switches to
its own page tables.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/boot/compressed/head_64.S      |  3 +++
 arch/x86/boot/compressed/ident_map_64.c | 24 +++++++++++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1c80f1738fd9..3976b4e92e1b 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -544,6 +544,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushq	%rsi
 	call	set_sev_encryption_mask
 	call	load_stage2_idt
+	/* Pass boot_params to initialize_identity_maps */
+	popq	%rdi
+	pushq	%rdi
 	call	initialize_identity_maps
 	popq	%rsi
 
diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index c6f7aef7e85a..bf61581277c2 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -33,6 +33,12 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
+#define _SETUP
+#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
+#undef _SETUP
+
+extern unsigned long get_cmd_line_ptr(void);
+
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
@@ -101,8 +107,10 @@ static void add_identity_map(unsigned long start, unsigned long end)
 }
 
 /* Locates and clears a region for a new top level page table. */
-void initialize_identity_maps(void)
+void initialize_identity_maps(void *rmode)
 {
+	unsigned long cmdline;
+
 	/* Exclude the encryption mask from __PHYSICAL_MASK */
 	physical_mask &= ~sme_me_mask;
 
@@ -143,10 +151,20 @@ void initialize_identity_maps(void)
 	}
 
 	/*
-	 * New page-table is set up - map the kernel image and load it
-	 * into cr3.
+	 * New page-table is set up - map the kernel image, boot_params and the
+	 * command line.
+	 * The uncompressed kernel requires boot_params and the command line to
+	 * be mapped in the identity mapping.
+	 * Map them explicitly here in case the compressed kernel does not
+	 * touch them, or does not touch all the pages covering them.
 	 */
 	add_identity_map((unsigned long)_head, (unsigned long)_end);
+	boot_params = rmode;
+	add_identity_map((unsigned long)boot_params, (unsigned long)(boot_params + 1));
+	cmdline = get_cmd_line_ptr();
+	add_identity_map(cmdline, cmdline + COMMAND_LINE_SIZE);
+
+	/* Load the new page-table. */
 	write_cr3(top_level_pgt);
 }
 
-- 
2.26.2

[PATCH v3 2/4] x86/boot: Initialize boot_params in startup code

[Arvind Sankar]

Save the boot_params pointer passed in by the bootloader in
startup_32/64. This avoids having to initialize it in multiple places in
C code, and having to preserve SI through the early assembly code.

Move boot_params from .bss to .data, since .bss will be cleared after
the kernel is relocated.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/boot/compressed/head_32.S      | 12 ++++----
 arch/x86/boot/compressed/head_64.S      | 38 ++++++++-----------------
 arch/x86/boot/compressed/ident_map_64.c |  3 +-
 arch/x86/boot/compressed/misc.c         | 10 +------
 arch/x86/boot/compressed/pgtable_64.c   |  5 +---
 5 files changed, 22 insertions(+), 46 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 659fad53ca82..9b5c88d8b290 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -113,6 +113,9 @@ SYM_FUNC_START(startup_32)
 	addl    BP_init_size(%esi), %ebx
 	subl    $_end@GOTOFF, %ebx
 
+	/* Initialize boot_params */
+	movl	%esi, boot_params@GOTOFF(%edx)
+
 	/* Set up the stack */
 	leal	boot_stack_end@GOTOFF(%ebx), %esp
 
@@ -124,7 +127,6 @@ SYM_FUNC_START(startup_32)
  * Copy the compressed kernel to the end of our buffer
  * where decompression in place becomes safe.
  */
-	pushl	%esi
 	leal	(_bss@GOTOFF-4)(%edx), %esi
 	leal	(_bss@GOTOFF-4)(%ebx), %edi
 	movl	$(_bss - startup_32), %ecx
@@ -132,7 +134,6 @@ SYM_FUNC_START(startup_32)
 	std
 	rep	movsl
 	cld
-	popl	%esi
 
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
@@ -187,14 +188,12 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushl	%eax			/* input_data */
 	leal	boot_heap@GOTOFF(%ebx), %eax
 	pushl	%eax			/* heap area */
-	pushl	%esi			/* real mode pointer */
 	call	extract_kernel		/* returns kernel location in %eax */
-	addl	$24, %esp
 
 /*
  * Jump to the extracted kernel.
  */
-	xorl	%ebx, %ebx
+	movl	boot_params@GOTOFF(%ebx), %esi
 	jmp	*%eax
 SYM_FUNC_END(.Lrelocated)
 
@@ -209,6 +208,9 @@ SYM_DATA_START_LOCAL(gdt)
 	.quad	0x00cf92000000ffff	/* __KERNEL_DS */
 SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end)
 
+/* boot_params must be in .data because .bss is cleared after the kernel is relocated */
+SYM_DATA(boot_params, .long 0)
+
 #ifdef CONFIG_EFI_STUB
 SYM_DATA(image_offset, .long 0)
 #endif
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 3976b4e92e1b..521b41ca14fe 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -375,6 +375,9 @@ SYM_CODE_START(startup_64)
 	subl	$ rva(_end), %ebx
 	addq	%rbp, %rbx
 
+	/* Initialize boot_params */
+	movq	%rsi, boot_params(%rip)
+
 	/* Set up the stack */
 	leaq	rva(boot_stack_end)(%rbx), %rsp
 
@@ -429,14 +432,8 @@ SYM_CODE_START(startup_64)
 	 *   - Address of the trampoline is returned in RAX.
 	 *   - Non zero RDX means trampoline needs to enable 5-level
 	 *     paging.
-	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
-	movq	%rsi, %rdi		/* real mode address */
 	call	paging_prepare
-	popq	%rsi
 
 	/* Save the trampoline address in RCX */
 	movq	%rax, %rcx
@@ -461,14 +458,9 @@ trampoline_return:
 	 *
 	 * RDI is address of the page table to use instead of page table
 	 * in trampoline memory (if required).
-	 *
-	 * RSI holds real mode data and needs to be preserved across
-	 * this function call.
 	 */
-	pushq	%rsi
 	leaq	rva(top_pgtable)(%rbx), %rdi
 	call	cleanup_trampoline
-	popq	%rsi
 
 	/* Zero EFLAGS */
 	pushq	$0
@@ -478,7 +470,6 @@ trampoline_return:
  * Copy the compressed kernel to the end of our buffer
  * where decompression in place becomes safe.
  */
-	pushq	%rsi
 	leaq	(_bss-8)(%rip), %rsi
 	leaq	rva(_bss-8)(%rbx), %rdi
 	movl	$(_bss - startup_32), %ecx
@@ -486,7 +477,6 @@ trampoline_return:
 	std
 	rep	movsq
 	cld
-	popq	%rsi
 
 	/*
 	 * The GDT may get overwritten either during the copy we just did or
@@ -541,31 +531,24 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
  * handler. Then load stage2 IDT and switch to the kernel's own
  * page-table.
  */
-	pushq	%rsi
 	call	set_sev_encryption_mask
 	call	load_stage2_idt
-	/* Pass boot_params to initialize_identity_maps */
-	popq	%rdi
-	pushq	%rdi
 	call	initialize_identity_maps
-	popq	%rsi
 
 /*
  * Do the extraction, and jump to the new kernel..
  */
-	pushq	%rsi			/* Save the real mode argument */
-	movq	%rsi, %rdi		/* real mode address */
-	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
-	leaq	input_data(%rip), %rdx  /* input_data */
-	movl	input_len(%rip), %ecx	/* input_len */
-	movq	%rbp, %r8		/* output target address */
-	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
+	leaq	boot_heap(%rip), %rdi	/* malloc area for uncompression */
+	leaq	input_data(%rip), %rsi  /* input_data */
+	movl	input_len(%rip), %edx	/* input_len */
+	movq	%rbp, %rcx		/* output target address */
+	movl	output_len(%rip), %r8d	/* decompressed length, end of relocs */
 	call	extract_kernel		/* returns kernel location in %rax */
-	popq	%rsi
 
 /*
  * Jump to the decompressed kernel.
  */
+	movq	boot_params(%rip), %rsi
 	jmp	*%rax
 SYM_FUNC_END(.Lrelocated)
 
@@ -694,6 +677,9 @@ SYM_DATA_START(boot_idt)
 	.endr
 SYM_DATA_END_LABEL(boot_idt, SYM_L_GLOBAL, boot_idt_end)
 
+/* boot_params must be in .data because .bss is cleared after the kernel is relocated */
+SYM_DATA(boot_params, .quad 0)
+
 #ifdef CONFIG_EFI_STUB
 SYM_DATA(image_offset, .long 0)
 #endif
diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index bf61581277c2..b679908c120e 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -107,7 +107,7 @@ static void add_identity_map(unsigned long start, unsigned long end)
 }
 
 /* Locates and clears a region for a new top level page table. */
-void initialize_identity_maps(void *rmode)
+void initialize_identity_maps(void)
 {
 	unsigned long cmdline;
 
@@ -159,7 +159,6 @@ void initialize_identity_maps(void *rmode)
 	 * touch them, or does not touch all the pages covering them.
 	 */
 	add_identity_map((unsigned long)_head, (unsigned long)_end);
-	boot_params = rmode;
 	add_identity_map((unsigned long)boot_params, (unsigned long)(boot_params + 1));
 	cmdline = get_cmd_line_ptr();
 	add_identity_map(cmdline, cmdline + COMMAND_LINE_SIZE);
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 267e7f93050e..279631650bd8 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -39,11 +39,6 @@
 /* Functions used by the included decompressor code below. */
 void *memmove(void *dest, const void *src, size_t n);
 
-/*
- * This is set up by the setup-routine at boot-time
- */
-struct boot_params *boot_params;
-
 memptr free_mem_ptr;
 memptr free_mem_end_ptr;
 
@@ -338,7 +333,7 @@ static void parse_elf(void *output)
  *             |-------uncompressed kernel image---------|
  *
  */
-asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
+asmlinkage __visible void *extract_kernel(memptr heap,
 				  unsigned char *input_data,
 				  unsigned long input_len,
 				  unsigned char *output,
@@ -348,9 +343,6 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 	unsigned long virt_addr = LOAD_PHYSICAL_ADDR;
 	unsigned long needed_size;
 
-	/* Retain x86 boot parameters pointer passed from startup_32/64. */
-	boot_params = rmode;
-
 	/* Clear flags intended for solely in-kernel use. */
 	boot_params->hdr.loadflags &= ~KASLR_FLAG;
 
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 5def1674d6f1..25add5510edc 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -105,13 +105,10 @@ static unsigned long find_trampoline_placement(void)
 	return bios_start - TRAMPOLINE_32BIT_SIZE;
 }
 
-struct paging_config paging_prepare(void *rmode)
+struct paging_config paging_prepare(void)
 {
 	struct paging_config paging_config = {};
 
-	/* Initialize boot_params. Required for cmdline_find_option_bool(). */
-	boot_params = rmode;
-
 	/*
 	 * Check if LA57 is desired and supported.
 	 *
-- 
2.26.2

[PATCH v3 3/4] x86/boot: Split out command-line related declarations

[Arvind Sankar]

Move prototypes for command-line related functions into a new header
file to split it out from misc.h.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/acpi.c                 |  1 +
 arch/x86/boot/compressed/cmdline.c              |  1 +
 arch/x86/boot/compressed/cmdline.h              | 13 +++++++++++++
 arch/x86/boot/compressed/early_serial_console.c |  1 +
 arch/x86/boot/compressed/ident_map_64.c         |  7 +------
 arch/x86/boot/compressed/kaslr.c                |  7 +------
 arch/x86/boot/compressed/misc.h                 |  4 ----
 arch/x86/boot/compressed/pgtable_64.c           |  2 +-
 8 files changed, 19 insertions(+), 17 deletions(-)
 create mode 100644 arch/x86/boot/compressed/cmdline.h

diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
index 8bcbcee54aa1..9097108c37e1 100644
--- a/arch/x86/boot/compressed/acpi.c
+++ b/arch/x86/boot/compressed/acpi.c
@@ -3,6 +3,7 @@
 #include "misc.h"
 #include "error.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #include <linux/numa.h>
 #include <linux/efi.h>
diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c
index f1add5d85da9..20f2e6d8b891 100644
--- a/arch/x86/boot/compressed/cmdline.c
+++ b/arch/x86/boot/compressed/cmdline.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include "misc.h"
+#include "cmdline.h"
 
 static unsigned long fs;
 static inline void set_fs(unsigned long seg)
diff --git a/arch/x86/boot/compressed/cmdline.h b/arch/x86/boot/compressed/cmdline.h
new file mode 100644
index 000000000000..72800770bd60
--- /dev/null
+++ b/arch/x86/boot/compressed/cmdline.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef BOOT_COMPRESSED_CMDLINE_H
+#define BOOT_COMPRESSED_CMDLINE_H
+
+#define _SETUP
+#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
+#undef _SETUP
+
+unsigned long get_cmd_line_ptr(void);
+int cmdline_find_option(const char *option, char *buffer, int bufsize);
+int cmdline_find_option_bool(const char *option);
+
+#endif /* BOOT_COMPRESSED_CMDLINE_H */
diff --git a/arch/x86/boot/compressed/early_serial_console.c b/arch/x86/boot/compressed/early_serial_console.c
index 261e81fb9582..64a1f557e122 100644
--- a/arch/x86/boot/compressed/early_serial_console.c
+++ b/arch/x86/boot/compressed/early_serial_console.c
@@ -1,4 +1,5 @@
 #include "misc.h"
+#include "cmdline.h"
 
 int early_serial_base;
 
diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index b679908c120e..06ebe5e3e489 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -21,6 +21,7 @@
 
 #include "error.h"
 #include "misc.h"
+#include "cmdline.h"
 
 /* These actually do the work of building the kernel identity maps. */
 #include <linux/pgtable.h>
@@ -33,12 +34,6 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
-#define _SETUP
-#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
-#undef _SETUP
-
-extern unsigned long get_cmd_line_ptr(void);
-
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b92fffbe761f..9eabd8bc7673 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -22,6 +22,7 @@
 #include "misc.h"
 #include "error.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #include <generated/compile.h>
 #include <linux/module.h>
@@ -36,12 +37,6 @@
 #define STATIC
 #include <linux/decompress/mm.h>
 
-#define _SETUP
-#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
-#undef _SETUP
-
-extern unsigned long get_cmd_line_ptr(void);
-
 /* Simplified build-specific string for starting entropy. */
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index 6d31f1b4c4d1..e3e2f312c025 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -69,10 +69,6 @@ static inline void debug_puthex(unsigned long value)
 
 #endif
 
-/* cmdline.c */
-int cmdline_find_option(const char *option, char *buffer, int bufsize);
-int cmdline_find_option_bool(const char *option);
-
 struct mem_vector {
 	u64 start;
 	u64 size;
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 25add5510edc..0976c2d2ab2f 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -4,6 +4,7 @@
 #include <asm/efi.h>
 #include "pgtable.h"
 #include "../string.h"
+#include "cmdline.h"
 
 #define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
 #define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
@@ -33,7 +34,6 @@ static char trampoline_save[TRAMPOLINE_32BIT_SIZE];
 unsigned long *trampoline_32bit __section(.data);
 
 extern struct boot_params *boot_params;
-int cmdline_find_option_bool(const char *option);
 
 static unsigned long find_trampoline_placement(void)
 {
-- 
2.26.2

[PATCH v3 4/4] x86/boot/64: Show original faulting address in case of error

[Arvind Sankar]

This makes the #PF handler print the original CR2 value in case of
error, instead of after aligning to PMD_SIZE.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
---
 arch/x86/boot/compressed/ident_map_64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index 06ebe5e3e489..505d6299b76e 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -333,9 +333,6 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
 
 	ghcb_fault = sev_es_check_ghcb_fault(address);
 
-	address   &= PMD_MASK;
-	end        = address + PMD_SIZE;
-
 	/*
 	 * Check for unexpected error codes. Unexpected are:
 	 *	- Faults on present pages
@@ -351,5 +348,8 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
 	 * Error code is sane - now identity map the 2M region around
 	 * the faulting address.
 	 */
+	address   &= PMD_MASK;
+	end        = address + PMD_SIZE;
+
 	add_identity_map(address, end);
 }
-- 
2.26.2

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

On Fri, Oct 16, 2020 at 07:32:32PM +0200, Borislav Petkov wrote:
> On Fri, Oct 16, 2020 at 01:20:58PM -0400, Arvind Sankar wrote:
> > This patch depends on 1 to initialize boot_params before
> > initialize_identity_maps() is called. You want me to rework it to avoid
> > that?
> 
> Yes please. If it doesn't become too ugly, that is. If it does, then
> we'll have to think of something else...
> 
> Thx.
> 

I just sent out v3, with the patches reordered. Maybe just pick up the
fix for now, and the other three patches I can resend sometime after the
merge window, along with other cleanups I've been working on.

Thanks.

Re: [PATCH v2 4/5] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Fri, Oct 16, 2020 at 05:18:41PM -0400, Arvind Sankar wrote:
> I just sent out v3, with the patches reordered. Maybe just pick up the
> fix for now, and the other three patches I can resend sometime after the
> merge window, along with other cleanups I've been working on.

Sure, however you prefer.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/4] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Fri, Oct 16, 2020 at 04:04:01PM -0400, Arvind Sankar wrote:
> Commits
> 
>   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
>   8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")
> 
> set up a new page table in the decompressor stub, but without explicit
> mappings for boot_params and the kernel command line, relying on the #PF
> handler instead.
> 
> This is fragile, as boot_params and the command line mappings are
> required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
> disabled, a QEMU/OVMF boot never accesses the command line in the
> decompressor stub, and so it never gets mapped. The main kernel accesses
> it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
> crash.
> 
> Fix this by adding back the explicit mapping of boot_params and the
> command line.
> 
> Note: the changes also removed the explicit mapping of the main kernel,
> with the result that .bss and .brk may not be in the identity mapping,
> but those don't get accessed by the main kernel before it switches to
> its own page tables.
> 
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> Reviewed-by: Joerg Roedel <jroedel@suse.de>
> ---
>  arch/x86/boot/compressed/head_64.S      |  3 +++
>  arch/x86/boot/compressed/ident_map_64.c | 24 +++++++++++++++++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 1c80f1738fd9..3976b4e92e1b 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -544,6 +544,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
>  	pushq	%rsi
>  	call	set_sev_encryption_mask
>  	call	load_stage2_idt
> +	/* Pass boot_params to initialize_identity_maps */
> +	popq	%rdi
> +	pushq	%rdi

Any reason why you're not doing

	movq    (%rsp), %rdi

here instead?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/4] x86/boot/64: Explicitly map boot_params and command line

[Arvind Sankar]

On Mon, Oct 19, 2020 at 04:51:15PM +0200, Borislav Petkov wrote:
> On Fri, Oct 16, 2020 at 04:04:01PM -0400, Arvind Sankar wrote:
> > Commits
> > 
> >   ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
> >   8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")
> > 
> > set up a new page table in the decompressor stub, but without explicit
> > mappings for boot_params and the kernel command line, relying on the #PF
> > handler instead.
> > 
> > This is fragile, as boot_params and the command line mappings are
> > required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
> > disabled, a QEMU/OVMF boot never accesses the command line in the
> > decompressor stub, and so it never gets mapped. The main kernel accesses
> > it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
> > crash.
> > 
> > Fix this by adding back the explicit mapping of boot_params and the
> > command line.
> > 
> > Note: the changes also removed the explicit mapping of the main kernel,
> > with the result that .bss and .brk may not be in the identity mapping,
> > but those don't get accessed by the main kernel before it switches to
> > its own page tables.
> > 
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Reviewed-by: Joerg Roedel <jroedel@suse.de>
> > ---
> >  arch/x86/boot/compressed/head_64.S      |  3 +++
> >  arch/x86/boot/compressed/ident_map_64.c | 24 +++++++++++++++++++++---
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> > index 1c80f1738fd9..3976b4e92e1b 100644
> > --- a/arch/x86/boot/compressed/head_64.S
> > +++ b/arch/x86/boot/compressed/head_64.S
> > @@ -544,6 +544,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> >  	pushq	%rsi
> >  	call	set_sev_encryption_mask
> >  	call	load_stage2_idt
> > +	/* Pass boot_params to initialize_identity_maps */
> > +	popq	%rdi
> > +	pushq	%rdi
> 
> Any reason why you're not doing
> 
> 	movq    (%rsp), %rdi
> 
> here instead?
> 

No real reason. This will disappear anyway in the cleanup patch.

Re: [PATCH v3 1/4] x86/boot/64: Explicitly map boot_params and command line

[Borislav Petkov]

On Mon, Oct 19, 2020 at 01:12:59PM -0400, Arvind Sankar wrote:
> No real reason. This will disappear anyway in the cleanup patch.

Ok, I'll change it to the MOV as it is less confusing, at least to me
:), this way.

Thx.

-- 
Regards/Gruss,
    Boris.

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

[tip: x86/seves] x86/boot/64: Explicitly map boot_params and command line

[tip-bot2 for Arvind Sankar]

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

Commit-ID:     b17a45b6e53f6613118b2e5cfc4a992cc50deb2c
Gitweb:        https://git.kernel.org/tip/b17a45b6e53f6613118b2e5cfc4a992cc50deb2c
Author:        Arvind Sankar <nivedita@alum.mit.edu>
AuthorDate:    Fri, 16 Oct 2020 16:04:01 -04:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 19 Oct 2020 19:39:50 +02:00

x86/boot/64: Explicitly map boot_params and command line

Commits

  ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
  8570978ea030 ("x86/boot/compressed/64: Don't pre-map memory in KASLR code")

set up a new page table in the decompressor stub, but without explicit
mappings for boot_params and the kernel command line, relying on the #PF
handler instead.

This is fragile, as boot_params and the command line mappings are
required for the main kernel. If EARLY_PRINTK and RANDOMIZE_BASE are
disabled, a QEMU/OVMF boot never accesses the command line in the
decompressor stub, and so it never gets mapped. The main kernel accesses
it from the identity mapping if AMD_MEM_ENCRYPT is enabled, and will
crash.

Fix this by adding back the explicit mapping of boot_params and the
command line.

Note: the changes also removed the explicit mapping of the main kernel,
with the result that .bss and .brk may not be in the identity mapping,
but those don't get accessed by the main kernel before it switches to
its own page tables.

 [ bp: Pass boot_params with a MOV %rsp... instead of PUSH/POP. Use
   block formatting for the comment. ]

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
Link: https://lkml.kernel.org/r/20201016200404.1615994-1-nivedita@alum.mit.edu
---
 arch/x86/boot/compressed/head_64.S      |  3 +++
 arch/x86/boot/compressed/ident_map_64.c | 23 ++++++++++++++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1c80f17..017de6c 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -544,6 +544,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushq	%rsi
 	call	set_sev_encryption_mask
 	call	load_stage2_idt
+
+	/* Pass boot_params to initialize_identity_maps() */
+	movq	(%rsp), %rdi
 	call	initialize_identity_maps
 	popq	%rsi
 
diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index c6f7aef..a5e5db6 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -33,6 +33,12 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
+#define _SETUP
+#include <asm/setup.h>	/* For COMMAND_LINE_SIZE */
+#undef _SETUP
+
+extern unsigned long get_cmd_line_ptr(void);
+
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
@@ -101,8 +107,10 @@ static void add_identity_map(unsigned long start, unsigned long end)
 }
 
 /* Locates and clears a region for a new top level page table. */
-void initialize_identity_maps(void)
+void initialize_identity_maps(void *rmode)
 {
+	unsigned long cmdline;
+
 	/* Exclude the encryption mask from __PHYSICAL_MASK */
 	physical_mask &= ~sme_me_mask;
 
@@ -143,10 +151,19 @@ void initialize_identity_maps(void)
 	}
 
 	/*
-	 * New page-table is set up - map the kernel image and load it
-	 * into cr3.
+	 * New page-table is set up - map the kernel image, boot_params and the
+	 * command line. The uncompressed kernel requires boot_params and the
+	 * command line to be mapped in the identity mapping. Map them
+	 * explicitly here in case the compressed kernel does not touch them,
+	 * or does not touch all the pages covering them.
 	 */
 	add_identity_map((unsigned long)_head, (unsigned long)_end);
+	boot_params = rmode;
+	add_identity_map((unsigned long)boot_params, (unsigned long)(boot_params + 1));
+	cmdline = get_cmd_line_ptr();
+	add_identity_map(cmdline, cmdline + COMMAND_LINE_SIZE);
+
+	/* Load the new page-table. */
 	write_cr3(top_level_pgt);
 }
 

[tip: x86/seves] x86/head/64: Disable stack protection for head$(BITS).o

[tip-bot2 for Arvind Sankar]

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

Commit-ID:     103a4908ad4da9decdf9bc7216ec5a4861edf703
Gitweb:        https://git.kernel.org/tip/103a4908ad4da9decdf9bc7216ec5a4861edf703
Author:        Arvind Sankar <nivedita@alum.mit.edu>
AuthorDate:    Thu, 08 Oct 2020 15:16:23 -04:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 19 Oct 2020 13:11:00 +02:00

x86/head/64: Disable stack protection for head$(BITS).o

On 64-bit, the startup_64_setup_env() function added in

  866b556efa12 ("x86/head/64: Install startup GDT")

has stack protection enabled because of set_bringup_idt_handler().
This happens when CONFIG_STACKPROTECTOR_STRONG is enabled. It
also currently needs CONFIG_AMD_MEM_ENCRYPT enabled because then
set_bringup_idt_handler() is not an empty stub but that might change in
the future, when the other vendor adds their similar technology.

At this point, %gs is not yet initialized, and this doesn't cause a
crash only because the #PF handler from the decompressor stub is still
installed and handles the page fault.

Disable stack protection for the whole file, and do it on 32-bit as
well to avoid surprises.

 [ bp: Extend commit message with the exact explanation how it happens. ]

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
Link: https://lkml.kernel.org/r/20201008191623.2881677-6-nivedita@alum.mit.edu
---
 arch/x86/kernel/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index 04ceea8..68608bd 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -47,6 +47,8 @@ endif
 # non-deterministic coverage.
 KCOV_INSTRUMENT		:= n
 
+CFLAGS_head$(BITS).o	+= -fno-stack-protector
+
 CFLAGS_irq.o := -I $(srctree)/$(src)/../include/asm/trace
 
 obj-y			:= process_$(BITS).o signal.o

[tip: x86/seves] x86/boot/64: Initialize 5-level paging variables earlier

[tip-bot2 for Arvind Sankar]

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

Commit-ID:     e5ceb9a02402b984feecb95a82239be151c9f4e2
Gitweb:        https://git.kernel.org/tip/e5ceb9a02402b984feecb95a82239be151c9f4e2
Author:        Arvind Sankar <nivedita@alum.mit.edu>
AuthorDate:    Sat, 10 Oct 2020 15:11:10 -04:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 19 Oct 2020 12:47:21 +02:00

x86/boot/64: Initialize 5-level paging variables earlier

Commit

  ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")

started using a new set of pagetables even without KASLR.

After that commit, initialize_identity_maps() is called before the
5-level paging variables are setup in choose_random_location(), which
will not work if 5-level paging is actually enabled.

Fix this by moving the initialization of __pgtable_l5_enabled,
pgdir_shift and ptrs_per_p4d into cleanup_trampoline(), which is called
immediately after the finalization of whether the kernel is executing
with 4- or 5-level paging. This will be earlier than anything that might
require those variables, and keeps the 4- vs 5-level paging code all in
one place.

Fixes: ca0e22d4f011 ("x86/boot/compressed/64: Always switch to own page table")
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Joerg Roedel <jroedel@suse.de>
Tested-by: Joerg Roedel <jroedel@suse.de>
Tested-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Link: https://lkml.kernel.org/r/20201010191110.4060905-1-nivedita@alum.mit.edu
---
 arch/x86/boot/compressed/ident_map_64.c |  6 ------
 arch/x86/boot/compressed/kaslr.c        |  8 --------
 arch/x86/boot/compressed/pgtable_64.c   | 16 ++++++++++++++++
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index 063a60e..c6f7aef 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -33,12 +33,6 @@
 #define __PAGE_OFFSET __PAGE_OFFSET_BASE
 #include "../../mm/ident_map.c"
 
-#ifdef CONFIG_X86_5LEVEL
-unsigned int __pgtable_l5_enabled;
-unsigned int pgdir_shift = 39;
-unsigned int ptrs_per_p4d = 1;
-#endif
-
 /* Used by PAGE_KERN* macros: */
 pteval_t __default_kernel_pte_mask __read_mostly = ~0;
 
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index b59547c..b92fffb 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -840,14 +840,6 @@ void choose_random_location(unsigned long input,
 		return;
 	}
 
-#ifdef CONFIG_X86_5LEVEL
-	if (__read_cr4() & X86_CR4_LA57) {
-		__pgtable_l5_enabled = 1;
-		pgdir_shift = 48;
-		ptrs_per_p4d = 512;
-	}
-#endif
-
 	boot_params->hdr.loadflags |= KASLR_FLAG;
 
 	if (IS_ENABLED(CONFIG_X86_32))
diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
index 7d0394f..5def167 100644
--- a/arch/x86/boot/compressed/pgtable_64.c
+++ b/arch/x86/boot/compressed/pgtable_64.c
@@ -8,6 +8,13 @@
 #define BIOS_START_MIN		0x20000U	/* 128K, less than this is insane */
 #define BIOS_START_MAX		0x9f000U	/* 640K, absolute maximum */
 
+#ifdef CONFIG_X86_5LEVEL
+/* __pgtable_l5_enabled needs to be in .data to avoid being cleared along with .bss */
+unsigned int __section(.data) __pgtable_l5_enabled;
+unsigned int __section(.data) pgdir_shift = 39;
+unsigned int __section(.data) ptrs_per_p4d = 1;
+#endif
+
 struct paging_config {
 	unsigned long trampoline_start;
 	unsigned long l5_required;
@@ -198,4 +205,13 @@ void cleanup_trampoline(void *pgtable)
 
 	/* Restore trampoline memory */
 	memcpy(trampoline_32bit, trampoline_save, TRAMPOLINE_32BIT_SIZE);
+
+	/* Initialize variables for 5-level paging */
+#ifdef CONFIG_X86_5LEVEL
+	if (__read_cr4() & X86_CR4_LA57) {
+		__pgtable_l5_enabled = 1;
+		pgdir_shift = 48;
+		ptrs_per_p4d = 512;
+	}
+#endif
 }