* [PATCH 0/5] Couple of bugfixes to sev-es series @ 2020-10-07 19:53 Arvind Sankar 2020-10-07 19:53 ` [PATCH 1/5] x86/boot: Initialize boot_params in startup code Arvind Sankar ` (4 more replies) 0 siblings, 5 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel 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. Arvind Sankar (5): x86/boot: Initialize boot_params in startup code x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h x86/boot/64: Change add_identity_map() to take size for ease of use x86/boot/64: Explicitly map boot_params and command line x86/head/64: Disable stack protection for head$(BITS).o arch/x86/boot/compressed/cmdline.c | 8 ------ arch/x86/boot/compressed/head_32.S | 11 ++++---- arch/x86/boot/compressed/head_64.S | 34 ++++++++----------------- arch/x86/boot/compressed/ident_map_64.c | 18 ++++++------- arch/x86/boot/compressed/kaslr.c | 6 ----- arch/x86/boot/compressed/misc.c | 10 +------- arch/x86/boot/compressed/misc.h | 13 ++++++++++ arch/x86/boot/compressed/pgtable_64.c | 5 +--- arch/x86/kernel/Makefile | 2 ++ 9 files changed, 43 insertions(+), 64 deletions(-) -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] x86/boot: Initialize boot_params in startup code 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar @ 2020-10-07 19:53 ` Arvind Sankar 2020-10-08 9:04 ` Joerg Roedel 2020-10-07 19:53 ` [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h Arvind Sankar ` (3 subsequent siblings) 4 siblings, 1 reply; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel 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. Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> --- arch/x86/boot/compressed/head_32.S | 11 +++++---- arch/x86/boot/compressed/head_64.S | 34 +++++++++------------------ arch/x86/boot/compressed/misc.c | 10 +------- arch/x86/boot/compressed/pgtable_64.c | 5 +--- 4 files changed, 19 insertions(+), 41 deletions(-) diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 659fad53ca82..c2b014ca92f7 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,8 @@ SYM_DATA_START_LOCAL(gdt) .quad 0x00cf92000000ffff /* __KERNEL_DS */ SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end) +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..78f873f76579 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,8 @@ SYM_DATA_START(boot_idt) .endr SYM_DATA_END_LABEL(boot_idt, SYM_L_GLOBAL, boot_idt_end) +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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] x86/boot: Initialize boot_params in startup code 2020-10-07 19:53 ` [PATCH 1/5] x86/boot: Initialize boot_params in startup code Arvind Sankar @ 2020-10-08 9:04 ` Joerg Roedel 2020-10-08 13:44 ` Arvind Sankar 0 siblings, 1 reply; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 9:04 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel On Wed, Oct 07, 2020 at 03:53:47PM -0400, Arvind Sankar wrote: > 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. > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> Nice cleanup! > /* > * Jump to the extracted kernel. > */ > - xorl %ebx, %ebx > + movl boot_params@GOTOFF(%ebx), %esi > jmp *%eax > SYM_FUNC_END(.Lrelocated) > > @@ -209,6 +208,8 @@ SYM_DATA_START_LOCAL(gdt) > .quad 0x00cf92000000ffff /* __KERNEL_DS */ > SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end) > > +SYM_DATA(boot_params, .long 0) > + You should add a comment here that boot_params needs to be in the .data section because in .bss it would get zeroed out again later. Same applies to the 64bit version of this. With that changed: Reviewed-by: Joerg Roedel <jroedel@suse.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] x86/boot: Initialize boot_params in startup code 2020-10-08 9:04 ` Joerg Roedel @ 2020-10-08 13:44 ` Arvind Sankar 0 siblings, 0 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 13:44 UTC (permalink / raw) To: Joerg Roedel; +Cc: Arvind Sankar, x86, linux-kernel On Thu, Oct 08, 2020 at 11:04:20AM +0200, Joerg Roedel wrote: > On Wed, Oct 07, 2020 at 03:53:47PM -0400, Arvind Sankar wrote: > > 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. > > > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > > Nice cleanup! > > > /* > > * Jump to the extracted kernel. > > */ > > - xorl %ebx, %ebx > > + movl boot_params@GOTOFF(%ebx), %esi > > jmp *%eax > > SYM_FUNC_END(.Lrelocated) > > > > @@ -209,6 +208,8 @@ SYM_DATA_START_LOCAL(gdt) > > .quad 0x00cf92000000ffff /* __KERNEL_DS */ > > SYM_DATA_END_LABEL(gdt, SYM_L_LOCAL, gdt_end) > > > > +SYM_DATA(boot_params, .long 0) > > + > > You should add a comment here that boot_params needs to be in the .data > section because in .bss it would get zeroed out again later. Same > applies to the 64bit version of this. > > With that changed: > > Reviewed-by: Joerg Roedel <jroedel@suse.de> > Ok. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar 2020-10-07 19:53 ` [PATCH 1/5] x86/boot: Initialize boot_params in startup code Arvind Sankar @ 2020-10-07 19:53 ` Arvind Sankar 2020-10-08 9:11 ` Joerg Roedel 2020-10-08 9:30 ` Borislav Petkov 2020-10-07 19:53 ` [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use Arvind Sankar ` (2 subsequent siblings) 4 siblings, 2 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h for easier use from multiple files. Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> --- arch/x86/boot/compressed/cmdline.c | 8 -------- arch/x86/boot/compressed/kaslr.c | 6 ------ arch/x86/boot/compressed/misc.h | 13 +++++++++++++ 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c index f1add5d85da9..d0e1d386749d 100644 --- a/arch/x86/boot/compressed/cmdline.c +++ b/arch/x86/boot/compressed/cmdline.c @@ -12,14 +12,6 @@ static inline char rdfs8(addr_t addr) return *((char *)(fs + addr)); } #include "../cmdline.c" -unsigned long get_cmd_line_ptr(void) -{ - unsigned long cmd_line_ptr = boot_params->hdr.cmd_line_ptr; - - cmd_line_ptr |= (u64)boot_params->ext_cmd_line_ptr << 32; - - return cmd_line_ptr; -} int cmdline_find_option(const char *option, char *buffer, int bufsize) { return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize); diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index b59547ce5b19..f3286a3bef36 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -36,12 +36,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..95aacc361f78 100644 --- a/arch/x86/boot/compressed/misc.h +++ b/arch/x86/boot/compressed/misc.h @@ -25,6 +25,10 @@ #include <asm/bootparam.h> #include <asm/desc_defs.h> +#define _SETUP +#include <asm/setup.h> /* For COMMAND_LINE_SIZE */ +#undef _SETUP + #define BOOT_CTYPE_H #include <linux/acpi.h> @@ -70,6 +74,15 @@ static inline void debug_puthex(unsigned long value) #endif /* cmdline.c */ +static inline +unsigned long get_cmd_line_ptr(void) +{ + unsigned long cmd_line_ptr = boot_params->hdr.cmd_line_ptr; + + cmd_line_ptr |= (u64)boot_params->ext_cmd_line_ptr << 32; + + return cmd_line_ptr; +} int cmdline_find_option(const char *option, char *buffer, int bufsize); int cmdline_find_option_bool(const char *option); -- 2.26.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-07 19:53 ` [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h Arvind Sankar @ 2020-10-08 9:11 ` Joerg Roedel 2020-10-08 9:30 ` Borislav Petkov 1 sibling, 0 replies; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 9:11 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel On Wed, Oct 07, 2020 at 03:53:48PM -0400, Arvind Sankar wrote: > Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h for easier > use from multiple files. > > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu> > --- > arch/x86/boot/compressed/cmdline.c | 8 -------- > arch/x86/boot/compressed/kaslr.c | 6 ------ > arch/x86/boot/compressed/misc.h | 13 +++++++++++++ > 3 files changed, 13 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/boot/compressed/cmdline.c b/arch/x86/boot/compressed/cmdline.c > index f1add5d85da9..d0e1d386749d 100644 > --- a/arch/x86/boot/compressed/cmdline.c > +++ b/arch/x86/boot/compressed/cmdline.c > @@ -12,14 +12,6 @@ static inline char rdfs8(addr_t addr) > return *((char *)(fs + addr)); > } > #include "../cmdline.c" > -unsigned long get_cmd_line_ptr(void) > -{ > - unsigned long cmd_line_ptr = boot_params->hdr.cmd_line_ptr; > - > - cmd_line_ptr |= (u64)boot_params->ext_cmd_line_ptr << 32; > - > - return cmd_line_ptr; > -} > int cmdline_find_option(const char *option, char *buffer, int bufsize) > { > return __cmdline_find_option(get_cmd_line_ptr(), option, buffer, bufsize); > diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c > index b59547ce5b19..f3286a3bef36 100644 > --- a/arch/x86/boot/compressed/kaslr.c > +++ b/arch/x86/boot/compressed/kaslr.c > @@ -36,12 +36,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..95aacc361f78 100644 > --- a/arch/x86/boot/compressed/misc.h > +++ b/arch/x86/boot/compressed/misc.h > @@ -25,6 +25,10 @@ > #include <asm/bootparam.h> > #include <asm/desc_defs.h> > > +#define _SETUP > +#include <asm/setup.h> /* For COMMAND_LINE_SIZE */ > +#undef _SETUP > + > #define BOOT_CTYPE_H > #include <linux/acpi.h> > > @@ -70,6 +74,15 @@ static inline void debug_puthex(unsigned long value) > #endif > > /* cmdline.c */ > +static inline > +unsigned long get_cmd_line_ptr(void) > +{ > + unsigned long cmd_line_ptr = boot_params->hdr.cmd_line_ptr; > + > + cmd_line_ptr |= (u64)boot_params->ext_cmd_line_ptr << 32; > + > + return cmd_line_ptr; > +} > int cmdline_find_option(const char *option, char *buffer, int bufsize); > int cmdline_find_option_bool(const char *option); Reviewed-by: Joerg Roedel <jroedel@suse.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-07 19:53 ` [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h Arvind Sankar 2020-10-08 9:11 ` Joerg Roedel @ 2020-10-08 9:30 ` Borislav Petkov 2020-10-08 13:47 ` Arvind Sankar 1 sibling, 1 reply; 21+ messages in thread From: Borislav Petkov @ 2020-10-08 9:30 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, Joerg Roedel, linux-kernel On Wed, Oct 07, 2020 at 03:53:48PM -0400, Arvind Sankar wrote: > Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h for easier > use from multiple files. Well, I don't like that. cmdline.c *is* for cmdline-related things. misc.h is a dumping ground for everything but the kitchen sink. Why can't you leave it there and make it visible to other compilation units? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-08 9:30 ` Borislav Petkov @ 2020-10-08 13:47 ` Arvind Sankar 2020-10-08 15:10 ` Borislav Petkov 0 siblings, 1 reply; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 13:47 UTC (permalink / raw) To: Borislav Petkov; +Cc: Arvind Sankar, x86, Joerg Roedel, linux-kernel On Thu, Oct 08, 2020 at 11:30:42AM +0200, Borislav Petkov wrote: > On Wed, Oct 07, 2020 at 03:53:48PM -0400, Arvind Sankar wrote: > > Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h for easier > > use from multiple files. > > Well, I don't like that. cmdline.c *is* for cmdline-related things. > misc.h is a dumping ground for everything but the kitchen sink. > > Why can't you leave it there and make it visible to other compilation > units? > Are you ok with the include of setup.h? I made the function inline because it's a tiny function, but I can simply add a prototype if that's preferred. KASLR does use it as one more memory region to avoid, rather than just for parsing the command line. Thanks. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-08 13:47 ` Arvind Sankar @ 2020-10-08 15:10 ` Borislav Petkov 2020-10-08 15:30 ` Arvind Sankar 0 siblings, 1 reply; 21+ messages in thread From: Borislav Petkov @ 2020-10-08 15:10 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, Joerg Roedel, linux-kernel On Thu, Oct 08, 2020 at 09:47:23AM -0400, Arvind Sankar wrote: > Are you ok with the include of setup.h? Or you could simply add cmdline.h and include that. It is high time we started cleaning up that include hell in compressed/ and all facilities there be nicely separated. Recently I started untangling it but it is a serious mess. And kernel proper includes leak in there, yuck. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-08 15:10 ` Borislav Petkov @ 2020-10-08 15:30 ` Arvind Sankar 2020-10-08 16:16 ` Borislav Petkov 0 siblings, 1 reply; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 15:30 UTC (permalink / raw) To: Borislav Petkov; +Cc: Arvind Sankar, x86, Joerg Roedel, linux-kernel On Thu, Oct 08, 2020 at 05:10:47PM +0200, Borislav Petkov wrote: > On Thu, Oct 08, 2020 at 09:47:23AM -0400, Arvind Sankar wrote: > > Are you ok with the include of setup.h? > > Or you could simply add cmdline.h and include that. It is high time we > started cleaning up that include hell in compressed/ and all facilities > there be nicely separated. Recently I started untangling it but it is a > serious mess. And kernel proper includes leak in there, yuck. > Ok, I can do that. I'm working on a couple of separate series to clean up cmdline and the compressed boot code a bit. I was actually planning to get rid of boot/compressed/cmdline.c entirely, replacing it with arch/x86/lib/cmdline.c instead: that one's better and is reusable as-is for the decompressor stub, instead of the current hack to use the real-mode boot stub's cmdline.c. The real mess in there is all the includes of .c files from various places. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h 2020-10-08 15:30 ` Arvind Sankar @ 2020-10-08 16:16 ` Borislav Petkov 0 siblings, 0 replies; 21+ messages in thread From: Borislav Petkov @ 2020-10-08 16:16 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, Joerg Roedel, linux-kernel On Thu, Oct 08, 2020 at 11:30:02AM -0400, Arvind Sankar wrote: > I'm working on a couple of separate series to clean up cmdline > and the compressed boot code a bit. I was actually planning to > get rid of boot/compressed/cmdline.c entirely, replacing it with > arch/x86/lib/cmdline.c instead: The problem with mixing code from kernel proper with the decompressor is that when someone changes former, latter gets all those changes too and gradual changes like that have lead to this mess. There's a reason the two are separate and we should separate them even more. I'm even fine with copying functionality between the two instead of sharing. > that one's better and is reusable as-is for the decompressor stub, > instead of the current hack to use the real-mode boot stub's > cmdline.c. The real mess in there is all the includes of .c files from > various places. Yes, and that needs untangling and making all separate. This will keep the decompressor simple and without all that undeffery/ifdeffery because of includes leaking symbols from kernel proper and whatnot. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar 2020-10-07 19:53 ` [PATCH 1/5] x86/boot: Initialize boot_params in startup code Arvind Sankar 2020-10-07 19:53 ` [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h Arvind Sankar @ 2020-10-07 19:53 ` Arvind Sankar 2020-10-08 9:14 ` Joerg Roedel 2020-10-07 19:53 ` [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line Arvind Sankar 2020-10-07 19:53 ` [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o Arvind Sankar 4 siblings, 1 reply; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel Change back the arguments of add_identity_map() to (start, size) instead of (start, end). This reverts 21cf2372618e ("x86/boot/compressed/64: Change add_identity_map() to take start and end") since we will soon have more callers that know the size rather than the end address. This also 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 | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c index 063a60edcf99..070cda70aef3 100644 --- a/arch/x86/boot/compressed/ident_map_64.c +++ b/arch/x86/boot/compressed/ident_map_64.c @@ -90,8 +90,9 @@ static struct x86_mapping_info mapping_info; /* * Adds the specified range to the identity mappings. */ -static void add_identity_map(unsigned long start, unsigned long end) +static void add_identity_map(unsigned long start, unsigned long size) { + unsigned long end = start + size; int ret; /* Align boundary to 2M. */ @@ -152,7 +153,7 @@ void initialize_identity_maps(void) * New page-table is set up - map the kernel image and load it * into cr3. */ - add_identity_map((unsigned long)_head, (unsigned long)_end); + add_identity_map((unsigned long)_head, (unsigned long)_end - (unsigned long)_head); write_cr3(top_level_pgt); } @@ -322,14 +323,10 @@ static void do_pf_error(const char *msg, unsigned long error_code, void do_boot_page_fault(struct pt_regs *regs, unsigned long error_code) { unsigned long address = native_read_cr2(); - unsigned long end; bool ghcb_fault; 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,5 @@ 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. */ - add_identity_map(address, end); + add_identity_map(address & PMD_MASK, PMD_SIZE); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use 2020-10-07 19:53 ` [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use Arvind Sankar @ 2020-10-08 9:14 ` Joerg Roedel 2020-10-08 13:49 ` Arvind Sankar 0 siblings, 1 reply; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 9:14 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel On Wed, Oct 07, 2020 at 03:53:49PM -0400, Arvind Sankar wrote: > Change back the arguments of add_identity_map() to (start, size) instead > of (start, end). This reverts > > 21cf2372618e ("x86/boot/compressed/64: Change add_identity_map() to take start and end") > > since we will soon have more callers that know the size rather than the > end address. > > This also 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 | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c > index 063a60edcf99..070cda70aef3 100644 > --- a/arch/x86/boot/compressed/ident_map_64.c > +++ b/arch/x86/boot/compressed/ident_map_64.c > @@ -90,8 +90,9 @@ static struct x86_mapping_info mapping_info; > /* > * Adds the specified range to the identity mappings. > */ > -static void add_identity_map(unsigned long start, unsigned long end) > +static void add_identity_map(unsigned long start, unsigned long size) > { > + unsigned long end = start + size; This has been discussed during the SEV-ES patch-review already and we settled on making add_identity_map() take start and end as parameter, as that is what kernel_ident_mapping_init() also takes as parameters. So please keep it that way :) Regards, Joerg ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use 2020-10-08 9:14 ` Joerg Roedel @ 2020-10-08 13:49 ` Arvind Sankar 0 siblings, 0 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 13:49 UTC (permalink / raw) To: Joerg Roedel; +Cc: Arvind Sankar, x86, linux-kernel On Thu, Oct 08, 2020 at 11:14:12AM +0200, Joerg Roedel wrote: > On Wed, Oct 07, 2020 at 03:53:49PM -0400, Arvind Sankar wrote: > > Change back the arguments of add_identity_map() to (start, size) instead > > of (start, end). This reverts > > > > 21cf2372618e ("x86/boot/compressed/64: Change add_identity_map() to take start and end") > > > > since we will soon have more callers that know the size rather than the > > end address. > > > > This also 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 | 11 ++++------- > > 1 file changed, 4 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c > > index 063a60edcf99..070cda70aef3 100644 > > --- a/arch/x86/boot/compressed/ident_map_64.c > > +++ b/arch/x86/boot/compressed/ident_map_64.c > > @@ -90,8 +90,9 @@ static struct x86_mapping_info mapping_info; > > /* > > * Adds the specified range to the identity mappings. > > */ > > -static void add_identity_map(unsigned long start, unsigned long end) > > +static void add_identity_map(unsigned long start, unsigned long size) > > { > > + unsigned long end = start + size; > > This has been discussed during the SEV-ES patch-review already and we > settled on making add_identity_map() take start and end as parameter, as > that is what kernel_ident_mapping_init() also takes as parameters. > > So please keep it that way :) > > Regards, > > Joerg At the time, it was one caller knowing end and one knowing size, but now there are two more callers that only know the size, so it seemed easier to have add_identity_map() do the conversion. Thanks. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar ` (2 preceding siblings ...) 2020-10-07 19:53 ` [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use Arvind Sankar @ 2020-10-07 19:53 ` Arvind Sankar 2020-10-08 9:17 ` Joerg Roedel 2020-10-08 9:48 ` Joerg Roedel 2020-10-07 19:53 ` [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o Arvind Sankar 4 siblings, 2 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel 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 | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c index 070cda70aef3..8edeff0d9324 100644 --- a/arch/x86/boot/compressed/ident_map_64.c +++ b/arch/x86/boot/compressed/ident_map_64.c @@ -150,10 +150,13 @@ 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, and load the new page-table into cr3. */ add_identity_map((unsigned long)_head, (unsigned long)_end - (unsigned long)_head); + add_identity_map((unsigned long)boot_params, sizeof(*boot_params)); + add_identity_map(get_cmd_line_ptr(), COMMAND_LINE_SIZE); + write_cr3(top_level_pgt); } -- 2.26.2 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line 2020-10-07 19:53 ` [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line Arvind Sankar @ 2020-10-08 9:17 ` Joerg Roedel 2020-10-08 9:48 ` Joerg Roedel 1 sibling, 0 replies; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 9:17 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel On Wed, Oct 07, 2020 at 03:53:50PM -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 | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/boot/compressed/ident_map_64.c b/arch/x86/boot/compressed/ident_map_64.c > index 070cda70aef3..8edeff0d9324 100644 > --- a/arch/x86/boot/compressed/ident_map_64.c > +++ b/arch/x86/boot/compressed/ident_map_64.c > @@ -150,10 +150,13 @@ 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, and load the new page-table into cr3. > */ Please extend this comment to state that boot_params and command-line need to be mapped here because they might not be touched (and thus mapped) before jumping to the uncompressed kernel image. Otherwise no one will remember why those need to be pre-mapped in a couple of years. With that change and the add_identity_map() call adjusted: Reviewed-by: Joerg Roedel <jroedel@suse.de> ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line 2020-10-07 19:53 ` [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line Arvind Sankar 2020-10-08 9:17 ` Joerg Roedel @ 2020-10-08 9:48 ` Joerg Roedel 2020-10-08 13:57 ` Arvind Sankar 1 sibling, 1 reply; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 9:48 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel On Wed, Oct 07, 2020 at 03:53:50PM -0400, Arvind Sankar wrote: > 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. Looked again, and I think that is wrong for boot_params, which are touched unconditionally at the beginning of extract_kernel(). For the cmdline you are right, but one of CONFIG_ACPI, CONFIG_RANDOMIZE_BASE, CONFIG_X86_5LEVEL or CONFIG_EARLY_PRINTK is sufficient to have it touched during this boot stage. Regards, Joerg ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line 2020-10-08 9:48 ` Joerg Roedel @ 2020-10-08 13:57 ` Arvind Sankar 0 siblings, 0 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 13:57 UTC (permalink / raw) To: Joerg Roedel; +Cc: Arvind Sankar, x86, linux-kernel On Thu, Oct 08, 2020 at 11:48:36AM +0200, Joerg Roedel wrote: > On Wed, Oct 07, 2020 at 03:53:50PM -0400, Arvind Sankar wrote: > > 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. > > Looked again, and I think that is wrong for boot_params, which are > touched unconditionally at the beginning of extract_kernel(). Yes, command line is the only thing that actually breaks, but it is more robust to explicitly make sure boot_params is mapped as well. There's no specific alignment requirement for boot_params AFAICT, so at least in theory it's possible that it would be split across a PMD boundary and only get half-mapped in the decompressor. It's easier not to have to worry about it. > > For the cmdline you are right, but one of CONFIG_ACPI, > CONFIG_RANDOMIZE_BASE, CONFIG_X86_5LEVEL or CONFIG_EARLY_PRINTK is > sufficient to have it touched during this boot stage. > X86_5LEVEL accesses it before the switch to the new page tables, so that doesn't help in getting it mapped. ACPI only accesses it if KASLR is enabled (as well as MEMORY_HOTREMOVE). Thanks. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar ` (3 preceding siblings ...) 2020-10-07 19:53 ` [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line Arvind Sankar @ 2020-10-07 19:53 ` Arvind Sankar 2020-10-08 8:42 ` Joerg Roedel 4 siblings, 1 reply; 21+ messages in thread From: Arvind Sankar @ 2020-10-07 19:53 UTC (permalink / raw) To: x86, Joerg Roedel; +Cc: linux-kernel 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 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o 2020-10-07 19:53 ` [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o Arvind Sankar @ 2020-10-08 8:42 ` Joerg Roedel 2020-10-08 14:52 ` Arvind Sankar 0 siblings, 1 reply; 21+ messages in thread From: Joerg Roedel @ 2020-10-08 8:42 UTC (permalink / raw) To: Arvind Sankar; +Cc: x86, linux-kernel 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o 2020-10-08 8:42 ` Joerg Roedel @ 2020-10-08 14:52 ` Arvind Sankar 0 siblings, 0 replies; 21+ messages in thread From: Arvind Sankar @ 2020-10-08 14:52 UTC (permalink / raw) To: Joerg Roedel; +Cc: Arvind Sankar, x86, linux-kernel 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); ^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-10-08 16:16 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-07 19:53 [PATCH 0/5] Couple of bugfixes to sev-es series Arvind Sankar 2020-10-07 19:53 ` [PATCH 1/5] x86/boot: Initialize boot_params in startup code Arvind Sankar 2020-10-08 9:04 ` Joerg Roedel 2020-10-08 13:44 ` Arvind Sankar 2020-10-07 19:53 ` [PATCH 2/5] x86/boot: Move get_cmd_line_ptr() and COMMAND_LINE_SIZE into misc.h Arvind Sankar 2020-10-08 9:11 ` Joerg Roedel 2020-10-08 9:30 ` Borislav Petkov 2020-10-08 13:47 ` Arvind Sankar 2020-10-08 15:10 ` Borislav Petkov 2020-10-08 15:30 ` Arvind Sankar 2020-10-08 16:16 ` Borislav Petkov 2020-10-07 19:53 ` [PATCH 3/5] x86/boot/64: Change add_identity_map() to take size for ease of use Arvind Sankar 2020-10-08 9:14 ` Joerg Roedel 2020-10-08 13:49 ` Arvind Sankar 2020-10-07 19:53 ` [PATCH 4/5] x86/boot/64: Explicitly map boot_params and command line Arvind Sankar 2020-10-08 9:17 ` Joerg Roedel 2020-10-08 9:48 ` Joerg Roedel 2020-10-08 13:57 ` Arvind Sankar 2020-10-07 19:53 ` [PATCH 5/5] x86/head/64: Disable stack protection for head$(BITS).o Arvind Sankar 2020-10-08 8:42 ` Joerg Roedel 2020-10-08 14:52 ` Arvind Sankar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).