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

[Arvind Sankar <nivedita@alum.mit.edu>]

On Thu, Oct 08, 2020 at 10:42:19AM +0200, Joerg Roedel wrote:
> On Wed, Oct 07, 2020 at 03:53:51PM -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.
> 
> Hmm, that is weird. Can you please share your config with which this
> happens? I have a commit in my local branch which disables page-faulting
> in the pre-decompression code before jumping to the uncompressed kernel
> image, and it did not break anything here.
> 
> Also, what compiler did you use to trigger this?
> 

The compiler is gcc-10.

QEMU usually puts the command line in the low 1Mb, so it needs to avoid
accessing the command line as well, as the stack canary is at 0x28. So
defconfig with
  AMD_MEM_ENCRYPT	enabled
  RANDOMIZE_BASE	disabled
  EARLY_PRINTK		disabled
crashes on boot if you disable the #PF handler.

I traced it down with the patch below, which produces
  $ qemu-system-x86_64 -m 2048 -kernel bzImage -append "earlyprintk=ttyS0,keep" -drive if=pflash,format=raw,readonly,file=OVMF_64.fd -nographic -enable-kvm
  
  ...
  
  Decompressing Linux... 
  Error Code: 0000000000000002
  CR2: 0x0000000003e00000
  RIP relative to _head: 0x000000000088521d
  
  Error Code: 0000000000000002
  CR2: 0x0000000004000000
  RIP relative to _head: 0x000000000088521d
  
  ...
  
  Error Code: 0000000000000002
  CR2: 0x0000000005a00000
  RIP relative to _head: 0x00000000008854d0
  Parsing ELF... done.
  Booting the kernel.
  
  Error Code: 0000000000000000
  CR2: 0x0000000000000028
  RIP relative to _head: 0xfffffffffe03b67c

That last page fault is in the main kernel at the stack canary's address.

Patch:

diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c
index 063a60edcf99..e0578dcbaecc 100644
--- a/arch/x86/boot/compressed/ident_map_64.c
+++ b/arch/x86/boot/compressed/ident_map_64.c
@@ -325,6 +325,13 @@ void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code)
 	unsigned long end;
 	bool ghcb_fault;
 
+	error_putstr("\nError Code: ");
+	error_puthex(error_code);
+	error_putstr("\nCR2: 0x");
+	error_puthex(address);
+	error_putstr("\nRIP relative to _head: 0x");
+	error_puthex(regs->ip - (unsigned long)_head);
+	error_putstr("\n");
 	ghcb_fault = sev_es_check_ghcb_fault(address);
 
 	address   &= PMD_MASK;
diff --git a/arch/x86/boot/early_serial_console.c b/arch/x86/boot/early_serial_console.c
index 023bf1c3de8b..d6f051e4b64b 100644
--- a/arch/x86/boot/early_serial_console.c
+++ b/arch/x86/boot/early_serial_console.c
@@ -50,6 +50,7 @@ static void parse_earlyprintk(void)
 	int pos = 0;
 	int port = 0;
 
+#if 0
 	if (cmdline_find_option("earlyprintk", arg, sizeof(arg)) > 0) {
 		char *e;
 
@@ -93,6 +94,8 @@ static void parse_earlyprintk(void)
 		if (baud == 0 || arg + pos == e)
 			baud = DEFAULT_BAUD;
 	}
+#endif
+	port = 0x3f8;
 
 	if (port)
 		early_serial_init(port, baud);