* [PATCH v3 0/2] x86/boot: fix relying on link order
@ 2023-01-09 17:04 Alexander Lobakin
2023-01-09 17:04 ` [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code Alexander Lobakin
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alexander Lobakin @ 2023-01-09 17:04 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
Cc: Alexander Lobakin, Jiri Slaby, H. Peter Anvin,
Peter Zijlstra (Intel), Tony Luck, Kees Cook, Masahiro Yamada,
x86, linux-kernel
Currently, the x86 decompressor code expects the kernel entry point to be
exactly at the beginning of the kernel image. It's always been true, but
is hacky in multiple ways: special .head.text section and linking certain
object files first to have them at the beginning.
Make the code independent from the link order and then kill the latter.
The former is to be resolved a bit later.
I didn't put any "Fixes:" tag since it's not linear. The lines changed
with 0001 came from the initial x86 KASLR series, but that unconditional
jump to the kernel beginning already was there. It goes at least from the
set that brought relocatable kernel support to x86, but this is quite
prehistoric already and might not look really relatable.
Alexander Lobakin (2):
x86/boot: robustify calling startup_{32,64}() from the decompressor
code
scripts/head-object-list: remove x86 from the list
arch/x86/boot/compressed/head_32.S | 2 +-
arch/x86/boot/compressed/head_64.S | 2 +-
arch/x86/boot/compressed/misc.c | 18 +++++++++++-------
scripts/head-object-list.txt | 6 ------
4 files changed, 13 insertions(+), 15 deletions(-)
---
From v2[0]:
* rebase on top of 6.2;
* prettify debug entry point print.
From v1[1]:
* collect the Tested-by tags (Jiri);
* don't add pathetic returns after noreturn error() (Jiri);
* debug-print the entry point offset via debug_putaddr() before
booting (Jiri);
* always have an empty line before return statements (Jiri).
[0] https://lore.kernel.org/all/20221101161529.1634188-1-alexandr.lobakin@intel.com
[1] https://lore.kernel.org/all/20221031151047.167288-1-alexandr.lobakin@intel.com
--
2.39.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code 2023-01-09 17:04 [PATCH v3 0/2] x86/boot: fix relying on link order Alexander Lobakin @ 2023-01-09 17:04 ` Alexander Lobakin 2023-01-09 18:22 ` [tip: x86/boot] x86/boot: Robustify " tip-bot2 for Alexander Lobakin 2023-01-09 17:04 ` [PATCH v3 2/2] scripts/head-object-list: remove x86 from the list Alexander Lobakin 2023-01-09 17:23 ` [PATCH v3 0/2] x86/boot: fix relying on link order Ingo Molnar 2 siblings, 1 reply; 6+ messages in thread From: Alexander Lobakin @ 2023-01-09 17:04 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen Cc: Alexander Lobakin, Jiri Slaby, H. Peter Anvin, Peter Zijlstra (Intel), Tony Luck, Kees Cook, Masahiro Yamada, x86, linux-kernel After commit ce697ccee1a8 ("kbuild: remove head-y syntax"), I started digging whether x86 is ready for removing this old cruft. Removing its objects from the list makes the kernel unbootable. This applies only to bzImage, vmlinux still works correctly. The reason is that with no strict object order determined by the linker arguments, not the linker script, startup_64 can be placed not right at the beginning of the kernel. Here's vmlinux.map's beginning before removing: ffffffff81000000 vmlinux.o:(.head.text) ffffffff81000000 startup_64 ffffffff81000070 secondary_startup_64 ffffffff81000075 secondary_startup_64_no_verify ffffffff81000160 verify_cpu and after: ffffffff81000000 vmlinux.o:(.head.text) ffffffff81000000 pvh_start_xen ffffffff81000080 startup_64 ffffffff810000f0 secondary_startup_64 ffffffff810000f5 secondary_startup_64_no_verify Not a problem itself, but the self-extractor code has the address of that function hardcoded the beginning, not looking onto the ELF header, which always contains the address of startup_{32,64}(). So, instead of doing an "act of blind faith", just take the address from the ELF header and extract a relative offset to the entry point. The decompressor function already returns a pointer to the beginning of the kernel to the Asm code, which then jumps to it, so add that offset to the return value. This doesn't change anything for now, but allows to resign from the "head object list" for x86 and makes sure valid Kbuild or any other improvements won't break anything here in general. Tested-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> --- arch/x86/boot/compressed/head_32.S | 2 +- arch/x86/boot/compressed/head_64.S | 2 +- arch/x86/boot/compressed/misc.c | 18 +++++++++++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 6589ddd4cfaf..987ae727cf9f 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -187,7 +187,7 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated) leal boot_heap@GOTOFF(%ebx), %eax pushl %eax /* heap area */ pushl %esi /* real mode pointer */ - call extract_kernel /* returns kernel location in %eax */ + call extract_kernel /* returns kernel entry point in %eax */ addl $24, %esp /* diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index a75712991df3..03c4328a88cb 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -569,7 +569,7 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated) movl input_len(%rip), %ecx /* input_len */ movq %rbp, %r8 /* output target address */ movl output_len(%rip), %r9d /* decompressed length, end of relocs */ - call extract_kernel /* returns kernel location in %rax */ + call extract_kernel /* returns kernel entry point in %rax */ popq %rsi /* diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index cf690d8712f4..014ff222bf4b 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -277,7 +277,7 @@ static inline void handle_relocations(void *output, unsigned long output_len, { } #endif -static void parse_elf(void *output) +static size_t parse_elf(void *output) { #ifdef CONFIG_X86_64 Elf64_Ehdr ehdr; @@ -293,10 +293,8 @@ static void parse_elf(void *output) if (ehdr.e_ident[EI_MAG0] != ELFMAG0 || ehdr.e_ident[EI_MAG1] != ELFMAG1 || ehdr.e_ident[EI_MAG2] != ELFMAG2 || - ehdr.e_ident[EI_MAG3] != ELFMAG3) { + ehdr.e_ident[EI_MAG3] != ELFMAG3) error("Kernel is not a valid ELF file"); - return; - } debug_putstr("Parsing ELF... "); @@ -328,6 +326,8 @@ static void parse_elf(void *output) } free(phdrs); + + return ehdr.e_entry - LOAD_PHYSICAL_ADDR; } /* @@ -356,6 +356,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, const unsigned long kernel_total_size = VO__end - VO__text; unsigned long virt_addr = LOAD_PHYSICAL_ADDR; unsigned long needed_size; + size_t entry_offset; /* Retain x86 boot parameters pointer passed from startup_32/64. */ boot_params = rmode; @@ -456,14 +457,17 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, debug_putstr("\nDecompressing Linux... "); __decompress(input_data, input_len, NULL, NULL, output, output_len, NULL, error); - parse_elf(output); + entry_offset = parse_elf(output); handle_relocations(output, output_len, virt_addr); - debug_putstr("done.\nBooting the kernel.\n"); + + debug_putstr("done.\nBooting the kernel (entry_offset: 0x"); + debug_puthex(entry_offset); + debug_putstr(").\n"); /* Disable exception handling before booting the kernel */ cleanup_exception_handling(); - return output; + return output + entry_offset; } void fortify_panic(const char *name) -- 2.39.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip: x86/boot] x86/boot: Robustify calling startup_{32,64}() from the decompressor code 2023-01-09 17:04 ` [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code Alexander Lobakin @ 2023-01-09 18:22 ` tip-bot2 for Alexander Lobakin 0 siblings, 0 replies; 6+ messages in thread From: tip-bot2 for Alexander Lobakin @ 2023-01-09 18:22 UTC (permalink / raw) To: linux-tip-commits Cc: Alexander Lobakin, Ingo Molnar, Jiri Slaby, H. Peter Anvin, Linus Torvalds, x86, linux-kernel The following commit has been merged into the x86/boot branch of tip: Commit-ID: 7734a0f31e99c433df3063bbb7e8ee5a16a2cb82 Gitweb: https://git.kernel.org/tip/7734a0f31e99c433df3063bbb7e8ee5a16a2cb82 Author: Alexander Lobakin <alexandr.lobakin@intel.com> AuthorDate: Mon, 09 Jan 2023 18:04:02 +01:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Mon, 09 Jan 2023 18:22:21 +01:00 x86/boot: Robustify calling startup_{32,64}() from the decompressor code After commit ce697ccee1a8 ("kbuild: remove head-y syntax"), I started digging whether x86 is ready for removing this old cruft. Removing its objects from the list makes the kernel unbootable. This applies only to bzImage, vmlinux still works correctly. The reason is that with no strict object order determined by the linker arguments, not the linker script, startup_64 can be placed not right at the beginning of the kernel. Here's vmlinux.map's beginning before removing: ffffffff81000000 vmlinux.o:(.head.text) ffffffff81000000 startup_64 ffffffff81000070 secondary_startup_64 ffffffff81000075 secondary_startup_64_no_verify ffffffff81000160 verify_cpu and after: ffffffff81000000 vmlinux.o:(.head.text) ffffffff81000000 pvh_start_xen ffffffff81000080 startup_64 ffffffff810000f0 secondary_startup_64 ffffffff810000f5 secondary_startup_64_no_verify Not a problem itself, but the self-extractor code has the address of that function hardcoded the beginning, not looking onto the ELF header, which always contains the address of startup_{32,64}(). So, instead of doing an "act of blind faith", just take the address from the ELF header and extract a relative offset to the entry point. The decompressor function already returns a pointer to the beginning of the kernel to the Asm code, which then jumps to it, so add that offset to the return value. This doesn't change anything for now, but allows to resign from the "head object list" for x86 and makes sure valid Kbuild or any other improvements won't break anything here in general. Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Tested-by: Jiri Slaby <jirislaby@kernel.org> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Link: https://lore.kernel.org/r/20230109170403.4117105-2-alexandr.lobakin@intel.com --- arch/x86/boot/compressed/head_32.S | 2 +- arch/x86/boot/compressed/head_64.S | 2 +- arch/x86/boot/compressed/misc.c | 18 +++++++++++------- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S index 6589ddd..987ae72 100644 --- a/arch/x86/boot/compressed/head_32.S +++ b/arch/x86/boot/compressed/head_32.S @@ -187,7 +187,7 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated) leal boot_heap@GOTOFF(%ebx), %eax pushl %eax /* heap area */ pushl %esi /* real mode pointer */ - call extract_kernel /* returns kernel location in %eax */ + call extract_kernel /* returns kernel entry point in %eax */ addl $24, %esp /* diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S index a757129..03c4328 100644 --- a/arch/x86/boot/compressed/head_64.S +++ b/arch/x86/boot/compressed/head_64.S @@ -569,7 +569,7 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated) movl input_len(%rip), %ecx /* input_len */ movq %rbp, %r8 /* output target address */ movl output_len(%rip), %r9d /* decompressed length, end of relocs */ - call extract_kernel /* returns kernel location in %rax */ + call extract_kernel /* returns kernel entry point in %rax */ popq %rsi /* diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index cf690d8..014ff22 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -277,7 +277,7 @@ static inline void handle_relocations(void *output, unsigned long output_len, { } #endif -static void parse_elf(void *output) +static size_t parse_elf(void *output) { #ifdef CONFIG_X86_64 Elf64_Ehdr ehdr; @@ -293,10 +293,8 @@ static void parse_elf(void *output) if (ehdr.e_ident[EI_MAG0] != ELFMAG0 || ehdr.e_ident[EI_MAG1] != ELFMAG1 || ehdr.e_ident[EI_MAG2] != ELFMAG2 || - ehdr.e_ident[EI_MAG3] != ELFMAG3) { + ehdr.e_ident[EI_MAG3] != ELFMAG3) error("Kernel is not a valid ELF file"); - return; - } debug_putstr("Parsing ELF... "); @@ -328,6 +326,8 @@ static void parse_elf(void *output) } free(phdrs); + + return ehdr.e_entry - LOAD_PHYSICAL_ADDR; } /* @@ -356,6 +356,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, const unsigned long kernel_total_size = VO__end - VO__text; unsigned long virt_addr = LOAD_PHYSICAL_ADDR; unsigned long needed_size; + size_t entry_offset; /* Retain x86 boot parameters pointer passed from startup_32/64. */ boot_params = rmode; @@ -456,14 +457,17 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, debug_putstr("\nDecompressing Linux... "); __decompress(input_data, input_len, NULL, NULL, output, output_len, NULL, error); - parse_elf(output); + entry_offset = parse_elf(output); handle_relocations(output, output_len, virt_addr); - debug_putstr("done.\nBooting the kernel.\n"); + + debug_putstr("done.\nBooting the kernel (entry_offset: 0x"); + debug_puthex(entry_offset); + debug_putstr(").\n"); /* Disable exception handling before booting the kernel */ cleanup_exception_handling(); - return output; + return output + entry_offset; } void fortify_panic(const char *name) ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] scripts/head-object-list: remove x86 from the list 2023-01-09 17:04 [PATCH v3 0/2] x86/boot: fix relying on link order Alexander Lobakin 2023-01-09 17:04 ` [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code Alexander Lobakin @ 2023-01-09 17:04 ` Alexander Lobakin 2023-01-09 18:22 ` [tip: x86/boot] scripts/head-object-list: Remove " tip-bot2 for Alexander Lobakin 2023-01-09 17:23 ` [PATCH v3 0/2] x86/boot: fix relying on link order Ingo Molnar 2 siblings, 1 reply; 6+ messages in thread From: Alexander Lobakin @ 2023-01-09 17:04 UTC (permalink / raw) To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen Cc: Alexander Lobakin, Jiri Slaby, H. Peter Anvin, Peter Zijlstra (Intel), Tony Luck, Kees Cook, Masahiro Yamada, x86, linux-kernel Now that x86 boot code is not hardcoded to the particular linking order, remove x86 files from the list and let them be placed inside the vmlinux according only to the linker script and linker preferences. Tested-by: Jiri Slaby <jirislaby@kernel.org> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> --- scripts/head-object-list.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/head-object-list.txt b/scripts/head-object-list.txt index b074134cfac2..b2a0e21ea8d7 100644 --- a/scripts/head-object-list.txt +++ b/scripts/head-object-list.txt @@ -42,10 +42,4 @@ arch/s390/kernel/head64.o arch/sh/kernel/head_32.o arch/sparc/kernel/head_32.o arch/sparc/kernel/head_64.o -arch/x86/kernel/head_32.o -arch/x86/kernel/head_64.o -arch/x86/kernel/head32.o -arch/x86/kernel/head64.o -arch/x86/kernel/ebda.o -arch/x86/kernel/platform-quirks.o arch/xtensa/kernel/head.o -- 2.39.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip: x86/boot] scripts/head-object-list: Remove x86 from the list 2023-01-09 17:04 ` [PATCH v3 2/2] scripts/head-object-list: remove x86 from the list Alexander Lobakin @ 2023-01-09 18:22 ` tip-bot2 for Alexander Lobakin 0 siblings, 0 replies; 6+ messages in thread From: tip-bot2 for Alexander Lobakin @ 2023-01-09 18:22 UTC (permalink / raw) To: linux-tip-commits Cc: Alexander Lobakin, Ingo Molnar, Jiri Slaby, x86, linux-kernel The following commit has been merged into the x86/boot branch of tip: Commit-ID: 5353fff29e42d0efc844dcaf764336d20a7f6b44 Gitweb: https://git.kernel.org/tip/5353fff29e42d0efc844dcaf764336d20a7f6b44 Author: Alexander Lobakin <alexandr.lobakin@intel.com> AuthorDate: Mon, 09 Jan 2023 18:04:03 +01:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Mon, 09 Jan 2023 18:22:21 +01:00 scripts/head-object-list: Remove x86 from the list Now that x86 boot code is not hardcoded to the particular linking order, remove x86 files from the list and let them be placed inside the vmlinux according only to the linker script and linker preferences. Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Tested-by: Jiri Slaby <jirislaby@kernel.org> Link: https://lore.kernel.org/r/20230109170403.4117105-3-alexandr.lobakin@intel.com --- scripts/head-object-list.txt | 6 ------ 1 file changed, 6 deletions(-) diff --git a/scripts/head-object-list.txt b/scripts/head-object-list.txt index b074134..b2a0e21 100644 --- a/scripts/head-object-list.txt +++ b/scripts/head-object-list.txt @@ -42,10 +42,4 @@ arch/s390/kernel/head64.o arch/sh/kernel/head_32.o arch/sparc/kernel/head_32.o arch/sparc/kernel/head_64.o -arch/x86/kernel/head_32.o -arch/x86/kernel/head_64.o -arch/x86/kernel/head32.o -arch/x86/kernel/head64.o -arch/x86/kernel/ebda.o -arch/x86/kernel/platform-quirks.o arch/xtensa/kernel/head.o ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 0/2] x86/boot: fix relying on link order 2023-01-09 17:04 [PATCH v3 0/2] x86/boot: fix relying on link order Alexander Lobakin 2023-01-09 17:04 ` [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code Alexander Lobakin 2023-01-09 17:04 ` [PATCH v3 2/2] scripts/head-object-list: remove x86 from the list Alexander Lobakin @ 2023-01-09 17:23 ` Ingo Molnar 2 siblings, 0 replies; 6+ messages in thread From: Ingo Molnar @ 2023-01-09 17:23 UTC (permalink / raw) To: Alexander Lobakin Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, Jiri Slaby, H. Peter Anvin, Peter Zijlstra (Intel), Tony Luck, Kees Cook, Masahiro Yamada, x86, linux-kernel * Alexander Lobakin <alexandr.lobakin@intel.com> wrote: > Currently, the x86 decompressor code expects the kernel entry point to be > exactly at the beginning of the kernel image. It's always been true, but > is hacky in multiple ways: special .head.text section and linking certain > object files first to have them at the beginning. > Make the code independent from the link order and then kill the latter. > The former is to be resolved a bit later. > > I didn't put any "Fixes:" tag since it's not linear. The lines changed > with 0001 came from the initial x86 KASLR series, but that unconditional > jump to the kernel beginning already was there. It goes at least from the > set that brought relocatable kernel support to x86, but this is quite > prehistoric already and might not look really relatable. > > Alexander Lobakin (2): > x86/boot: robustify calling startup_{32,64}() from the decompressor > code > scripts/head-object-list: remove x86 from the list > > arch/x86/boot/compressed/head_32.S | 2 +- > arch/x86/boot/compressed/head_64.S | 2 +- > arch/x86/boot/compressed/misc.c | 18 +++++++++++------- > scripts/head-object-list.txt | 6 ------ > 4 files changed, 13 insertions(+), 15 deletions(-) That's a really nice fix/robustification improvement - I've applied your series to tip:x86/boot and will push it out after some testing. Thanks, Ingo ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-09 18:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09 17:04 [PATCH v3 0/2] x86/boot: fix relying on link order Alexander Lobakin
2023-01-09 17:04 ` [PATCH v3 1/2] x86/boot: robustify calling startup_{32,64}() from the decompressor code Alexander Lobakin
2023-01-09 18:22 ` [tip: x86/boot] x86/boot: Robustify " tip-bot2 for Alexander Lobakin
2023-01-09 17:04 ` [PATCH v3 2/2] scripts/head-object-list: remove x86 from the list Alexander Lobakin
2023-01-09 18:22 ` [tip: x86/boot] scripts/head-object-list: Remove " tip-bot2 for Alexander Lobakin
2023-01-09 17:23 ` [PATCH v3 0/2] x86/boot: fix relying on link order Ingo Molnar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox