2023-08-18 18:00:55

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 00/17] x86/boot: Rework PE header generation

Now that the EFI stub boot flow no longer relies on memory that is
executable and writable at the same time, we can reorganize the PE/COFF
view of the kernel image and expose the decompressor binary's code and
r/o data as a .text section and data/bss as a .data section, using 4k
alignment and limited permissions.

Doing so is necessary for compatibility with hardening measures that are
being rolled out on x86 PCs built to run Windows (i.e., the majority of
them). The EFI boot environment that the Linux EFI stub executes in is
especially sensitive to safety issues, given that a vulnerability in the
loader of one OS can be abused to attack another.

In true x86 fashion, this is more complicated than on other
architectures, which have implemented this code/data split with 4k
alignment from the beginning. The complicating factor here is that the
boot image consists of two different parts, which are stitched together
and fixed up using a special build tool.

The first three patches simplify the x86 EFI stub code so it does not
even bother reading the setup header from the image - passing arguments
this way is not supported by EFI boot anyway.

Then, the bzImage is simplified and reorganized, primarily by:
- dropping the ancient 'bugger off' message occupying much of the header
space
- using a fixed size of 16k for the setup block
- setting header values from asm instead of via the build tool

Finally, the payload is split into .text and .data, and the section and
file alignment increased to 4k/512 respectively.

The only remaining task performed by the build tool is generating the
CRC-32 that is fundamentally broken in practice and never used, so that
is dropped entirely at the end.

This supersedes the work proposed by Evgeniy last year, which did a
major rewrite of the build tool in order to clean it up, before updating
it to generate the new 4k aligned image layout. As this series proves,
the build tool is mostly unnecessary, and we have too many of those
already.

Cc: Evgeniy Baskov <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Peter Jones <[email protected]>
Cc: Matthew Garrett <[email protected]>
Cc: Gerd Hoffmann <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Marvin Häuser <[email protected]>

Ard Biesheuvel (17):
x86/efi: Drop EFI stub .bss from .data section
x86/efi: Disregard setup header of loaded image
x86/efi: Drop alignment flags from PE section headers
x86/boot: Remove the 'bugger off' message
x86/boot: Omit compression buffer from PE/COFF image memory footprint
x86/boot: Drop redundant code setting the root device
x86/boot: Grab kernel_info offset from zoffset header directly
x86/boot: Drop references to startup_64
x86/boot: Set EFI handover offset directly in header asm
x86/boot: Drop workaround for binutils 2.14 in linker script ASSERTs
x86/boot: Use fixed size of 16k for setup block
x86/boot: Derive file size from _edata symbol
x86/boot: Construct PE/COFF .text section from assembler
x86/boot: Drop PE/COFF .reloc section
x86/boot: Split off PE/COFF .data section
x86/boot: Increase section and file alignment to 4k/512
x86/boot: Drop CRC-32 checksum and the build tool that generates it

Documentation/arch/x86/boot.rst | 10 -
arch/x86/boot/Makefile | 12 +-
arch/x86/boot/compressed/vmlinux.lds.S | 5 +-
arch/x86/boot/header.S | 217 ++++-----
arch/x86/boot/setup.ld | 21 +-
arch/x86/boot/tools/.gitignore | 2 -
arch/x86/boot/tools/build.c | 502 --------------------
drivers/firmware/efi/libstub/Makefile | 7 -
drivers/firmware/efi/libstub/x86-stub.c | 46 +-
9 files changed, 113 insertions(+), 709 deletions(-)
delete mode 100644 arch/x86/boot/tools/.gitignore
delete mode 100644 arch/x86/boot/tools/build.c

--
2.39.2



2023-08-18 19:41:49

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 10/17] x86/boot: Drop workaround for binutils 2.14 in linker script ASSERTs

The minimum binutils version was bumped to 2.20 in 2017 so there is no
longer a need to work around quirks in older versions than that. So drop
some meaningless linker script assignments to '.' of ASSERT() return
values.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/setup.ld | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index b11c45b9e51ed90e..a05dcaa4b74cd9f8 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -56,12 +56,8 @@ SECTIONS
*(.note*)
}

- /*
- * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
- */
- . = ASSERT(_end <= 0x8000, "Setup too big!");
- . = ASSERT(hdr == 0x1f1, "The setup header has the wrong offset!");
+ ASSERT(_end <= 0x8000, "Setup too big!")
+ ASSERT(hdr == 0x1f1, "The setup header has the wrong offset!")
/* Necessary for the very-old-loader check to work... */
- . = ASSERT(__end_init <= 5*512, "init sections too big!");
-
+ ASSERT(__end_init <= 5*512, "init sections too big!")
}
--
2.39.2


2023-08-18 19:59:04

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 16/17] x86/boot: Increase section and file alignment to 4k/512

Align x86 with other EFI architectures, and increase the section
alignment to the EFI page size (4k), so that firmware is able to honour
the section permission attributes and map code read-only and data
non-executable.

There are a number of requirements that have to be taken into account:
- the sign tools get cranky when there are gaps between sections in the
file view of the image
- the virtual offset of each section must be aligned to the image's
section alignment
- the file offset *and size* of each section must be aligned to the
image's file alignment
- the image size must be aligned to the section alignment
- each section's virtual offset must be greater than or equal to the
size of the headers.

In order to meet all these requirements, while avoiding the need for
lots of padding the accommodate the .pecompat section, the latter is
placed at an arbitrary offset >= 4k in the image, but aligned to
the minimum file alignment (512 bytes). The space before the .text
section is therefore distributed between the PE header, the .setup
section and the .pecompat section, leaving no gaps in the file coverage,
making the signing tools happy.

The virtual placement of the .pecompat section is at the end of the
image. Whether or not the data gets loaded there depends on how the PE
loader interprets the EFI_IMAGE_SCN_MEM_DISCARDABLE section attribute,
but this doesn't really matter as the contents are only relevant to
mixed mode capable PE loaders anyway.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/Makefile | 1 +
arch/x86/boot/compressed/vmlinux.lds.S | 4 +-
arch/x86/boot/header.S | 81 +++++++++--------
arch/x86/boot/setup.ld | 3 +-
arch/x86/boot/tools/build.c | 91 --------------------
5 files changed, 51 insertions(+), 129 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 50c50fce646e2417..18548e351ffb4867 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -68,6 +68,7 @@ targets += cpustr.h
KBUILD_CFLAGS := $(REALMODE_CFLAGS) -D_SETUP
KBUILD_AFLAGS := $(KBUILD_CFLAGS) -D__ASSEMBLY__
KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
+KBUILD_CFLAGS += $(call cc-option,-Oz)
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-option,-Oz)
GCOV_PROFILE := n
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 5326f3b441948c5d..3df57cdf500375f2 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -43,13 +43,13 @@ SECTIONS
*(.rodata.*)
_erodata = . ;
}
- .data : {
+ .data : ALIGN(0x1000) {
_data = . ;
*(.data)
*(.data.*)

/* add 4 bytes of extra space for a CRC-32 checksum */
- . = ALIGN(. + 4, 0x20);
+ . = ALIGN(. + 4, 0x200);
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 25dda40dacb52292..695ce5344350a4db 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -40,6 +40,9 @@ SYSSEG = 0x1000 /* historical load address >> 4 */
.globl setup_size
.set setup_size, 0x4000

+ .set salign, 0x1000
+ .set falign, 0x200
+
.code16
.section ".bstext", "ax"
#ifdef CONFIG_EFI_STUB
@@ -86,7 +89,7 @@ optional_header:

.long setup_size + ZO_efi_pe_entry # AddressOfEntryPoint

- .long 0x0200 # BaseOfCode
+ .long setup_size # BaseOfCode
#ifdef CONFIG_X86_32
.long 0 # data
#endif
@@ -97,8 +100,8 @@ extra_header_fields:
#else
.quad 0 # ImageBase
#endif
- .long 0x20 # SectionAlignment
- .long 0x20 # FileAlignment
+ .long salign # SectionAlignment
+ .long falign # FileAlignment
.word 0 # MajorOperatingSystemVersion
.word 0 # MinorOperatingSystemVersion
.word LINUX_EFISTUB_MAJOR_VERSION # MajorImageVersion
@@ -107,9 +110,10 @@ extra_header_fields:
.word 0 # MinorSubsystemVersion
.long 0 # Win32VersionValue

- .long setup_size + ZO__end # SizeOfImage
+ .long setup_size + ZO__end + pecompat_vsize
+ # SizeOfImage

- .long 0x200 # SizeOfHeaders
+ .long salign # SizeOfHeaders
.long 0 # CheckSum
.word IMAGE_SUBSYSTEM_EFI_APPLICATION # Subsystem (EFI application)
#ifdef CONFIG_EFI_DXE_MEM_ATTRIBUTES
@@ -140,44 +144,51 @@ extra_header_fields:

# Section table
section_table:
- #
- # The offset & size fields are filled in by build.c.
- #
.ascii ".setup"
.byte 0
.byte 0
- .long 0
- .long 0x0 # startup_{32,64}
- .long 0 # Size of initialized data
- # on disk
- .long 0x0 # startup_{32,64}
- .long 0 # PointerToRelocations
- .long 0 # PointerToLineNumbers
- .word 0 # NumberOfRelocations
- .word 0 # NumberOfLineNumbers
- .long IMAGE_SCN_CNT_CODE | \
- IMAGE_SCN_MEM_READ | \
- IMAGE_SCN_MEM_EXECUTE # Characteristics
+ .long setup_size - salign # VirtualSize
+ .long salign # VirtualAddress
+ .long pecompat_fstart - salign # SizeOfRawData
+ .long salign # PointerToRawData

-#ifdef CONFIG_EFI_MIXED
- #
- # The offset & size fields are filled in by build.c.
- #
- .asciz ".compat"
- .long 0
- .long 0x0
- .long 0 # Size of initialized data
- # on disk
- .long 0x0
- .long 0 # PointerToRelocations
- .long 0 # PointerToLineNumbers
- .word 0 # NumberOfRelocations
- .word 0 # NumberOfLineNumbers
+ .long 0, 0, 0
.long IMAGE_SCN_CNT_INITIALIZED_DATA | \
IMAGE_SCN_MEM_READ | \
IMAGE_SCN_MEM_DISCARDABLE # Characteristics
-#endif

+#ifdef CONFIG_EFI_MIXED
+ .asciz ".compat"
+
+ .long 8 # VirtualSize
+ .long setup_size + ZO__end # VirtualAddress
+ .long pecompat_fsize # SizeOfRawData
+ .long pecompat_fstart # PointerToRawData
+
+ .long 0, 0, 0
+ .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
+ IMAGE_SCN_MEM_READ | \
+ IMAGE_SCN_MEM_DISCARDABLE # Characteristics
+
+ /*
+ * Put the IA-32 machine type and the associated entry point address in
+ * the .compat section, so loaders can figure out which other execution
+ * modes this image supports.
+ */
+ .pushsection ".pecompat", "a", @progbits
+ .balign falign
+ .set pecompat_vsize, salign
+ .globl pecompat_fstart
+pecompat_fstart:
+ .byte 0x1 # version
+ .byte 8 # size
+ .word IMAGE_FILE_MACHINE_I386 # PE machine type
+ .long setup_size + ZO_efi32_pe_entry # entrypoint
+ .popsection
+#else
+ .set pecompat_vsize, 0
+ .set pecompat_fstart, setup_size
+#endif
.ascii ".text"
.byte 0
.byte 0
diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index f1c14616cd80390d..e44750db4b1f2e55 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -25,7 +25,8 @@ SECTIONS
.text32 : { *(.text32) }

. = ALIGN(16);
- .rodata : { *(.rodata*) }
+ .rodata : { *(.pecompat) *(.rodata*) }
+ PROVIDE(pecompat_fsize = setup_size - pecompat_fstart);

.videocards : {
video_cards = .;
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 08065c333b482174..bc2585df100572bc 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -45,13 +45,6 @@ typedef unsigned int u32;
/* This must be large enough to hold the entire setup */
u8 buf[(SETUP_SECT_NUM+1)*512];

-#ifdef CONFIG_EFI_MIXED
-#define PECOFF_COMPAT_RESERVE 0x20
-#else
-#define PECOFF_COMPAT_RESERVE 0x0
-#endif
-
-static unsigned long efi32_pe_entry;
static unsigned long _edata;

/*----------------------------------------------------------------------*/
@@ -138,85 +131,6 @@ static void usage(void)
die("Usage: build setup system zoffset.h image");
}

-#ifdef CONFIG_EFI_STUB
-
-static void update_pecoff_section_header_fields(char *section_name, u32 vma, u32 size, u32 datasz, u32 offset)
-{
- unsigned int pe_header;
- unsigned short num_sections;
- u8 *section;
-
- pe_header = get_unaligned_le32(&buf[0x3c]);
- num_sections = get_unaligned_le16(&buf[pe_header + 6]);
-
-#ifdef CONFIG_X86_32
- section = &buf[pe_header + 0xa8];
-#else
- section = &buf[pe_header + 0xb8];
-#endif
-
- while (num_sections > 0) {
- if (strncmp((char*)section, section_name, 8) == 0) {
- /* section header size field */
- put_unaligned_le32(size, section + 0x8);
-
- /* section header vma field */
- put_unaligned_le32(vma, section + 0xc);
-
- /* section header 'size of initialised data' field */
- put_unaligned_le32(datasz, section + 0x10);
-
- /* section header 'file offset' field */
- put_unaligned_le32(offset, section + 0x14);
-
- break;
- }
- section += 0x28;
- num_sections--;
- }
-}
-
-static void update_pecoff_section_header(char *section_name, u32 offset, u32 size)
-{
- update_pecoff_section_header_fields(section_name, offset, size, size, offset);
-}
-
-static void update_pecoff_setup(unsigned int size)
-{
- u32 setup_offset = 0x200;
- u32 compat_offset = size - PECOFF_COMPAT_RESERVE;
- u32 setup_size = compat_offset - setup_offset;
-
- update_pecoff_section_header(".setup", setup_offset, setup_size);
-
-#ifdef CONFIG_EFI_MIXED
- update_pecoff_section_header(".compat", compat_offset, PECOFF_COMPAT_RESERVE);
-
- /*
- * Put the IA-32 machine type (0x14c) and the associated entry point
- * address in the .compat section, so loaders can figure out which other
- * execution modes this image supports.
- */
- buf[compat_offset] = 0x1;
- buf[compat_offset + 1] = 0x8;
- put_unaligned_le16(0x14c, &buf[compat_offset + 2]);
- put_unaligned_le32(efi32_pe_entry + size, &buf[compat_offset + 4]);
-#endif
-}
-
-#else
-
-static inline void update_pecoff_setup(unsigned int size) {}
-
-#endif /* CONFIG_EFI_STUB */
-
-static int reserve_pecoff_compat_section(int c)
-{
- /* Reserve 0x20 bytes for .compat section */
- memset(buf+c, 0, PECOFF_COMPAT_RESERVE);
- return PECOFF_COMPAT_RESERVE;
-}
-
/*
* Parse zoffset.h and find the entry points. We could just #include zoffset.h
* but that would mean tools/build would have to be rebuilt every time. It's
@@ -245,7 +159,6 @@ static void parse_zoffset(char *fname)
p = (char *)buf;

while (p && *p) {
- PARSE_ZOFS(p, efi32_pe_entry);
PARSE_ZOFS(p, _edata);

p = strchr(p, '\n');
@@ -285,8 +198,6 @@ int main(int argc, char ** argv)
die("Boot block hasn't got boot flag (0xAA55)");
fclose(file);

- c += reserve_pecoff_compat_section(c);
-
/* Pad unused space with zeros */
setup_sectors = (c + 511) / 512;
if (setup_sectors > SETUP_SECT_NUM)
@@ -295,8 +206,6 @@ int main(int argc, char ** argv)
i = setup_sectors*512;
memset(buf+c, 0, i-c);

- update_pecoff_setup(i);
-
/* Open and stat the kernel file */
fd = open(argv[2], O_RDONLY);
if (fd < 0)
--
2.39.2


2023-08-19 01:47:46

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 08/17] x86/boot: Drop references to startup_64

The x86 boot image generation tool assign a default value to startup_64
and subsequently parses the actual value from zoffset.h but it never
actually uses the value anywhere. So remove this code.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/tools/build.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index f33e45ed143765f9..0e98bc5036994715 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -89,7 +89,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE

SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))

-sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|efi32_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p'
+sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|efi32_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p'

quiet_cmd_zoffset = ZOFFSET $@
cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 660627ea6cbb46f5..14ef13fe7ab021e7 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -59,7 +59,6 @@ static unsigned long efi32_stub_entry;
static unsigned long efi64_stub_entry;
static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
-static unsigned long startup_64;
static unsigned long _end;

/*----------------------------------------------------------------------*/
@@ -263,7 +262,6 @@ static void efi_stub_defaults(void)
efi_pe_entry = 0x10;
#else
efi_pe_entry = 0x210;
- startup_64 = 0x200;
#endif
}

@@ -338,7 +336,6 @@ static void parse_zoffset(char *fname)
PARSE_ZOFS(p, efi64_stub_entry);
PARSE_ZOFS(p, efi_pe_entry);
PARSE_ZOFS(p, efi32_pe_entry);
- PARSE_ZOFS(p, startup_64);
PARSE_ZOFS(p, _end);

p = strchr(p, '\n');
--
2.39.2


2023-08-19 09:56:57

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 13/17] x86/boot: Construct PE/COFF .text section from assembler

Now that the size of the setup block is fixed and visible to the
assembler, it is possible to populate the PE/COFF header fields from the
asm code directly, instead of poking the values into the binary using
the build tool. This will make it easier to reorganize the section
layout without having to tweak the build tool in lockstep.

Note that this change results in no differences in the resulting bzImage
binary.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/header.S | 22 +++------
arch/x86/boot/tools/build.c | 47 --------------------
2 files changed, 7 insertions(+), 62 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index f1fdffc9d2ca984b..c23c5feef37e55ed 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -79,14 +79,12 @@ optional_header:
.byte 0x02 # MajorLinkerVersion
.byte 0x14 # MinorLinkerVersion

- # Filled in by build.c
- .long 0 # SizeOfCode
+ .long setup_size + ZO__end - 0x200 # SizeOfCode

.long 0 # SizeOfInitializedData
.long 0 # SizeOfUninitializedData

- # Filled in by build.c
- .long 0x0000 # AddressOfEntryPoint
+ .long setup_size + ZO_efi_pe_entry # AddressOfEntryPoint

.long 0x0200 # BaseOfCode
#ifdef CONFIG_X86_32
@@ -109,10 +107,7 @@ extra_header_fields:
.word 0 # MinorSubsystemVersion
.long 0 # Win32VersionValue

- #
- # The size of the bzImage is written in tools/build.c
- #
- .long 0 # SizeOfImage
+ .long setup_size + ZO__end # SizeOfImage

.long 0x200 # SizeOfHeaders
.long 0 # CheckSum
@@ -203,18 +198,15 @@ section_table:
IMAGE_SCN_MEM_DISCARDABLE # Characteristics
#endif

- #
- # The offset & size fields are filled in by build.c.
- #
.ascii ".text"
.byte 0
.byte 0
.byte 0
- .long 0
- .long 0x0 # startup_{32,64}
- .long 0 # Size of initialized data
+ .long ZO__end
+ .long setup_size
+ .long ZO__edata # Size of initialized data
# on disk
- .long 0x0 # startup_{32,64}
+ .long setup_size
.long 0 # PointerToRelocations
.long 0 # PointerToLineNumbers
.word 0 # NumberOfRelocations
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 082c38a097713a2d..6b6282a96c6ab24d 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -53,10 +53,8 @@ u8 buf[(SETUP_SECT_NUM+1)*512];
#define PECOFF_COMPAT_RESERVE 0x0
#endif

-static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
static unsigned long _edata;
-static unsigned long _end;

/*----------------------------------------------------------------------*/

@@ -219,32 +217,6 @@ static void update_pecoff_setup_and_reloc(unsigned int size)
#endif
}

-static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
-{
- unsigned int pe_header;
- unsigned int text_sz = file_sz - text_start;
- unsigned int bss_sz = _end - text_sz;
-
- pe_header = get_unaligned_le32(&buf[0x3c]);
-
- /*
- * Size of code: Subtract the size of the first sector (512 bytes)
- * which includes the header.
- */
- put_unaligned_le32(file_sz - 512 + bss_sz, &buf[pe_header + 0x1c]);
-
- /* Size of image */
- put_unaligned_le32(file_sz + bss_sz, &buf[pe_header + 0x50]);
-
- /*
- * Address of entry point for PE/COFF executable
- */
- put_unaligned_le32(text_start + efi_pe_entry, &buf[pe_header + 0x28]);
-
- update_pecoff_section_header_fields(".text", text_start, text_sz + bss_sz,
- text_sz, text_start);
-}
-
static int reserve_pecoff_reloc_section(int c)
{
/* Reserve 0x20 bytes for .reloc section */
@@ -252,22 +224,9 @@ static int reserve_pecoff_reloc_section(int c)
return PECOFF_RELOC_RESERVE;
}

-static void efi_stub_defaults(void)
-{
- /* Defaults for old kernel */
-#ifdef CONFIG_X86_32
- efi_pe_entry = 0x10;
-#else
- efi_pe_entry = 0x210;
-#endif
-}
-
#else

static inline void update_pecoff_setup_and_reloc(unsigned int size) {}
-static inline void update_pecoff_text(unsigned int text_start,
- unsigned int file_sz) {}
-static inline void efi_stub_defaults(void) {}

static inline int reserve_pecoff_reloc_section(int c)
{
@@ -310,10 +269,8 @@ static void parse_zoffset(char *fname)
p = (char *)buf;

while (p && *p) {
- PARSE_ZOFS(p, efi_pe_entry);
PARSE_ZOFS(p, efi32_pe_entry);
PARSE_ZOFS(p, _edata);
- PARSE_ZOFS(p, _end);

p = strchr(p, '\n');
while (p && (*p == '\r' || *p == '\n'))
@@ -331,8 +288,6 @@ int main(int argc, char ** argv)
void *kernel;
u32 crc = 0xffffffffUL;

- efi_stub_defaults();
-
if (argc != 5)
usage();
parse_zoffset(argv[3]);
@@ -380,8 +335,6 @@ int main(int argc, char ** argv)
kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
if (kernel == MAP_FAILED)
die("Unable to mmap '%s': %m", argv[2]);
- update_pecoff_text(setup_sectors * 512, i + _edata);
-

crc = partial_crc32(buf, i, crc);
if (fwrite(buf, 1, i, dest) != i)
--
2.39.2


2023-08-19 10:53:00

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 09/17] x86/boot: Set EFI handover offset directly in header asm

The offsets of the EFI handover entrypoints are available to the
assembler when constructing the header, so there is no need to set them
from the build tool afterwards.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/header.S | 18 ++++++++++++++-
arch/x86/boot/tools/build.c | 24 --------------------
2 files changed, 17 insertions(+), 25 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index 5575d0f06bab1918..72744ba440f6ea09 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -524,8 +524,24 @@ pref_address: .quad LOAD_PHYSICAL_ADDR # preferred load addr
# define INIT_SIZE VO_INIT_SIZE
#endif

+ .macro __handover_offset
+#ifndef CONFIG_EFI_HANDOVER_PROTOCOL
+ .long 0
+#elif !defined(CONFIG_X86_64)
+ .long ZO_efi32_stub_entry
+#else
+ /* Yes, this is really how we defined it :( */
+ .long ZO_efi64_stub_entry - 0x200
+#ifdef CONFIG_EFI_MIXED
+ .if ZO_efi32_stub_entry != ZO_efi64_stub_entry - 0x200
+ .error "32-bit and 64-bit EFI entry points do not match"
+ .endif
+#endif
+#endif
+ .endm
+
init_size: .long INIT_SIZE # kernel initialization size
-handover_offset: .long 0 # Filled in by build.c
+handover_offset: __handover_offset
kernel_info_offset: .long ZO_kernel_info

# End of setup header #####################################################
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 14ef13fe7ab021e7..06949754316458ce 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -55,8 +55,6 @@ u8 buf[SETUP_SECT_MAX*512];
#define PECOFF_COMPAT_RESERVE 0x0
#endif

-static unsigned long efi32_stub_entry;
-static unsigned long efi64_stub_entry;
static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
static unsigned long _end;
@@ -265,31 +263,12 @@ static void efi_stub_defaults(void)
#endif
}

-static void efi_stub_entry_update(void)
-{
- unsigned long addr = efi32_stub_entry;
-
-#ifdef CONFIG_EFI_HANDOVER_PROTOCOL
-#ifdef CONFIG_X86_64
- /* Yes, this is really how we defined it :( */
- addr = efi64_stub_entry - 0x200;
-#endif
-
-#ifdef CONFIG_EFI_MIXED
- if (efi32_stub_entry != addr)
- die("32-bit and 64-bit EFI entry points do not match\n");
-#endif
-#endif
- put_unaligned_le32(addr, &buf[0x264]);
-}
-
#else

static inline void update_pecoff_setup_and_reloc(unsigned int size) {}
static inline void update_pecoff_text(unsigned int text_start,
unsigned int file_sz) {}
static inline void efi_stub_defaults(void) {}
-static inline void efi_stub_entry_update(void) {}

static inline int reserve_pecoff_reloc_section(int c)
{
@@ -332,8 +311,6 @@ static void parse_zoffset(char *fname)
p = (char *)buf;

while (p && *p) {
- PARSE_ZOFS(p, efi32_stub_entry);
- PARSE_ZOFS(p, efi64_stub_entry);
PARSE_ZOFS(p, efi_pe_entry);
PARSE_ZOFS(p, efi32_pe_entry);
PARSE_ZOFS(p, _end);
@@ -416,7 +393,6 @@ int main(int argc, char ** argv)

update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));

- efi_stub_entry_update();

crc = partial_crc32(buf, i, crc);
if (fwrite(buf, 1, i, dest) != i)
--
2.39.2


2023-08-19 11:40:28

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 12/17] x86/boot: Derive file size from _edata symbol

Tweak the linker script so that the value of _edata represents the
decompressor binary's file size rounded up to the appropriate alignment.
This removes the need to calculate it in the build tool, and will make
it easier to refer to the file size from the header directly in
subsequent changes to the PE header layout.

While adding _edata to the sed regex that parses the compressed
vmlinux's symbol list, tweak the regex a bit for conciseness.

Note that the resulting binary is identical (for CONFIG_EFI_STUB=y
builds)

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/compressed/vmlinux.lds.S | 3 ++
arch/x86/boot/header.S | 2 +-
arch/x86/boot/tools/build.c | 30 +++++---------------
4 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index be1e8b94c93afa4a..b26e30a2d865f72d 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -90,7 +90,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE

SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))

-sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|efi32_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p'
+sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi.._stub_entry\|efi\(32\)\?_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|_edata\|z_.*\)$$/\#define ZO_\2 0x\1/p'

quiet_cmd_zoffset = ZOFFSET $@
cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 4ff6ab1b67d9b336..5326f3b441948c5d 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -47,6 +47,9 @@ SECTIONS
_data = . ;
*(.data)
*(.data.*)
+
+ /* add 4 bytes of extra space for a CRC-32 checksum */
+ . = ALIGN(. + 4, 0x20);
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index bef9265173757a5a..f1fdffc9d2ca984b 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -237,7 +237,7 @@ sentinel: .byte 0xff, 0xff /* Used to detect broken loaders */
hdr:
setup_sects: .byte (setup_size / 512) - 1
root_flags: .word ROOT_RDONLY
-syssize: .long 0 /* Filled in by build.c */
+syssize: .long ZO__edata / 16
ram_size: .word 0 /* Obsolete */
vid_mode: .word SVGA_MODE
root_dev: .word 0 /* Default to major/minor 0/0 */
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 665ce7241542e475..082c38a097713a2d 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -55,6 +55,7 @@ u8 buf[(SETUP_SECT_NUM+1)*512];

static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
+static unsigned long _edata;
static unsigned long _end;

/*----------------------------------------------------------------------*/
@@ -311,6 +312,7 @@ static void parse_zoffset(char *fname)
while (p && *p) {
PARSE_ZOFS(p, efi_pe_entry);
PARSE_ZOFS(p, efi32_pe_entry);
+ PARSE_ZOFS(p, _edata);
PARSE_ZOFS(p, _end);

p = strchr(p, '\n');
@@ -323,7 +325,6 @@ int main(int argc, char ** argv)
{
unsigned int i, sz, setup_sectors;
int c;
- u32 sys_size;
struct stat sb;
FILE *file, *dest;
int fd;
@@ -372,24 +373,14 @@ int main(int argc, char ** argv)
die("Unable to open `%s': %m", argv[2]);
if (fstat(fd, &sb))
die("Unable to stat `%s': %m", argv[2]);
- sz = sb.st_size;
+ if (_edata != sb.st_size)
+ die("Unexpected file size `%s': %u != %u", argv[2], _edata,
+ sb.st_size);
+ sz = _edata - 4;
kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
if (kernel == MAP_FAILED)
die("Unable to mmap '%s': %m", argv[2]);
- /* Number of 16-byte paragraphs, including space for a 4-byte CRC */
- sys_size = (sz + 15 + 4) / 16;
-#ifdef CONFIG_EFI_STUB
- /*
- * COFF requires minimum 32-byte alignment of sections, and
- * adding a signature is problematic without that alignment.
- */
- sys_size = (sys_size + 1) & ~1;
-#endif
-
- /* Patch the setup code with the appropriate size parameters */
- put_unaligned_le32(sys_size, &buf[0x1f4]);
-
- update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));
+ update_pecoff_text(setup_sectors * 512, i + _edata);


crc = partial_crc32(buf, i, crc);
@@ -401,13 +392,6 @@ int main(int argc, char ** argv)
if (fwrite(kernel, 1, sz, dest) != sz)
die("Writing kernel failed");

- /* Add padding leaving 4 bytes for the checksum */
- while (sz++ < (sys_size*16) - 4) {
- crc = partial_crc32_one('\0', crc);
- if (fwrite("\0", 1, 1, dest) != 1)
- die("Writing padding failed");
- }
-
/* Write the CRC */
put_unaligned_le32(crc, buf);
if (fwrite(buf, 1, 4, dest) != 4)
--
2.39.2


2023-08-19 16:01:48

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 05/17] x86/boot: Omit compression buffer from PE/COFF image memory footprint

Now that the EFI stub decompresses the kernel and hands over to the
decompressed image directly, there is no longer a need to provide a
decompression buffer as part of the .BSS allocation of the PE/COFF
image. It also means the PE/COFF image can be loaded anywhere in memory,
and setting the preferred image base is unnecessary. So drop the
handling of this from the header and from the build tool.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/header.S | 6 +--
arch/x86/boot/tools/build.c | 50 +++-----------------
2 files changed, 8 insertions(+), 48 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index b24fa50a98986945..a87d9133384b0986 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -90,12 +90,10 @@ optional_header:
#endif

extra_header_fields:
- # PE specification requires ImageBase to be 64k aligned
- .set image_base, (LOAD_PHYSICAL_ADDR + 0xffff) & ~0xffff
#ifdef CONFIG_X86_32
- .long image_base # ImageBase
+ .long 0 # ImageBase
#else
- .quad image_base # ImageBase
+ .quad 0 # ImageBase
#endif
.long 0x20 # SectionAlignment
.long 0x20 # FileAlignment
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index bd247692b70174f0..0354c223e35492b6 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -65,7 +65,6 @@ static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
static unsigned long kernel_info;
static unsigned long startup_64;
-static unsigned long _ehead;
static unsigned long _end;

/*----------------------------------------------------------------------*/
@@ -229,27 +228,14 @@ static void update_pecoff_setup_and_reloc(unsigned int size)
#endif
}

-static void update_pecoff_text(unsigned int text_start, unsigned int file_sz,
- unsigned int init_sz)
+static void update_pecoff_text(unsigned int text_start, unsigned int file_sz)
{
unsigned int pe_header;
unsigned int text_sz = file_sz - text_start;
- unsigned int bss_sz = init_sz - file_sz;
+ unsigned int bss_sz = _end - text_sz;

pe_header = get_unaligned_le32(&buf[0x3c]);

- /*
- * The PE/COFF loader may load the image at an address which is
- * misaligned with respect to the kernel_alignment field in the setup
- * header.
- *
- * In order to avoid relocating the kernel to correct the misalignment,
- * add slack to allow the buffer to be aligned within the declared size
- * of the image.
- */
- bss_sz += CONFIG_PHYSICAL_ALIGN;
- init_sz += CONFIG_PHYSICAL_ALIGN;
-
/*
* Size of code: Subtract the size of the first sector (512 bytes)
* which includes the header.
@@ -257,7 +243,7 @@ static void update_pecoff_text(unsigned int text_start, unsigned int file_sz,
put_unaligned_le32(file_sz - 512 + bss_sz, &buf[pe_header + 0x1c]);

/* Size of image */
- put_unaligned_le32(init_sz, &buf[pe_header + 0x50]);
+ put_unaligned_le32(file_sz + bss_sz, &buf[pe_header + 0x50]);

/*
* Address of entry point for PE/COFF executable
@@ -308,8 +294,7 @@ static void efi_stub_entry_update(void)

static inline void update_pecoff_setup_and_reloc(unsigned int size) {}
static inline void update_pecoff_text(unsigned int text_start,
- unsigned int file_sz,
- unsigned int init_sz) {}
+ unsigned int file_sz) {}
static inline void efi_stub_defaults(void) {}
static inline void efi_stub_entry_update(void) {}

@@ -360,7 +345,6 @@ static void parse_zoffset(char *fname)
PARSE_ZOFS(p, efi32_pe_entry);
PARSE_ZOFS(p, kernel_info);
PARSE_ZOFS(p, startup_64);
- PARSE_ZOFS(p, _ehead);
PARSE_ZOFS(p, _end);

p = strchr(p, '\n');
@@ -371,7 +355,7 @@ static void parse_zoffset(char *fname)

int main(int argc, char ** argv)
{
- unsigned int i, sz, setup_sectors, init_sz;
+ unsigned int i, sz, setup_sectors;
int c;
u32 sys_size;
struct stat sb;
@@ -442,31 +426,9 @@ int main(int argc, char ** argv)
buf[0x1f1] = setup_sectors-1;
put_unaligned_le32(sys_size, &buf[0x1f4]);

- init_sz = get_unaligned_le32(&buf[0x260]);
-#ifdef CONFIG_EFI_STUB
- /*
- * The decompression buffer will start at ImageBase. When relocating
- * the compressed kernel to its end, we must ensure that the head
- * section does not get overwritten. The head section occupies
- * [i, i + _ehead), and the destination is [init_sz - _end, init_sz).
- *
- * At present these should never overlap, because 'i' is at most 32k
- * because of SETUP_SECT_MAX, '_ehead' is less than 1k, and the
- * calculation of INIT_SIZE in boot/header.S ensures that
- * 'init_sz - _end' is at least 64k.
- *
- * For future-proofing, increase init_sz if necessary.
- */
-
- if (init_sz - _end < i + _ehead) {
- init_sz = (i + _ehead + _end + 4095) & ~4095;
- put_unaligned_le32(init_sz, &buf[0x260]);
- }
-#endif
- update_pecoff_text(setup_sectors * 512, i + (sys_size * 16), init_sz);
+ update_pecoff_text(setup_sectors * 512, i + (sys_size * 16));

efi_stub_entry_update();
-
/* Update kernel_info offset. */
put_unaligned_le32(kernel_info, &buf[0x268]);

--
2.39.2


2023-08-20 02:29:16

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 14/17] x86/boot: Drop PE/COFF .reloc section

Ancient buggy EFI loaders may have required a .reloc section to be
present at some point in time, but this has not been true for a long
time so the .reloc section can just be dropped.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/header.S | 20 -----------
arch/x86/boot/tools/build.c | 35 +++-----------------
2 files changed, 5 insertions(+), 50 deletions(-)

diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
index c23c5feef37e55ed..ccfb7a7d8c29275e 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -159,26 +159,6 @@ section_table:
IMAGE_SCN_MEM_READ | \
IMAGE_SCN_MEM_EXECUTE # Characteristics

- #
- # The EFI application loader requires a relocation section
- # because EFI applications must be relocatable. The .reloc
- # offset & size fields are filled in by build.c.
- #
- .ascii ".reloc"
- .byte 0
- .byte 0
- .long 0
- .long 0
- .long 0 # SizeOfRawData
- .long 0 # PointerToRawData
- .long 0 # PointerToRelocations
- .long 0 # PointerToLineNumbers
- .word 0 # NumberOfRelocations
- .word 0 # NumberOfLineNumbers
- .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
- IMAGE_SCN_MEM_READ | \
- IMAGE_SCN_MEM_DISCARDABLE # Characteristics
-
#ifdef CONFIG_EFI_MIXED
#
# The offset & size fields are filled in by build.c.
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index 6b6282a96c6ab24d..08065c333b482174 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -45,8 +45,6 @@ typedef unsigned int u32;
/* This must be large enough to hold the entire setup */
u8 buf[(SETUP_SECT_NUM+1)*512];

-#define PECOFF_RELOC_RESERVE 0x20
-
#ifdef CONFIG_EFI_MIXED
#define PECOFF_COMPAT_RESERVE 0x20
#else
@@ -183,24 +181,13 @@ static void update_pecoff_section_header(char *section_name, u32 offset, u32 siz
update_pecoff_section_header_fields(section_name, offset, size, size, offset);
}

-static void update_pecoff_setup_and_reloc(unsigned int size)
+static void update_pecoff_setup(unsigned int size)
{
u32 setup_offset = 0x200;
- u32 reloc_offset = size - PECOFF_RELOC_RESERVE - PECOFF_COMPAT_RESERVE;
-#ifdef CONFIG_EFI_MIXED
- u32 compat_offset = reloc_offset + PECOFF_RELOC_RESERVE;
-#endif
- u32 setup_size = reloc_offset - setup_offset;
+ u32 compat_offset = size - PECOFF_COMPAT_RESERVE;
+ u32 setup_size = compat_offset - setup_offset;

update_pecoff_section_header(".setup", setup_offset, setup_size);
- update_pecoff_section_header(".reloc", reloc_offset, PECOFF_RELOC_RESERVE);
-
- /*
- * Modify .reloc section contents with a single entry. The
- * relocation is applied to offset 10 of the relocation section.
- */
- put_unaligned_le32(reloc_offset + 10, &buf[reloc_offset]);
- put_unaligned_le32(10, &buf[reloc_offset + 4]);

#ifdef CONFIG_EFI_MIXED
update_pecoff_section_header(".compat", compat_offset, PECOFF_COMPAT_RESERVE);
@@ -217,21 +204,10 @@ static void update_pecoff_setup_and_reloc(unsigned int size)
#endif
}

-static int reserve_pecoff_reloc_section(int c)
-{
- /* Reserve 0x20 bytes for .reloc section */
- memset(buf+c, 0, PECOFF_RELOC_RESERVE);
- return PECOFF_RELOC_RESERVE;
-}
-
#else

-static inline void update_pecoff_setup_and_reloc(unsigned int size) {}
+static inline void update_pecoff_setup(unsigned int size) {}

-static inline int reserve_pecoff_reloc_section(int c)
-{
- return 0;
-}
#endif /* CONFIG_EFI_STUB */

static int reserve_pecoff_compat_section(int c)
@@ -310,7 +286,6 @@ int main(int argc, char ** argv)
fclose(file);

c += reserve_pecoff_compat_section(c);
- c += reserve_pecoff_reloc_section(c);

/* Pad unused space with zeros */
setup_sectors = (c + 511) / 512;
@@ -320,7 +295,7 @@ int main(int argc, char ** argv)
i = setup_sectors*512;
memset(buf+c, 0, i-c);

- update_pecoff_setup_and_reloc(i);
+ update_pecoff_setup(i);

/* Open and stat the kernel file */
fd = open(argv[2], O_RDONLY);
--
2.39.2


2023-08-20 12:16:35

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 17/17] x86/boot: Drop CRC-32 checksum and the build tool that generates it

The only remaining task carried out by the boot/tools/build.c build tool
is generating the CRC-32 checksum of the bzImage. This feature was added
in commit

7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")

without any motivation (or any commit log text, for that matter). This
checksum is not verified by any known bootloader, and given that

a) the checksum of the entire bzImage is reported by most tools (zlib,
rhash) as 0xffffffff and not 0x0 as documented,
b) the checksum is corrupted when the image is signed for secure boot,
which means that no distro ships x86 images with valid CRCs,

it seems quite unlikely that this checksum is being used, so let's just
drop it, along with the tool that generates it.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
Documentation/arch/x86/boot.rst | 10 -
arch/x86/boot/Makefile | 8 +-
arch/x86/boot/compressed/vmlinux.lds.S | 3 +-
arch/x86/boot/tools/.gitignore | 2 -
arch/x86/boot/tools/build.c | 245 --------------------
5 files changed, 4 insertions(+), 264 deletions(-)

diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
index cdbca15a4fc23833..bdcd2bebb2fe10be 100644
--- a/Documentation/arch/x86/boot.rst
+++ b/Documentation/arch/x86/boot.rst
@@ -1028,16 +1028,6 @@ Offset/size: 0x000c/4
This field contains maximal allowed type for setup_data and setup_indirect structs.


-The Image Checksum
-==================
-
-From boot protocol version 2.08 onwards the CRC-32 is calculated over
-the entire file using the characteristic polynomial 0x04C11DB7 and an
-initial remainder of 0xffffffff. The checksum is appended to the
-file; therefore the CRC of the file up to the limit specified in the
-syssize field of the header is always 0.
-
-
The Kernel Command Line
=======================

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 18548e351ffb4867..51f70500d38a10ae 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -48,7 +48,6 @@ setup-y += video-vesa.o
setup-y += video-bios.o

targets += $(setup-y)
-hostprogs := tools/build
hostprogs += mkcpustr

HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
@@ -77,11 +76,10 @@ UBSAN_SANITIZE := n
$(obj)/bzImage: asflags-y := $(SVGA_MODE)

quiet_cmd_image = BUILD $@
-silent_redirect_image = >/dev/null
-cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
- $(obj)/zoffset.h $@ $($(quiet)redirect_image)
+ cmd_image = truncate -s %4K $(obj)/setup.bin; \
+ cat $(obj)/setup.bin $(obj)/vmlinux.bin >$@

-$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
+$(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin FORCE
$(call if_changed,image)
@$(kecho) 'Kernel: $@ is ready' ' (#'$(or $(KBUILD_BUILD_VERSION),`cat .version`)')'

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 3df57cdf500375f2..48d0b51845571e7e 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -48,8 +48,7 @@ SECTIONS
*(.data)
*(.data.*)

- /* add 4 bytes of extra space for a CRC-32 checksum */
- . = ALIGN(. + 4, 0x200);
+ . = ALIGN(0x200);
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/arch/x86/boot/tools/.gitignore b/arch/x86/boot/tools/.gitignore
deleted file mode 100644
index ae91f4d0d78b56af..0000000000000000
--- a/arch/x86/boot/tools/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only
-build
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
deleted file mode 100644
index bc2585df100572bc..0000000000000000
--- a/arch/x86/boot/tools/build.c
+++ /dev/null
@@ -1,245 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 1991, 1992 Linus Torvalds
- * Copyright (C) 1997 Martin Mares
- * Copyright (C) 2007 H. Peter Anvin
- */
-
-/*
- * This file builds a disk-image from three different files:
- *
- * - setup: 8086 machine code, sets up system parm
- * - system: 80386 code for actual system
- * - zoffset.h: header with ZO_* defines
- *
- * It does some checking that all files are of the correct type, and writes
- * the result to the specified destination, removing headers and padding to
- * the right amount. It also writes some system data to stdout.
- */
-
-/*
- * Changes by tytso to allow root device specification
- * High loaded stuff by Hans Lermen & Werner Almesberger, Feb. 1996
- * Cross compiling fixes by Gertjan van Wingerde, July 1996
- * Rewritten by Martin Mares, April 1997
- * Substantially overhauled by H. Peter Anvin, April 2007
- */
-
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <sys/mman.h>
-#include <tools/le_byteshift.h>
-
-typedef unsigned char u8;
-typedef unsigned short u16;
-typedef unsigned int u32;
-
-#define SETUP_SECT_NUM 32
-
-/* This must be large enough to hold the entire setup */
-u8 buf[(SETUP_SECT_NUM+1)*512];
-
-static unsigned long _edata;
-
-/*----------------------------------------------------------------------*/
-
-static const u32 crctab32[] = {
- 0x00000000, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419,
- 0x706af48f, 0xe963a535, 0x9e6495a3, 0x0edb8832, 0x79dcb8a4,
- 0xe0d5e91e, 0x97d2d988, 0x09b64c2b, 0x7eb17cbd, 0xe7b82d07,
- 0x90bf1d91, 0x1db71064, 0x6ab020f2, 0xf3b97148, 0x84be41de,
- 0x1adad47d, 0x6ddde4eb, 0xf4d4b551, 0x83d385c7, 0x136c9856,
- 0x646ba8c0, 0xfd62f97a, 0x8a65c9ec, 0x14015c4f, 0x63066cd9,
- 0xfa0f3d63, 0x8d080df5, 0x3b6e20c8, 0x4c69105e, 0xd56041e4,
- 0xa2677172, 0x3c03e4d1, 0x4b04d447, 0xd20d85fd, 0xa50ab56b,
- 0x35b5a8fa, 0x42b2986c, 0xdbbbc9d6, 0xacbcf940, 0x32d86ce3,
- 0x45df5c75, 0xdcd60dcf, 0xabd13d59, 0x26d930ac, 0x51de003a,
- 0xc8d75180, 0xbfd06116, 0x21b4f4b5, 0x56b3c423, 0xcfba9599,
- 0xb8bda50f, 0x2802b89e, 0x5f058808, 0xc60cd9b2, 0xb10be924,
- 0x2f6f7c87, 0x58684c11, 0xc1611dab, 0xb6662d3d, 0x76dc4190,
- 0x01db7106, 0x98d220bc, 0xefd5102a, 0x71b18589, 0x06b6b51f,
- 0x9fbfe4a5, 0xe8b8d433, 0x7807c9a2, 0x0f00f934, 0x9609a88e,
- 0xe10e9818, 0x7f6a0dbb, 0x086d3d2d, 0x91646c97, 0xe6635c01,
- 0x6b6b51f4, 0x1c6c6162, 0x856530d8, 0xf262004e, 0x6c0695ed,
- 0x1b01a57b, 0x8208f4c1, 0xf50fc457, 0x65b0d9c6, 0x12b7e950,
- 0x8bbeb8ea, 0xfcb9887c, 0x62dd1ddf, 0x15da2d49, 0x8cd37cf3,
- 0xfbd44c65, 0x4db26158, 0x3ab551ce, 0xa3bc0074, 0xd4bb30e2,
- 0x4adfa541, 0x3dd895d7, 0xa4d1c46d, 0xd3d6f4fb, 0x4369e96a,
- 0x346ed9fc, 0xad678846, 0xda60b8d0, 0x44042d73, 0x33031de5,
- 0xaa0a4c5f, 0xdd0d7cc9, 0x5005713c, 0x270241aa, 0xbe0b1010,
- 0xc90c2086, 0x5768b525, 0x206f85b3, 0xb966d409, 0xce61e49f,
- 0x5edef90e, 0x29d9c998, 0xb0d09822, 0xc7d7a8b4, 0x59b33d17,
- 0x2eb40d81, 0xb7bd5c3b, 0xc0ba6cad, 0xedb88320, 0x9abfb3b6,
- 0x03b6e20c, 0x74b1d29a, 0xead54739, 0x9dd277af, 0x04db2615,
- 0x73dc1683, 0xe3630b12, 0x94643b84, 0x0d6d6a3e, 0x7a6a5aa8,
- 0xe40ecf0b, 0x9309ff9d, 0x0a00ae27, 0x7d079eb1, 0xf00f9344,
- 0x8708a3d2, 0x1e01f268, 0x6906c2fe, 0xf762575d, 0x806567cb,
- 0x196c3671, 0x6e6b06e7, 0xfed41b76, 0x89d32be0, 0x10da7a5a,
- 0x67dd4acc, 0xf9b9df6f, 0x8ebeeff9, 0x17b7be43, 0x60b08ed5,
- 0xd6d6a3e8, 0xa1d1937e, 0x38d8c2c4, 0x4fdff252, 0xd1bb67f1,
- 0xa6bc5767, 0x3fb506dd, 0x48b2364b, 0xd80d2bda, 0xaf0a1b4c,
- 0x36034af6, 0x41047a60, 0xdf60efc3, 0xa867df55, 0x316e8eef,
- 0x4669be79, 0xcb61b38c, 0xbc66831a, 0x256fd2a0, 0x5268e236,
- 0xcc0c7795, 0xbb0b4703, 0x220216b9, 0x5505262f, 0xc5ba3bbe,
- 0xb2bd0b28, 0x2bb45a92, 0x5cb36a04, 0xc2d7ffa7, 0xb5d0cf31,
- 0x2cd99e8b, 0x5bdeae1d, 0x9b64c2b0, 0xec63f226, 0x756aa39c,
- 0x026d930a, 0x9c0906a9, 0xeb0e363f, 0x72076785, 0x05005713,
- 0x95bf4a82, 0xe2b87a14, 0x7bb12bae, 0x0cb61b38, 0x92d28e9b,
- 0xe5d5be0d, 0x7cdcefb7, 0x0bdbdf21, 0x86d3d2d4, 0xf1d4e242,
- 0x68ddb3f8, 0x1fda836e, 0x81be16cd, 0xf6b9265b, 0x6fb077e1,
- 0x18b74777, 0x88085ae6, 0xff0f6a70, 0x66063bca, 0x11010b5c,
- 0x8f659eff, 0xf862ae69, 0x616bffd3, 0x166ccf45, 0xa00ae278,
- 0xd70dd2ee, 0x4e048354, 0x3903b3c2, 0xa7672661, 0xd06016f7,
- 0x4969474d, 0x3e6e77db, 0xaed16a4a, 0xd9d65adc, 0x40df0b66,
- 0x37d83bf0, 0xa9bcae53, 0xdebb9ec5, 0x47b2cf7f, 0x30b5ffe9,
- 0xbdbdf21c, 0xcabac28a, 0x53b39330, 0x24b4a3a6, 0xbad03605,
- 0xcdd70693, 0x54de5729, 0x23d967bf, 0xb3667a2e, 0xc4614ab8,
- 0x5d681b02, 0x2a6f2b94, 0xb40bbe37, 0xc30c8ea1, 0x5a05df1b,
- 0x2d02ef8d
-};
-
-static u32 partial_crc32_one(u8 c, u32 crc)
-{
- return crctab32[(crc ^ c) & 0xff] ^ (crc >> 8);
-}
-
-static u32 partial_crc32(const u8 *s, int len, u32 crc)
-{
- while (len--)
- crc = partial_crc32_one(*s++, crc);
- return crc;
-}
-
-static void die(const char * str, ...)
-{
- va_list args;
- va_start(args, str);
- vfprintf(stderr, str, args);
- va_end(args);
- fputc('\n', stderr);
- exit(1);
-}
-
-static void usage(void)
-{
- die("Usage: build setup system zoffset.h image");
-}
-
-/*
- * Parse zoffset.h and find the entry points. We could just #include zoffset.h
- * but that would mean tools/build would have to be rebuilt every time. It's
- * not as if parsing it is hard...
- */
-#define PARSE_ZOFS(p, sym) do { \
- if (!strncmp(p, "#define ZO_" #sym " ", 11+sizeof(#sym))) \
- sym = strtoul(p + 11 + sizeof(#sym), NULL, 16); \
-} while (0)
-
-static void parse_zoffset(char *fname)
-{
- FILE *file;
- char *p;
- int c;
-
- file = fopen(fname, "r");
- if (!file)
- die("Unable to open `%s': %m", fname);
- c = fread(buf, 1, sizeof(buf) - 1, file);
- if (ferror(file))
- die("read-error on `zoffset.h'");
- fclose(file);
- buf[c] = 0;
-
- p = (char *)buf;
-
- while (p && *p) {
- PARSE_ZOFS(p, _edata);
-
- p = strchr(p, '\n');
- while (p && (*p == '\r' || *p == '\n'))
- p++;
- }
-}
-
-int main(int argc, char ** argv)
-{
- unsigned int i, sz, setup_sectors;
- int c;
- struct stat sb;
- FILE *file, *dest;
- int fd;
- void *kernel;
- u32 crc = 0xffffffffUL;
-
- if (argc != 5)
- usage();
- parse_zoffset(argv[3]);
-
- dest = fopen(argv[4], "w");
- if (!dest)
- die("Unable to write `%s': %m", argv[4]);
-
- /* Copy the setup code */
- file = fopen(argv[1], "r");
- if (!file)
- die("Unable to open `%s': %m", argv[1]);
- c = fread(buf, 1, sizeof(buf), file);
- if (ferror(file))
- die("read-error on `setup'");
- if (c < 1024)
- die("The setup must be at least 1024 bytes");
- if (get_unaligned_le16(&buf[510]) != 0xAA55)
- die("Boot block hasn't got boot flag (0xAA55)");
- fclose(file);
-
- /* Pad unused space with zeros */
- setup_sectors = (c + 511) / 512;
- if (setup_sectors > SETUP_SECT_NUM)
- die("setup size exceeds maximum");
- setup_sectors = SETUP_SECT_NUM;
- i = setup_sectors*512;
- memset(buf+c, 0, i-c);
-
- /* Open and stat the kernel file */
- fd = open(argv[2], O_RDONLY);
- if (fd < 0)
- die("Unable to open `%s': %m", argv[2]);
- if (fstat(fd, &sb))
- die("Unable to stat `%s': %m", argv[2]);
- if (_edata != sb.st_size)
- die("Unexpected file size `%s': %u != %u", argv[2], _edata,
- sb.st_size);
- sz = _edata - 4;
- kernel = mmap(NULL, sz, PROT_READ, MAP_SHARED, fd, 0);
- if (kernel == MAP_FAILED)
- die("Unable to mmap '%s': %m", argv[2]);
-
- crc = partial_crc32(buf, i, crc);
- if (fwrite(buf, 1, i, dest) != i)
- die("Writing setup failed");
-
- /* Copy the kernel code */
- crc = partial_crc32(kernel, sz, crc);
- if (fwrite(kernel, 1, sz, dest) != sz)
- die("Writing kernel failed");
-
- /* Write the CRC */
- put_unaligned_le32(crc, buf);
- if (fwrite(buf, 1, 4, dest) != 4)
- die("Writing CRC failed");
-
- /* Catch any delayed write failures */
- if (fclose(dest))
- die("Writing image failed");
-
- close(fd);
-
- /* Everything is OK */
- return 0;
-}
--
2.39.2


2023-08-20 20:14:38

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH 01/17] x86/efi: Drop EFI stub .bss from .data section

Now that the EFI stub always zero inits its BSS section upon entry,
there is no longer a need to place the BSS symbols carried by the stub
into the .data section.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/compressed/vmlinux.lds.S | 1 -
drivers/firmware/efi/libstub/Makefile | 7 -------
2 files changed, 8 deletions(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index b22f34b8684a725b..4ff6ab1b67d9b336 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -47,7 +47,6 @@ SECTIONS
_data = . ;
*(.data)
*(.data.*)
- *(.bss.efistub)
_edata = . ;
}
. = ALIGN(L1_CACHE_BYTES);
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index ae8874401a9f1490..8302ba66ef0b3d45 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -108,13 +108,6 @@ lib-y := $(patsubst %.o,%.stub.o,$(lib-y))
# https://bugs.llvm.org/show_bug.cgi?id=46480
STUBCOPY_FLAGS-y += --remove-section=.note.gnu.property

-#
-# For x86, bootloaders like systemd-boot or grub-efi do not zero-initialize the
-# .bss section, so the .bss section of the EFI stub needs to be included in the
-# .data section of the compressed kernel to ensure initialization. Rename the
-# .bss section here so it's easy to pick out in the linker script.
-#
-STUBCOPY_FLAGS-$(CONFIG_X86) += --rename-section .bss=.bss.efistub,load,alloc
STUBCOPY_RELOC-$(CONFIG_X86_32) := R_386_32
STUBCOPY_RELOC-$(CONFIG_X86_64) := R_X86_64_64

--
2.39.2


2023-08-21 12:12:02

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH 17/17] x86/boot: Drop CRC-32 checksum and the build tool that generates it

On Sun, 20 Aug 2023 at 03:03, H. Peter Anvin <[email protected]> wrote:
>
>
>
> On 8/18/23 06:44, Ard Biesheuvel wrote:
> > The only remaining task carried out by the boot/tools/build.c build tool
> > is generating the CRC-32 checksum of the bzImage. This feature was added
> > in commit
> >
> > 7d6e737c8d2698b6 ("x86: add a crc32 checksum to the kernel image.")
> >
> > without any motivation (or any commit log text, for that matter). This
> > checksum is not verified by any known bootloader, and given that
> >
> > a) the checksum of the entire bzImage is reported by most tools (zlib,
> > rhash) as 0xffffffff and not 0x0 as documented,
> > b) the checksum is corrupted when the image is signed for secure boot,
> > which means that no distro ships x86 images with valid CRCs,
> >
> > it seems quite unlikely that this checksum is being used, so let's just
> > drop it, along with the tool that generates it.
> >
>
> This one I have concerns with.
>

I understand. I deliberately put this change at the very end because I
was anticipating some debate on this.

Do you have any recollection of why this CRC32 was introduced in the
first place? The commit logs are empty and the lore thread doesn't
contain any justification either.