2023-09-16 00:20:47

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v3 0/8] x86/boot: Rework PE header generation

From: Ard Biesheuvel <[email protected]>

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 a lot 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.

After this series is applied, the only remaining task performed by the
build tool is generating the CRC-32. Even though this checksum is
usually wrong (given that distro kernels are signed for secure boot in a
way that corrupts the CRC), this feature is retained as we cannot be
sure that nobody is relying on this.

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.

Changes since v2:
- rebase onto tip/master
- drop patches that have been picked up already
- fix issue in the linker script that resulted in a bogus setup_size in
some cases when using ld.bfd
- fix comment capitalization

Changes since v1:
- drop patch that removed the CRC and the build tool
- do not use fixed setup_size but derive it in the setup.ld linker
script
- reorganize the PE header so the .compat section only covers its
payload and the padding that follows it
- add hpa's ack to patch #4

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]>


Ard Biesheuvel (8):
x86/boot: Grab kernel_info offset from zoffset header directly
x86/boot: Set EFI handover offset directly in header asm
x86/boot: Define setup size in linker script
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

arch/x86/boot/Makefile | 2 +-
arch/x86/boot/compressed/vmlinux.lds.S | 5 +-
arch/x86/boot/header.S | 146 +++++++------
arch/x86/boot/setup.ld | 7 +-
arch/x86/boot/tools/build.c | 223 +-------------------
5 files changed, 97 insertions(+), 286 deletions(-)

--
2.42.0.459.ge4e396fd5e-goog


2023-09-16 01:45:53

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v3 4/8] x86/boot: Derive file size from _edata symbol

From: Ard Biesheuvel <[email protected]>

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.

This change has no impact on the resulting bzImage binary when
configured with CONFIG_EFI_STUB=y.

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 0e98bc503699..cc04917b1ac6 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\|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 4ff6ab1b67d9..b688598db28e 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 06bd72a324c1..34e9b35b827c 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -233,7 +233,7 @@ sentinel: .byte 0xff, 0xff /* Used to detect broken loaders */
hdr:
.byte setup_sects - 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 745d64b6d930..e792c6c5a634 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -52,6 +52,7 @@ u8 buf[SETUP_SECT_MAX*512];

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

/*----------------------------------------------------------------------*/
@@ -308,6 +309,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');
@@ -320,7 +322,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;
@@ -368,24 +369,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);
@@ -397,13 +388,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.42.0.459.ge4e396fd5e-goog

2023-09-16 02:38:18

by Ard Biesheuvel

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

From: Ard Biesheuvel <[email protected]>

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 to accommodate the .compat section, the latter is placed
at an arbitrary offset towards the end of 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 .compat section, leaving no gaps in the file coverage, making the
signing tools happy.

Signed-off-by: Ard Biesheuvel <[email protected]>
---
arch/x86/boot/compressed/vmlinux.lds.S | 4 +-
arch/x86/boot/header.S | 75 +++++++++-------
arch/x86/boot/setup.ld | 7 +-
arch/x86/boot/tools/build.c | 90 +-------------------
4 files changed, 51 insertions(+), 125 deletions(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index b688598db28e..083ec6d7722a 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 a1f986105f00..b2771710ed98 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -36,6 +36,9 @@ SYSSEG = 0x1000 /* historical load address >> 4 */
#define ROOT_RDONLY 1
#endif

+ .set salign, 0x1000
+ .set falign, 0x200
+
.code16
.section ".bstext", "ax"
#ifdef CONFIG_EFI_STUB
@@ -82,7 +85,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
@@ -93,8 +96,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
@@ -103,9 +106,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
@@ -136,44 +140,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 | \
+ .long setup_size - salign # VirtualSize
+ .long salign # VirtualAddress
+ .long pecompat_fstart - salign # SizeOfRawData
+ .long salign # PointerToRawData
+
+ .long 0, 0, 0
+ .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
IMAGE_SCN_MEM_READ | \
- IMAGE_SCN_MEM_EXECUTE # Characteristics
+ IMAGE_SCN_MEM_DISCARDABLE # Characteristics

#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 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
-#endif

+ /*
+ * 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 6d389499565c..83bb7efad8ae 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -36,16 +36,17 @@ SECTIONS
. = ALIGN(16);
.data : { *(.data*) }

+ .pecompat : { *(.pecompat) }
+ PROVIDE(pecompat_fsize = setup_size - pecompat_fstart);
+
.signature : {
setup_sig = .;
LONG(0x5a5aaa55)

- /* Reserve some extra space for the compat section */
- setup_size = ALIGN(ABSOLUTE(.) + 32, 512);
+ setup_size = ALIGN(ABSOLUTE(.), 4096);
setup_sects = ABSOLUTE(setup_size / 512);
}

-
. = ALIGN(16);
.bss :
{
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index faccff9743a3..10311d77c67f 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -47,9 +47,6 @@ typedef unsigned int u32;
/* This must be large enough to hold the entire setup */
u8 buf[SETUP_SECT_MAX*512];

-#define PECOFF_COMPAT_RESERVE 0x20
-
-static unsigned long efi32_pe_entry;
static unsigned long _edata;

/*----------------------------------------------------------------------*/
@@ -136,85 +133,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
@@ -243,7 +161,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');
@@ -283,17 +200,14 @@ 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;
+ setup_sectors = (c + 4095) / 4096;
+ setup_sectors *= 8;
if (setup_sectors < SETUP_SECT_MIN)
setup_sectors = SETUP_SECT_MIN;
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.42.0.459.ge4e396fd5e-goog

2023-09-16 02:55:23

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v3 5/8] x86/boot: Construct PE/COFF .text section from assembler

From: Ard Biesheuvel <[email protected]>

Now that the size of the setup block is 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.

This change has no impact on 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 34e9b35b827c..2b07bc596c39 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -75,14 +75,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
@@ -105,10 +103,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
@@ -199,18 +194,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 e792c6c5a634..9712f27e32c1 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -50,10 +50,8 @@ u8 buf[SETUP_SECT_MAX*512];
#define PECOFF_RELOC_RESERVE 0x20
#define PECOFF_COMPAT_RESERVE 0x20

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

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

@@ -216,32 +214,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 */
@@ -249,22 +221,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)
{
@@ -307,10 +266,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'))
@@ -328,8 +285,6 @@ int main(int argc, char ** argv)
void *kernel;
u32 crc = 0xffffffffUL;

- efi_stub_defaults();
-
if (argc != 5)
usage();
parse_zoffset(argv[3]);
@@ -376,8 +331,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.42.0.459.ge4e396fd5e-goog

2023-09-17 17:51:55

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] x86/boot: Rework PE header generation


* Ard Biesheuvel <[email protected]> wrote:

> From: Ard Biesheuvel <[email protected]>
>
> 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 a lot 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.
>
> After this series is applied, the only remaining task performed by the
> build tool is generating the CRC-32. Even though this checksum is
> usually wrong (given that distro kernels are signed for secure boot in a
> way that corrupts the CRC), this feature is retained as we cannot be
> sure that nobody is relying on this.
>
> 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.
>
> Changes since v2:
> - rebase onto tip/master
> - drop patches that have been picked up already
> - fix issue in the linker script that resulted in a bogus setup_size in
> some cases when using ld.bfd
> - fix comment capitalization
>
> Changes since v1:
> - drop patch that removed the CRC and the build tool
> - do not use fixed setup_size but derive it in the setup.ld linker
> script
> - reorganize the PE header so the .compat section only covers its
> payload and the padding that follows it
> - add hpa's ack to patch #4
>
> 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]>
>
>
> Ard Biesheuvel (8):
> x86/boot: Grab kernel_info offset from zoffset header directly
> x86/boot: Set EFI handover offset directly in header asm
> x86/boot: Define setup size in linker script
> 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
>
> arch/x86/boot/Makefile | 2 +-
> arch/x86/boot/compressed/vmlinux.lds.S | 5 +-
> arch/x86/boot/header.S | 146 +++++++------
> arch/x86/boot/setup.ld | 7 +-
> arch/x86/boot/tools/build.c | 223 +-------------------
> 5 files changed, 97 insertions(+), 286 deletions(-)

Applied to tip:x86/boot, thanks!

Ingo

Subject: [tip: x86/boot] x86/boot: Increase section and file alignment to 4k/512

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: 3e3eabe26dc88692d34cf76ca0e0dd331481cc15
Gitweb: https://git.kernel.org/tip/3e3eabe26dc88692d34cf76ca0e0dd331481cc15
Author: Ard Biesheuvel <[email protected]>
AuthorDate: Fri, 15 Sep 2023 17:16:32
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 17 Sep 2023 19:48:44 +02:00

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 to accommodate the .compat section, the latter is placed
at an arbitrary offset towards the end of 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 .compat section, leaving no gaps in the file coverage, making the
signing tools happy.

Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/boot/compressed/vmlinux.lds.S | 4 +-
arch/x86/boot/header.S | 75 ++++++++++++---------
arch/x86/boot/setup.ld | 7 +-
arch/x86/boot/tools/build.c | 90 +-------------------------
4 files changed, 51 insertions(+), 125 deletions(-)

diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index b688598..083ec6d 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 a1f9861..b277171 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -36,6 +36,9 @@ SYSSEG = 0x1000 /* historical load address >> 4 */
#define ROOT_RDONLY 1
#endif

+ .set salign, 0x1000
+ .set falign, 0x200
+
.code16
.section ".bstext", "ax"
#ifdef CONFIG_EFI_STUB
@@ -82,7 +85,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
@@ -93,8 +96,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
@@ -103,9 +106,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
@@ -136,44 +140,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 | \
+ .long setup_size - salign # VirtualSize
+ .long salign # VirtualAddress
+ .long pecompat_fstart - salign # SizeOfRawData
+ .long salign # PointerToRawData
+
+ .long 0, 0, 0
+ .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
IMAGE_SCN_MEM_READ | \
- IMAGE_SCN_MEM_EXECUTE # Characteristics
+ IMAGE_SCN_MEM_DISCARDABLE # Characteristics

#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 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
-#endif

+ /*
+ * 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 6d38949..83bb7ef 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -36,16 +36,17 @@ SECTIONS
. = ALIGN(16);
.data : { *(.data*) }

+ .pecompat : { *(.pecompat) }
+ PROVIDE(pecompat_fsize = setup_size - pecompat_fstart);
+
.signature : {
setup_sig = .;
LONG(0x5a5aaa55)

- /* Reserve some extra space for the compat section */
- setup_size = ALIGN(ABSOLUTE(.) + 32, 512);
+ setup_size = ALIGN(ABSOLUTE(.), 4096);
setup_sects = ABSOLUTE(setup_size / 512);
}

-
. = ALIGN(16);
.bss :
{
diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
index faccff9..10311d7 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -47,9 +47,6 @@ typedef unsigned int u32;
/* This must be large enough to hold the entire setup */
u8 buf[SETUP_SECT_MAX*512];

-#define PECOFF_COMPAT_RESERVE 0x20
-
-static unsigned long efi32_pe_entry;
static unsigned long _edata;

/*----------------------------------------------------------------------*/
@@ -136,85 +133,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
@@ -243,7 +161,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');
@@ -283,17 +200,14 @@ 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;
+ setup_sectors = (c + 4095) / 4096;
+ setup_sectors *= 8;
if (setup_sectors < SETUP_SECT_MIN)
setup_sectors = SETUP_SECT_MIN;
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)

Subject: [tip: x86/boot] x86/boot: Construct PE/COFF .text section from assembler

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: efa089e63b56bdc5eca754b995cb039dd7a5457e
Gitweb: https://git.kernel.org/tip/efa089e63b56bdc5eca754b995cb039dd7a5457e
Author: Ard Biesheuvel <[email protected]>
AuthorDate: Fri, 15 Sep 2023 17:16:29
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 17 Sep 2023 19:48:43 +02:00

x86/boot: Construct PE/COFF .text section from assembler

Now that the size of the setup block is 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.

This change has no impact on the resulting bzImage binary.

Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[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 34e9b35..2b07bc5 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -75,14 +75,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
@@ -105,10 +103,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
@@ -199,18 +194,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 e792c6c..9712f27 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -50,10 +50,8 @@ u8 buf[SETUP_SECT_MAX*512];
#define PECOFF_RELOC_RESERVE 0x20
#define PECOFF_COMPAT_RESERVE 0x20

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

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

@@ -216,32 +214,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 */
@@ -249,22 +221,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)
{
@@ -307,10 +266,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'))
@@ -328,8 +285,6 @@ int main(int argc, char ** argv)
void *kernel;
u32 crc = 0xffffffffUL;

- efi_stub_defaults();
-
if (argc != 5)
usage();
parse_zoffset(argv[3]);
@@ -376,8 +331,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)

Subject: [tip: x86/boot] x86/boot: Derive file size from _edata symbol

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: aeb92067f6ae994b541d7f9752fe54ed3d108bcc
Gitweb: https://git.kernel.org/tip/aeb92067f6ae994b541d7f9752fe54ed3d108bcc
Author: Ard Biesheuvel <[email protected]>
AuthorDate: Fri, 15 Sep 2023 17:16:28
Committer: Ingo Molnar <[email protected]>
CommitterDate: Sun, 17 Sep 2023 19:48:42 +02:00

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.

This change has no impact on the resulting bzImage binary when
configured with CONFIG_EFI_STUB=y.

Signed-off-by: Ard Biesheuvel <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[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 0e98bc5..cc04917 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\|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 4ff6ab1..b688598 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 06bd72a..34e9b35 100644
--- a/arch/x86/boot/header.S
+++ b/arch/x86/boot/header.S
@@ -233,7 +233,7 @@ sentinel: .byte 0xff, 0xff /* Used to detect broken loaders */
hdr:
.byte setup_sects - 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 745d64b..e792c6c 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -52,6 +52,7 @@ u8 buf[SETUP_SECT_MAX*512];

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

/*----------------------------------------------------------------------*/
@@ -308,6 +309,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');
@@ -320,7 +322,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;
@@ -368,24 +369,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);
@@ -397,13 +388,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)