From 1c2da5ded11863afe00cab9cf4bf55b418758125 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 21 Jul 2026 10:35:57 -0400 Subject: [PATCH 1/4] mkosi: don't require a minimum mkosi version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently we have this in our mkosi configs: [Config] MinimumVersion=commit:dec7c3e754810ae6d2382d0b5e4762d7250b0254 The theory is this ensures we have a new enough mkosi for all the features / configs we're using to actually work. In practice this is always true, so what this config option does is break things when I try to duplicate results on a distro-packaged mkosi (i.e. not a git checkout done by the github systemd/mkosi@main action), with this not at all cryptic error message: $ mkosi sandbox -- mkosi --distribution fedora --kernel-command-line-extra=systemd.unit=mkosi-test.service qemu ‣ Cannot check mkosi git version, not running mkosi from a git repository So this patch comments it out. I'd remove it entirely, but it's useful to see which version things were intended for. Signed-off-by: Peter Jones --- mkosi/mkosi.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mkosi/mkosi.conf b/mkosi/mkosi.conf index 496efbb02..3ecb898a8 100644 --- a/mkosi/mkosi.conf +++ b/mkosi/mkosi.conf @@ -1,5 +1,5 @@ -[Config] -MinimumVersion=commit:dec7c3e754810ae6d2382d0b5e4762d7250b0254 +#[Config] +#MinimumVersion=commit:dec7c3e754810ae6d2382d0b5e4762d7250b0254 [Output] RepartDirectories=mkosi.repart From 5543c647034b8d9699760b4cba3fa46b66304704 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 16:46:14 -0400 Subject: [PATCH 2/4] post-process-pe: make it output section names Previously post-process-pe would complain like: validate_nx_compat():467: Section 9 has Virtual Address 0x00227000 that isn't section aligned (0x00010000) which is moderately useful, but stealthily tricks the user into trying to figure out if we're counting from 0 or 1, which then makes them have to resolve this by looking at the actual addresses we're complaining about to match it up. This patch changes that output to: validate_nx_compat():467: Section 9 (".rodata") has Virtual Address 0x00227000 that isn't section aligned (0x00010000) Signed-off-by: Peter Jones --- include/peimage.h | 2 ++ post-process-pe.c | 67 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/include/peimage.h b/include/peimage.h index 293ec1a7a..ba9d48f28 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -828,6 +828,8 @@ typedef struct { EFI_IMAGE_DATA_DIRECTORY *SecDir; UINT64 NumberOfRvaAndSizes; UINT16 DllCharacteristics; + UINT64 SymbolTable; + UINT16 NumberOfSymbols; EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr; } PE_COFF_LOADER_IMAGE_CONTEXT; diff --git a/post-process-pe.c b/post-process-pe.c index 4b71d8aa4..c414ae727 100644 --- a/post-process-pe.c +++ b/post-process-pe.c @@ -6,6 +6,7 @@ #define _GNU_SOURCE 1 +#include #include #include #include @@ -190,6 +191,9 @@ load_pe(const char *const file, void *const data, const size_t datasize, ctx->NumberOfSections = PEHdr->Pe32.FileHeader.NumberOfSections; + ctx->NumberOfSymbols = PEHdr->Pe32.FileHeader.NumberOfSymbols; + ctx->SymbolTable = PEHdr->Pe32.FileHeader.PointerToSymbolTable; + debug(NOISE, "Number of RVAs:%"PRIu64" EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES:%d\n", ctx->NumberOfRvaAndSizes, EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES); @@ -292,7 +296,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, errx(1, "%s: Unsupported image - Relocations have been stripped", file); if (image_is_64_bit(PEHdr)) { - ctx->ImageAddress = PEHdr->Pe32Plus.OptionalHeader.ImageBase; + ctx->ImageAddress = (uintptr_t)data + PEHdr->Pe32Plus.OptionalHeader.ImageBase; ctx->EntryPoint = PEHdr->Pe32Plus.OptionalHeader.AddressOfEntryPoint; ctx->RelocDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory @@ -300,7 +304,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, ctx->SecDir = &PEHdr->Pe32Plus.OptionalHeader.DataDirectory [EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; } else { - ctx->ImageAddress = PEHdr->Pe32.OptionalHeader.ImageBase; + ctx->ImageAddress = (uintptr_t)data + PEHdr->Pe32.OptionalHeader.ImageBase; ctx->EntryPoint = PEHdr->Pe32.OptionalHeader.AddressOfEntryPoint; ctx->RelocDir = &PEHdr->Pe32.OptionalHeader.DataDirectory @@ -363,6 +367,48 @@ set_dll_characteristics(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) ctx->DllCharacteristics = newflags; } +static int +get_section_name_offset(UINT8 section_name[8], uint32_t *section_name_offset) +{ + if (section_name[0] != '/') + return -1; + for (size_t i = 1; i < 8 && section_name[i] != '\0'; i++) { + if (!isdigit(section_name[i])) + return -1; + } + + *section_name_offset = strtoull((char *)§ion_name[1], NULL, 10); + return 0; +} + +static void +get_section_name(PE_COFF_LOADER_IMAGE_CONTEXT *ctx, + EFI_IMAGE_SECTION_HEADER *Section, + char **section_name) +{ + const uint32_t * const strtab_size = (uint32_t *)((uintptr_t)ctx->ImageAddress + + (uintptr_t)ctx->SymbolTable + + (ctx->NumberOfSymbols * EFI_IMAGE_SIZEOF_SYMBOL)); + const char * const strtab = (char *)strtab_size; + uint32_t section_name_offset; + int rc; + char tmpname[9], *newname; + + rc = get_section_name_offset(Section->Name, §ion_name_offset); + if (rc < 0) { + memcpy(tmpname, (char *)&Section->Name[0], 8); + tmpname[8] = '\0'; + newname = strdup(tmpname); + if (newname) + *section_name = newname; + return; + } + + *section_name = strdup(&strtab[section_name_offset]); + if (!*section_name) + err(5, "Couldn't allocate section name"); +} + static int validate_nx_compat(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) { @@ -400,23 +446,30 @@ validate_nx_compat(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) Section = ctx->FirstSection; for (i=0, Section = ctx->FirstSection; i < ctx->NumberOfSections; i++, Section++) { - debug(NOISE, "Section %d has WRITE=%d and EXECUTE=%d\n", i, + char *section_name = NULL; + get_section_name(ctx, Section, §ion_name); + debug(NOISE, "Section %d is \"%s\"\n", i, section_name); + debug(NOISE, "Section %d (\"%s\") has WRITE=%d and EXECUTE=%d\n", i, + section_name, (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) ? 1 : 0, (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) ? 1 : 0); if ((Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) && (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE)) { - debug(level, "Section %d is writable and executable\n", i); + debug(level, "Section %d (\"%s\") is writable and executable\n", i, + section_name); if (require_nx_compat) ret = -1; } - debug(NOISE, "Section %d has VA of 0x%08x\n", i, Section->VirtualAddress); + debug(NOISE, "Section %d (\"%s\") has VA of 0x%08x\n", i, section_name, + Section->VirtualAddress); if (Section->VirtualAddress != 0 && ((Section->VirtualAddress) & (ctx->SectionAlignment - 1))) { - debug(level, "Section %d has Virtual Address 0x%08x that isn't section aligned (0x%08x)\n", - i, Section->VirtualAddress, ctx->SectionAlignment); + debug(level, "Section %d (\"%s\") has Virtual Address 0x%08x that isn't section aligned (0x%08x)\n", + i, section_name, Section->VirtualAddress, ctx->SectionAlignment); } + free(section_name); } return ret; From 56e8e3d7b2bf8850271021ea2fe505e57490d670 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 16:46:14 -0400 Subject: [PATCH 3/4] post-process-pe: (mostly) strip COFF symbol and string tables While investigating how to make post-process-pe show section names, I discovered yet another way our objcopy magic binaries are amazing. If you dump, say, our shimx64.efi binary with objdump, you'll see something like: $ objdump -h shimx64.efi shimx64.efi: file format pei-x86-64 Sections: Idx Name Size VMA LMA File off Algn 0 .eh_frame 00038f6c 000000000000a000 000000000000a000 00001000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 1 .text 000e4d52 0000000000043000 0000000000043000 0003a000 2**4 CONTENTS, ALLOC, LOAD, READONLY, CODE 2 .data.ident 00000066 0000000000129000 0000000000129000 0011f000 2**4 CONTENTS, ALLOC, LOAD, DATA 3 .sbatlevel 0000004f 000000000012a000 000000000012a000 00120000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 4 .data 0005d128 000000000012b000 000000000012b000 00121000 2**4 CONTENTS, ALLOC, LOAD, DATA 5 .reloc 0000000c 0000000000189000 0000000000189000 0017f000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 6 .vendor_cert 000008fa 000000000018a000 000000000018a000 00180000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 7 .dynamic 00000130 000000000018b000 000000000018b000 00181000 2**2 CONTENTS, ALLOC, LOAD, DATA 8 .rela 00034de8 000000000018c000 000000000018c000 00182000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 9 .sbat 00000083 00000000001c1000 00000000001c1000 001b7000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 10 .dynsym 0001cd70 00000000001c2000 00000000001c2000 001b8000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA 11 .dynstr 0001a063 00000000001df000 00000000001df000 001d5000 2**2 CONTENTS, ALLOC, LOAD, READONLY, DATA There's no .strtab or .symtab (and objdump would hide them from you anyway...), because we post-processed this from a shared object, so everything is in .dynsym and .dynstr. So you figure the section names are in there, right? And this binary is approximately 2027619 bytes long, plus maybe a few KiB for signatures at the end. Right? But then you look harder at ".eh_frame", the first section name longer than 8 characters, has this in the raw section header: 00000188 2f 34 00 00 00 00 00 00 |/4......| 00000190 6c 8f 03 00 00 a0 00 00 00 90 03 00 00 10 00 00 |l...............| 000001a0 00 00 00 00 00 00 00 00 00 00 00 00 40 00 00 40 |............@..@| In PE, the /4 means it's at position 4 in the strings. But it's not: String section [15] '.dynstr' contains 106595 bytes at offset 0x1df000: [ 0] [ 1] ImageBase [ b] _relocate [ 15] __stack_chk_guard [ 27] SHIM_LOCK_GUID [ 36] get_variable [ 43] console_print [ 51] __stack_chk_fail Because it's in another string table that's completely hidden! Which objdump knows how to look up but doesn't tell you about because it's not in a section for no good reason. If you dump the COFF file header, you'll see: 0x8C 0x8 PointerToSymbolTable: 0x1F0000 0x90 0xC NumberOfSymbols: 0x3B1B And sure enough if you hex dump at 0x1f0000 you'll see a COFF symbol table that absolutely nothing is using, and if you dump 0x1f0000 + 0x3b1b * 18, you'll see: 002327e6 ff 4f 02 00 2e 65 68 5f 66 72 |..O...eh_fr| 002327f0 61 6d 65 00 2e 64 61 74 61 2e 69 64 65 6e 74 00 |ame..data.ident.| 00232800 2e 73 62 61 74 6c 65 76 65 6c 00 2e 76 65 6e 64 |.sbatlevel..vend| 00232810 6f 72 5f 63 65 72 74 00 64 65 62 75 67 5f 68 6f |or_cert.debug_ho| 00232820 6f 6b 00 77 61 69 74 5f 66 6f 72 5f 64 65 62 75 |ok.wait_for_debu| Which is a uint32_t that's 0x024fff and then a perfectly normal string table that starts with, that's right, .eh_frame. And if you add that length up you get 0x2577e5. And sure enough: 002577c0 61 72 69 61 62 6c 65 5f 64 61 74 61 00 6f 73 73 |ariable_data.oss| 002577d0 6c 5f 64 65 72 5f 6f 69 64 5f 63 32 74 6e 62 31 |l_der_oid_c2tnb1| 002577e0 39 31 76 31 00 |91v1.| 002577e5 That's the end of the binary. tl;dr: there's 394KiB hanging around at the end of this binary, and the only thing that's using it is using it is objdump, which is using... 49 bytes of it to show us section names. This patch strips the COFF symbol table entirely, and replaces the COFF string table with one that includes only the section names. Signed-off-by: Peter Jones --- include/peimage.h | 1 + post-process-pe.c | 106 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/include/peimage.h b/include/peimage.h index ba9d48f28..2b7123148 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -830,6 +830,7 @@ typedef struct { UINT16 DllCharacteristics; UINT64 SymbolTable; UINT16 NumberOfSymbols; + EFI_IMAGE_FILE_HEADER *FileHdr; EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr; } PE_COFF_LOADER_IMAGE_CONTEXT; diff --git a/post-process-pe.c b/post-process-pe.c index c414ae727..d74a3fe61 100644 --- a/post-process-pe.c +++ b/post-process-pe.c @@ -45,6 +45,7 @@ static int verbosity; static bool set_nx_compat = false; static bool require_nx_compat = false; +static bool strip_coff_symbols = false; typedef uint8_t UINT8; typedef uint16_t UINT16; @@ -165,6 +166,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, PEHdr->Pe32Plus.OptionalHeader.SectionAlignment; ctx->DllCharacteristics = PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; ctx->FileAlignment = PEHdr->Pe32Plus.OptionalHeader.FileAlignment; + ctx->FileHdr = &PEHdr->Pe32Plus.FileHeader; OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER64); } else { debug(NOISE, "image is 32bit\n"); @@ -176,6 +178,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, PEHdr->Pe32.OptionalHeader.SectionAlignment; ctx->DllCharacteristics = PEHdr->Pe32.OptionalHeader.DllCharacteristics; ctx->FileAlignment = PEHdr->Pe32.OptionalHeader.FileAlignment; + ctx->FileHdr = &PEHdr->Pe32.FileHeader; OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER32); } @@ -475,6 +478,80 @@ validate_nx_compat(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) return ret; } +static int +strip_coff_syms(PE_COFF_LOADER_IMAGE_CONTEXT *ctx, size_t *file_size) +{ + EFI_IMAGE_SECTION_HEADER *Section; + int i; + + uint16_t pos = 0; + char *new_strtab = NULL; + uint32_t new_strtab_size = 0; + + uint32_t old_symtab_size = ctx->NumberOfSymbols * EFI_IMAGE_SIZEOF_SYMBOL; + uint32_t old_strtab_size = 0; + uintptr_t old_strtab_location = 0; + + /* + * On arches where we don't have objcopy support for efi-app-$arch, + * and are thus using -O binary with hacks galore to build our + * binaries, there isn't a coff symbol table. + * + * We also don't have indirect symbol names for our sections, which + * nobody has ever noticed because "objdump -h" doesn't work. + * + * Just call it done on those architectures. + */ + if (ctx->SymbolTable == 0) { + debug(INFO, "This binary has no COFF symbol or string table; skipping.\n"); + return 0; + } + + char *old_symtab = (void *)((uintptr_t)ctx->ImageAddress + ctx->SymbolTable); + + Section = ctx->FirstSection; + for (i=0, Section = ctx->FirstSection; i < ctx->NumberOfSections; i++, Section++) { + char *section_name = ""; + char *tmp; + size_t len; + + get_section_name(ctx, Section, §ion_name); + + len = strlen(section_name); + + if (len > 8 || section_name[0] == '/') { + char new_name[9] = { 0, 0, 0, 0, 0, 0, 0, 0, 0 }; + + snprintf(new_name, 9, "/%hu", pos+4); + new_strtab_size += len + 1; + + tmp = realloc(new_strtab, new_strtab_size); + if (!tmp) + err(5, "Could not allocate new string table"); + strcpy(&tmp[pos], section_name); + pos += len + 1; + + new_strtab = tmp; + memcpy(Section->Name, new_name, 8); + } + free(section_name); + } + + old_strtab_location = (uintptr_t)old_symtab + old_symtab_size; + old_strtab_size = *(uint32_t *)old_strtab_location; + + debug(INFO, "Old COFF symtab:%"PRIu32" bytes strtab:%"PRIu32" bytes. New strtab:%"PRIu32" bytes.\n", + old_symtab_size, old_strtab_size, new_strtab_size); + ctx->NumberOfSymbols = 0; + ctx->FileHdr->NumberOfSymbols = 0; + *(uint32_t *)old_symtab = new_strtab_size; + memcpy(&((char *)old_symtab)[4], new_strtab, new_strtab_size); + *file_size -= old_strtab_size + old_symtab_size; + *file_size += sizeof(new_strtab_size) + new_strtab_size; + + return 0; +} + static void fix_timestamp(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) { @@ -568,6 +645,12 @@ handle_one(char *f) if (rc < 0) err(2, "NX compatibility check failed\n"); + if (strip_coff_symbols) { + rc = strip_coff_syms(&ctx, &sz); + if (rc < 0) + err(3, "Stripping COFF symbols failed\n"); + } + fix_timestamp(&ctx); fix_checksum(&ctx, map, sz); @@ -582,6 +665,13 @@ handle_one(char *f) warn("munmap(%p, %zu) failed", map, sz); failed = 1; } + + rc = ftruncate(fd, sz); + if (rc < 0) { + warn("ftruncte(%d, %zu) failed", fd, sz); + failed = 1; + } + rc = close(fd); if (rc < 0) { warn("close(%d) failed", fd); @@ -600,6 +690,8 @@ static void __attribute__((__noreturn__)) usage(int status) fprintf(out, "Options:\n"); fprintf(out, " -q Be more quiet\n"); fprintf(out, " -v Be more verbose\n"); + fprintf(out, " -C Disable stripping COFF symbols\n"); + fprintf(out, " -c Enable stripping COFF symbols\n"); fprintf(out, " -N Disable the NX compatibility flag\n"); fprintf(out, " -n Enable the NX compatibility flag\n"); fprintf(out, " -x Error on NX incompatibility\n"); @@ -618,6 +710,12 @@ int main(int argc, char **argv) {.name = "usage", .val = '?', }, + {.name = "strip-coff", + .val = 'C', + }, + {.name = "no-strip-coff", + .val = 'c', + }, {.name = "disable-nx-compat", .val = 'N', }, @@ -637,12 +735,18 @@ int main(int argc, char **argv) }; int longindex = -1; - while ((i = getopt_long(argc, argv, "hNnqvx", options, &longindex)) != -1) { + while ((i = getopt_long(argc, argv, "hCcNnqvx", options, &longindex)) != -1) { switch (i) { case 'h': case '?': usage(longindex == -1 ? 1 : 0); break; + case 'C': + strip_coff_symbols = false; + break; + case 'c': + strip_coff_symbols = true; + break; case 'N': set_nx_compat = false; break; From a52a4db4d53baf393088a01f2b5d4518d031bed9 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 20 Jul 2026 11:10:40 -0400 Subject: [PATCH 4/4] post-process-pe: move flags to Make.defaults and add -c This moves all of the flags for post-process-pe to POST_PROCESS_PE_FLAGS and sets a default of "-c -vv" in Make.defaults. Signed-off-by: Peter Jones --- Make.defaults | 2 +- Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Make.defaults b/Make.defaults index 1d2f188b8..66832d299 100644 --- a/Make.defaults +++ b/Make.defaults @@ -150,7 +150,7 @@ CFLAGS = $(FEATUREFLAGS) \ $(INCLUDES) \ $(DEFINES) -POST_PROCESS_PE_FLAGS = +POST_PROCESS_PE_FLAGS ?= -c -vv ifneq ($(origin OVERRIDE_SECURITY_POLICY), undefined) DEFINES += -DOVERRIDE_SECURITY_POLICY diff --git a/Makefile b/Makefile index 391ca1986..4db4e624d 100644 --- a/Makefile +++ b/Makefile @@ -436,7 +436,7 @@ endif --file-alignment 0x1000 \ --section-alignment $(ARCH_SECTION_ALIGNMENT) \ $(FORMAT) $< $@ - ./post-process-pe -vv $(POST_PROCESS_PE_FLAGS) $@ + ./post-process-pe $(POST_PROCESS_PE_FLAGS) $@ ifneq ($(origin ENABLE_SHIM_HASH),undefined) %.hash : %.efi