On March 4, 2024 1:23:11 PM PST, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>6.1-stable review patch. If anyone has any objections, please let me know.
>
>------------------
>
>From: Alexander Lobakin <alexandr.lobakin@xxxxxxxxx>
>
>commit 7734a0f31e99c433df3063bbb7e8ee5a16a2cb82 upstream.
>
>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@xxxxxxxxx>
>Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
>Tested-by: Jiri Slaby <jirislaby@xxxxxxxxxx>
>Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>Link: https://lore.kernel.org/r/20230109170403.4117105-2-alexandr.lobakin@xxxxxxxxx
>Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>---
> 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(-)
>
>--- 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
>
> /*
>--- a/arch/x86/boot/compressed/head_64.S
>+++ b/arch/x86/boot/compressed/head_64.S
>@@ -580,7 +580,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
>
> /*
>--- a/arch/x86/boot/compressed/misc.c
>+++ b/arch/x86/boot/compressed/misc.c
>@@ -277,7 +277,7 @@ static inline void handle_relocations(vo
> { }
> #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_kerne
> 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_kerne
> 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)
>
>
I would be surprised if this *didn't* break some boot loader. ..
