From 9cffea72259caf827abe477b1ce61b0498d6c5b2 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 21 Jul 2026 10:35:57 -0400 Subject: [PATCH 01/14] 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 dc180469892c1c2fe622fdd8a4fb9c9166c1022a Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 16:46:14 -0400 Subject: [PATCH 02/14] 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 16361bda3e92b4e2c26597be9ec2241aa15f1b2a Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 16:46:14 -0400 Subject: [PATCH 03/14] 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 79d7974cd887e9ba870a98f69c32a2ec8cb576ee Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 20 Jul 2026 11:10:40 -0400 Subject: [PATCH 04/14] 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 d7fa2a250..117ba5467 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 From e26dd55ee94d4d1a6eece6a3291f1dc0f4d3a486 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 Jul 2026 14:40:30 -0400 Subject: [PATCH 05/14] *.lds: Remove the .hash must come first comment... We've had a comment since forever, borrowed from some old ia64 code somewhere else, that inexplicably says .hash (or sometimes .gnu.hash) absolutely must come first. As far as I can tell there's no reason for it, and it isn't in any way true. I don't think it was true on Itanic, either. What *is* special is that .hash winds up /defaulting/ to having a VMA of 0 set with a file offset of 0x1000, which winds up clearing space for PE headers that are otherwise not being reserved. So when we objcopy to our PE, by virtue of being after .hash, whatever comes next lands after the PE headers. If we don't have .hash but just move . to 0x1000, that also works just fine. This patch does the following: - adds that offset and an explanation of why - removes the wrong comment In theory we could also move .hash, .gnu.hash, and .eh_frame to later in the binary (which would save a few KiB since we're not copying it), but I'm not doing that here. Signed-off-by: Peter Jones --- elf_aarch64_efi.lds | 5 ++++- elf_ia32_efi.lds | 5 ++++- elf_ia64_efi.lds | 6 +++++- elf_riscv64_efi.lds | 5 ++++- elf_x86_64_efi.lds | 5 ++++- 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/elf_aarch64_efi.lds b/elf_aarch64_efi.lds index 7f26a1457..477f43c68 100644 --- a/elf_aarch64_efi.lds +++ b/elf_aarch64_efi.lds @@ -5,7 +5,10 @@ SECTIONS { . = 0; ImageBase = .; - /* this MUST come first! */ + /* + * we have to leave space for PE headers + */ + . = 0x1000; .hash : { *(.hash) } . = ALIGN(65536); .eh_frame : diff --git a/elf_ia32_efi.lds b/elf_ia32_efi.lds index 3f22c9336..56dbac6f5 100644 --- a/elf_ia32_efi.lds +++ b/elf_ia32_efi.lds @@ -5,7 +5,10 @@ SECTIONS { . = 0; ImageBase = .; - /* .hash and/or .gnu.hash MUST come first! */ + /* + * we have to leave space for PE headers + */ + . = 0x1000; .hash : { *(.hash) } . = ALIGN(4096); .text : diff --git a/elf_ia64_efi.lds b/elf_ia64_efi.lds index a2195609c..89c175898 100644 --- a/elf_ia64_efi.lds +++ b/elf_ia64_efi.lds @@ -5,7 +5,11 @@ SECTIONS { . = 0; ImageBase = .; - .hash : { *(.hash) } /* this MUST come first! */ + /* + * we have to leave space for PE headers + */ + . = 0x1000; + .hash : { *(.hash) } . = ALIGN(4096); .text : { diff --git a/elf_riscv64_efi.lds b/elf_riscv64_efi.lds index ec7fe6440..325b87cb7 100644 --- a/elf_riscv64_efi.lds +++ b/elf_riscv64_efi.lds @@ -7,7 +7,10 @@ SECTIONS { . = 0; ImageBase = .; - /* .hash and/or .gnu.hash MUST come first! */ + /* + * we have to leave space for PE headers + */ + . = 0x1000; .hash : { *(.hash) } .gnu.hash : { *(.gnu.hash) } . = ALIGN(4096); diff --git a/elf_x86_64_efi.lds b/elf_x86_64_efi.lds index 56a1c71bf..eec9eba27 100644 --- a/elf_x86_64_efi.lds +++ b/elf_x86_64_efi.lds @@ -6,7 +6,10 @@ SECTIONS { . = 0; ImageBase = .; - /* this MUST come first! */ + /* + * we have to leave space for PE headers + */ + . = 0x1000; .hash : { *(.hash) } . = ALIGN(4096); .eh_frame : From 7a3637974ff4992f709d6d0ddad0c11ed068465d Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 8 Jul 2026 13:32:46 -0400 Subject: [PATCH 06/14] Add a basic backtrace facility. This adds a minimal backtrace facility, which is only built when we've built with ENABLE_SHIM_DEVEL. This backtrace facility depends on __builtin_return_address(1) being relatively safe (i.e. not crashing further) on the platform. The gcc documentation isn't particularly clear on when that isn't true, but it seems to be true for the platforms we generally care about. Signed-off-by: Peter Jones --- Make.defaults | 5 +- Makefile | 8 +- backtrace.c | 293 ++++++++++++++++++++++++++++++++++++++++++ elf_aarch64_efi.lds | 4 + elf_arm_efi.lds | 4 + elf_ia32_efi.lds | 4 + elf_ia64_efi.lds | 4 + elf_riscv64_efi.lds | 4 + elf_x86_64_efi.lds | 5 + include/backtrace.h | 21 +++ include/elf.h | 151 ++++++++++++++++++++++ include/elf/aa64.h | 18 +++ include/elf/arm.h | 18 +++ include/elf/elf32.h | 29 +++++ include/elf/elf64.h | 30 +++++ include/elf/ia32.h | 18 +++ include/elf/riscv64.h | 18 +++ include/elf/x64.h | 18 +++ shim.h | 1 + 19 files changed, 649 insertions(+), 4 deletions(-) create mode 100644 backtrace.c create mode 100644 include/backtrace.h create mode 100644 include/elf.h create mode 100644 include/elf/aa64.h create mode 100644 include/elf/arm.h create mode 100644 include/elf/elf32.h create mode 100644 include/elf/elf64.h create mode 100644 include/elf/ia32.h create mode 100644 include/elf/riscv64.h create mode 100644 include/elf/x64.h diff --git a/Make.defaults b/Make.defaults index 66832d299..b22e3d910 100644 --- a/Make.defaults +++ b/Make.defaults @@ -122,6 +122,7 @@ override DEFAULT_FEATUREFLAGS = \ -std=gnu11 \ -ggdb -gdwarf-4 -gstrict-dwarf \ -ffreestanding \ + -Wno-frame-address -fno-omit-frame-pointer \ $(shell $(CC) -fmacro-prefix-map=./=./ -E -x c /dev/null >/dev/null 2>&1 && echo -fmacro-prefix-map='$(TOPDIR)/=$(DEBUGSRC)') \ -fno-stack-protector \ -fno-strict-aliasing \ @@ -141,8 +142,8 @@ override DEFAULT_WERRFLAGS = \ -Werror $(call update-variable,WERRFLAGS) -CFLAGS = $(FEATUREFLAGS) \ - $(OPTIMIZATIONS) \ +CFLAGS = $(OPTIMIZATIONS) \ + $(FEATUREFLAGS) \ $(WARNFLAGS) \ $(if $(findstring clang,$(CC)),$(CLANG_WARNINGS)) \ $(ARCH_CFLAGS) \ diff --git a/Makefile b/Makefile index 117ba5467..538bc439d 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,7 @@ TARGETS += $(MMNAME) $(FBNAME) endif OBJS = shim.o \ + backtrace.o \ cert.o \ csv.o \ dp.o \ @@ -74,6 +75,7 @@ KEYS = shim_cert.h \ shim.cer \ ORIG_SOURCES = shim.c \ + backtrace.c \ cert.S \ csv.c \ dp.c \ @@ -100,6 +102,7 @@ ORIG_SOURCES = shim.c \ MOK_OBJS = MokManager.o \ crypt_blowfish.o \ + backtrace.o \ dp.o \ errlog.o \ globals.o \ @@ -116,6 +119,7 @@ ORIG_MOK_SOURCES = MokManager.c \ $(wildcard include/*.h) \ FALLBACK_OBJS = fallback.o \ + backtrace.o \ errlog.o \ globals.o \ hexdump.o \ @@ -431,7 +435,7 @@ ifneq ($(OBJCOPY_GTE224),1) endif $(OBJCOPY) -D -j .text -j .sdata -j .data -j .data.ident \ -j .dynamic -j .rodata -j .rel* \ - -j .rela* -j .dyn -j .reloc -j .eh_frame \ + -j .rela* -j .dyn* -j .reloc -j .eh_frame \ -j .vendor_cert -j .sbat -j .sbatlevel \ --file-alignment 0x1000 \ --section-alignment $(ARCH_SECTION_ALIGNMENT) \ @@ -449,7 +453,7 @@ ifneq ($(OBJCOPY_GTE224),1) endif $(OBJCOPY) -D -j .text -j .sdata -j .data \ -j .dynamic -j .rodata -j .rel* \ - -j .rela* -j .dyn -j .reloc -j .eh_frame -j .sbat \ + -j .rela* -j .dyn* -j .reloc -j .eh_frame -j .sbat \ -j .sbatlevel \ -j .debug_info -j .debug_abbrev -j .debug_aranges \ -j .debug_line -j .debug_str -j .debug_ranges \ diff --git a/backtrace.c b/backtrace.c new file mode 100644 index 000000000..11e55a9a1 --- /dev/null +++ b/backtrace.c @@ -0,0 +1,293 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * backtrace.c - sigh, backtrace. + * Copyright Peter Jones + */ + +#include "shim.h" + +#if defined(ENABLE_SHIM_DEVEL) + +#include + +#define MAX_STACK_FRAME 102400 + +extern const void ImageBase; +extern const void _text; +extern const void _data; +extern const void _dynstr, _dynstr_end; +extern const void _dynsym, _dynsym_end; + +static bool +elf_sym_matches(uintptr_t needle, elf_sym *hay) +{ + uintptr_t vma = &_text - &ImageBase; + uintptr_t start = hay->st_value; + uintptr_t end = hay->st_value + hay->st_size; + bool found = false; + + if (needle == 0) + return false; + + needle -= (uintptr_t)&_text - vma; + + if (hay && + hay->st_shndx == 3 && + start <= needle && + end >= needle) + found = true; + +#if defined(DEBUG_BACKTRACE) + dprint(L"%p %a between %p and %p\n", needle, found ? "is" : "is not", start, end); +#endif + return found; +} + +static int +get_name(elf_word st_name, char **name) +{ + uint64_t limit = &_dynstr_end - &_dynstr; + char *dynstr = (char *)&_dynstr; + + if (st_name > limit) + return -1; + + *name = &dynstr[st_name]; + return 0; +} + +#if defined(DEBUG_BACKTRACE) +static char +elf_bind_decode(uint8_t st_info) +{ + switch (ELF_ST_BIND(st_info)) { + case STB_LOCAL: + return 'l'; + case STB_GLOBAL: + return 'g'; + case STB_WEAK: + return 'w'; + default: + break; + } + return '?'; +} + +static char +elf_type_decode(uint8_t st_info) +{ + switch (ELF_ST_TYPE(st_info)) { + case STT_NOTYPE: + return ' '; + case STT_OBJECT: + return 'O'; + case STT_FUNC: + return 'F'; + case STT_SECTION: + return 'S'; + case STT_FILE: + return 'f'; + default: + break; + } + return '?'; +} +#endif /* DEBUG_BACKTRACE */ + +static int +get_symbol(uintptr_t func, char **symbol, uintptr_t *symaddr) +{ + elf_sym *sym = NULL; + +#if defined(DEBUG_BACKTRACE) + static bool once = false; + + if (!once) { + dprint(L"_dynsym:%p _dynsym_end:%p _dynstr:%p _dynstr_end:%p\n", + &_dynsym, &_dynsym_end, &_dynstr, &_dynstr_end); + once = true; + } + dprint(L"Looking for 0x%p\n", func); +#endif + for (elf_sym *hay = ((elf_sym *)&_dynsym)+1; (uintptr_t)hay < (uintptr_t)&_dynsym_end; hay++) { + if (hay->st_shndx != 3) + continue; +#if defined(DEBUG_BACKTRACE) + dprint(L"entry %p %c %c %lu 0x%lx %lu\n", + (void *)(hay->st_value), elf_bind_decode(hay->st_info), + elf_type_decode(hay->st_info), hay->st_shndx, hay->st_size, hay->st_name); +#endif + if (elf_sym_matches(func, hay)) { +#if defined(DEBUG_BACKTRACE) + dprint(L"found! %p %c %c %lu 0x%lx %lu\n", + (void *)(hay->st_value), elf_bind_decode(hay->st_info), + elf_type_decode(hay->st_info), hay->st_shndx, hay->st_size, hay->st_name); +#endif + sym = hay; + break; + } + } + if (!sym) + return -1; + + *symaddr = (uintptr_t)&ImageBase + sym->st_value; +#if defined(DEBUG_BACKTRACE) + dprint(L"_dynsym:%p ImageBase:%p %p+0x%lx:%p\n", + &_dynsym, &ImageBase, &ImageBase, sym->st_value, *symaddr); +#endif + + get_name(sym->st_name, symbol); + + return 0; +} + +void +backtrace(unsigned int skip) +{ + bool done = false; + int rc; + +#if defined(DEBUG_BACKTRACE) + dprint(L"ImageBase:%p\n", &ImageBase); + dprint(L"Backtrace .text:%p .data:%p\n", &_text, &_data); + dprint(L"_text - ImageBase: %p\n", (void *)((uintptr_t)&_text - (uintptr_t)&ImageBase)); + dprint(L"backtrace function is at %p\n", backtrace); +#endif + + do { + void *func = NULL; + char *sym = NULL; + uintptr_t addr = 0; + + switch(skip) { + case 0: + func = __builtin_return_address(0); + break; + /* + * Clang takes the "some platforms can't deal with nonzero + * arguments" thing "seriously" by just not ever + * implementing it: + * + * backtrace.c:166:11: error: calling '__builtin_return_address' with a nonzero argument is unsafe [-Werror,-Wframe-address] + * 166 | func = __builtin_return_address(1); + * | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ + */ +#ifndef __clang__ + case 1: + func = __builtin_return_address(1); + break; + case 2: + func = __builtin_return_address(2); + break; + case 3: + func = __builtin_return_address(3); + break; + case 4: + func = __builtin_return_address(4); + break; + case 5: + func = __builtin_return_address(5); + break; + case 6: + func = __builtin_return_address(6); + break; + case 7: + func = __builtin_return_address(7); + break; + case 8: + func = __builtin_return_address(8); + break; + case 9: + func = __builtin_return_address(9); + break; + case 10: + func = __builtin_return_address(10); + break; + case 11: + func = __builtin_return_address(11); + break; + case 12: + func = __builtin_return_address(12); + break; + case 13: + func = __builtin_return_address(13); + break; + case 14: + func = __builtin_return_address(14); + break; + case 15: + func = __builtin_return_address(15); + break; + case 16: + func = __builtin_return_address(16); + break; + case 17: + func = __builtin_return_address(17); + break; + case 18: + func = __builtin_return_address(18); + break; + case 19: + func = __builtin_return_address(19); + break; + case 20: + func = __builtin_return_address(20); + break; + case 21: + func = __builtin_return_address(21); + break; + case 22: + func = __builtin_return_address(22); + break; + case 23: + func = __builtin_return_address(23); + break; + case 24: + func = __builtin_return_address(24); + break; + case 25: + func = __builtin_return_address(25); + break; + case 26: + func = __builtin_return_address(26); + break; + case 27: + func = __builtin_return_address(27); + break; + case 28: + func = __builtin_return_address(28); + break; + case 29: + func = __builtin_return_address(29); + break; + case 30: + func = __builtin_return_address(30); +#endif /* __clang__ */ + done = true; + break; + } + if (func == NULL) { +#if defined(DEBUG_BACKTRACE) + dprint(L"__builtin_return_address(%u) == NULL\n", skip); +#endif + break; + } + func = __builtin_extract_return_addr(func); + if (func == NULL) { +#if defined(DEBUG_BACKTRACE) + dprint(L"__builtin_extract_return_addr(__builtin_return_address(%u)) == NULL\n", skip); +#endif + break; + } + rc = get_symbol((uintptr_t)func, &sym, &addr); + if (rc >= 0 && sym) { + dprint(L"%p (%a+0x%lx)\n", func, sym, (uintptr_t)func - addr); + } else { + dprint(L"%p\n", func); + } + skip += 1; + } while (!done); +} +#endif /* !defined(ENABLE_SHIM_DEVEL) */ + +// vim:fenc=utf-8:tw=75:noet diff --git a/elf_aarch64_efi.lds b/elf_aarch64_efi.lds index 477f43c68..9419eebb8 100644 --- a/elf_aarch64_efi.lds +++ b/elf_aarch64_efi.lds @@ -89,9 +89,13 @@ SECTIONS _sbat_size = . - _sbat; . = ALIGN(65536); + _dynsym = .; .dynsym : { *(.dynsym) } + _dynsym_end = .; . = ALIGN(65536); + _dynstr = .; .dynstr : { *(.dynstr) } + _dynstr_end = .; . = ALIGN(65536); .ignored.reloc : { diff --git a/elf_arm_efi.lds b/elf_arm_efi.lds index 66185a4fc..8d1051b4e 100644 --- a/elf_arm_efi.lds +++ b/elf_arm_efi.lds @@ -102,8 +102,12 @@ SECTIONS . = ALIGN(4096); .dyn : { + _dynsym = .; *(.dynsym) + _dynsym_end = .; + _dynstr = .; *(.dynstr) + _dynstr_end = .; _evrodata = .; . = ALIGN(4096); } diff --git a/elf_ia32_efi.lds b/elf_ia32_efi.lds index 56dbac6f5..65a0fa9ba 100644 --- a/elf_ia32_efi.lds +++ b/elf_ia32_efi.lds @@ -89,9 +89,13 @@ SECTIONS _sbat_size = . - _sbat; . = ALIGN(4096); + _dynsym = .; .dynsym : { *(.dynsym) } + _dynsym_end = .; . = ALIGN(4096); + _dynstr = .; .dynstr : { *(.dynstr) } + _dynstr_end = .; . = ALIGN(4096); /DISCARD/ : { diff --git a/elf_ia64_efi.lds b/elf_ia64_efi.lds index 89c175898..d30095ae0 100644 --- a/elf_ia64_efi.lds +++ b/elf_ia64_efi.lds @@ -94,9 +94,13 @@ SECTIONS *(.reloc) } . = ALIGN(4096); + _dynsym = .; .dynsym : { *(.dynsym) } + _dynsym_end = .; . = ALIGN(4096); + _dynstr = .; .dynstr : { *(.dynstr) } + _dynstr_end = .; /DISCARD/ : { *(.rela.plabel) diff --git a/elf_riscv64_efi.lds b/elf_riscv64_efi.lds index 325b87cb7..095cbe32e 100644 --- a/elf_riscv64_efi.lds +++ b/elf_riscv64_efi.lds @@ -140,9 +140,13 @@ SECTIONS _sbat_vsize = _esbat - _sbat; . = ALIGN(4096); + _dynsym = .; .dynsym : { *(.dynsym) } + _dynsym_end = .; . = ALIGN(4096); + _dynstr = .; .dynstr : { *(.dynstr) } + _dynstr_end = .; . = ALIGN(4096); .ignored.reloc : { diff --git a/elf_x86_64_efi.lds b/elf_x86_64_efi.lds index eec9eba27..0b22e797f 100644 --- a/elf_x86_64_efi.lds +++ b/elf_x86_64_efi.lds @@ -57,6 +57,7 @@ SECTIONS *(COMMON) *(.rel.local) } + . = ALIGN(4096); .reloc : { @@ -90,9 +91,13 @@ SECTIONS _sbat_size = . - _sbat; . = ALIGN(4096); + _dynsym = .; .dynsym : { *(.dynsym) } + _dynsym_end = .; . = ALIGN(4096); + _dynstr = .; .dynstr : { *(.dynstr) } + _dynstr_end = .; . = ALIGN(4096); .ignored.reloc : { diff --git a/include/backtrace.h b/include/backtrace.h new file mode 100644 index 000000000..988f73e50 --- /dev/null +++ b/include/backtrace.h @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * backtrace.h - sigh, backtrace. + * Copyright Peter Jones + */ + +#pragma once +#ifndef SHIM_UNIT_TEST + +#if defined(ENABLE_SHIM_DEVEL) +extern void backtrace(unsigned int skip); +#else +static inline void UNUSED +backtrace(unsigned int skip UNUSED) +{ + +} +#endif + +#endif /* SHIM_UNIT_TEST */ +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf.h b/include/elf.h new file mode 100644 index 000000000..45e18db6a --- /dev/null +++ b/include/elf.h @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf.h - definitions of elf structures we need + * Copyright Peter Jones + */ + +#pragma once + +#include + +typedef uint16_t elf32_half; +typedef int16_t elf64_half; + +typedef uint32_t elf32_word; +typedef int32_t elf32_sword; +typedef uint32_t elf64_word; +typedef int32_t elf64_sword; + +typedef uint64_t elf32_xword; +typedef int64_t elf32_sxword; +typedef uint64_t elf64_xword; +typedef int64_t elf64_sxword; + +typedef uint32_t elf32_addr; +typedef uint64_t elf64_addr; + +/* + * Legal values for d_tag (dynamic entry type). + */ +#define DT_NULL 0 /* Marks end of dynamic section */ +#define DT_NEEDED 1 /* Name of needed library */ +#define DT_PLTRELSZ 2 /* Size in bytes of PLT relocs */ +#define DT_PLTGOT 3 /* Processor defined value */ +#define DT_HASH 4 /* Address of symbol hash table */ +#define DT_STRTAB 5 /* Address of string table */ +#define DT_SYMTAB 6 /* Address of symbol table */ +#define DT_RELA 7 /* Address of Rela relocs */ +#define DT_RELASZ 8 /* Total size of Rela relocs */ +#define DT_RELAENT 9 /* Size of one Rela reloc */ +#define DT_STRSZ 10 /* Size of string table */ +#define DT_SYMENT 11 /* Size of one symbol table entry */ +#define DT_INIT 12 /* Address of init function */ +#define DT_FINI 13 /* Address of termination function */ +#define DT_SONAME 14 /* Name of shared object */ +#define DT_RPATH 15 /* Library search path (deprecated) */ +#define DT_SYMBOLIC 16 /* Start symbol search here */ +#define DT_REL 17 /* Address of Rel relocs */ +#define DT_RELSZ 18 /* Total size of Rel relocs */ +#define DT_RELENT 19 /* Size of one Rel reloc */ +#define DT_PLTREL 20 /* Type of reloc in PLT */ +#define DT_DEBUG 21 /* For debugging; unspecified */ +#define DT_TEXTREL 22 /* Reloc might modify .text */ +#define DT_JMPREL 23 /* Address of PLT relocs */ +#define DT_BIND_NOW 24 /* Process relocations of object */ +#define DT_INIT_ARRAY 25 /* Array with addresses of init fct */ +#define DT_FINI_ARRAY 26 /* Array with addresses of fini fct */ +#define DT_INIT_ARRAYSZ 27 /* Size in bytes of DT_INIT_ARRAY */ +#define DT_FINI_ARRAYSZ 28 /* Size in bytes of DT_FINI_ARRAY */ +#define DT_RUNPATH 29 /* Library search path */ +#define DT_FLAGS 30 /* Flags for the object being loaded */ +#define DT_ENCODING 32 /* Start of encoded range */ +#define DT_PREINIT_ARRAY 32 /* Array with addresses of preinit fct */ +#define DT_PREINIT_ARRAYSZ 33 /* size in bytes of DT_PREINIT_ARRAY */ +#define DT_SYMTAB_SHNDX 34 /* Address of SYMTAB_SHNDX section */ +#define DT_NUM 35 /* Number used */ + +#include "elf/aa64.h" // IWYU pragma: export +#include "elf/arm.h" // IWYU pragma: export +#include "elf/ia32.h" // IWYU pragma: export +#include "elf/riscv64.h" // IWYU pragma: export +#include "elf/x64.h" // IWYU pragma: export + +typedef struct elf_dynamic { + elf_sword d_tag; /* dynamic entry type */ + union { + elf_word d_val; /* integer value */ + elf_addr d_ptr; /* address value */ + }; +} elf_dynamic; + +struct elf32_rel { + elf_addr r_offset; /* address */ + elf_word r_info; /* type and symbol index */ +}; + +struct elf64_rel { + elf_addr r_offset; /* address */ + elf_xword r_info; /* type and symbol index */ +}; + +struct elf32_rela { + elf_addr r_offset; /* address */ + elf_word r_info; /* type and symbol index */ + elf_sword r_addend; /* addend */ +}; + +#define ELF32_R_SYM(val) ((val) >> 8) +#define ELF32_R_TYPE(val) ((val) & 0xff) +#define ELF32_R_INFO(sym, type) (((sym) << 8) + ((type) & 0xff)) + +struct elf64_rela { + elf_addr r_offset; /* address */ + elf_xword r_info; /* type and symbol index */ + elf_sxword r_addend; /* addend */ +}; + +#define ELF64_R_SYM(val) ((val) >> 32) +#define ELF64_R_TYPE(val) ((val) & 0xffffffff) +#define ELF64_R_INFO(sym, type) ((((elf64_xword) (sym)) << 32) + (type)) + +struct elf32_sym { + elf_word st_name; /* symbol name, index in string table */ + elf_addr st_value; /* symbol value (address) */ + elf_word st_size; /* symbol size */ + uint8_t st_info; /* symbol binding and type */ + uint8_t st_other; /* visibility and other data */ + elf_half st_shndx; /* symbol section index */ +}; + +#define ELF32_ST_BIND(i) ((i)>>4) +#define ELF32_ST_TYPE(i) ((i)&0xf) +#define ELF32_ST_INFO(b,t) (((b)<<4)+((t)&0xf)) + +struct elf64_sym { + elf_word st_name; /* symbol name, index in string table */ + uint8_t st_info; /* symbol binding and type */ + uint8_t st_other; /* visibility and other data */ + elf_half st_shndx; /* symbol section index */ + elf_addr st_value; /* symbol value (address) */ + elf_xword st_size; /* symbol size */ +}; + +#define ELF64_ST_BIND(i) ((i)>>4) +#define ELF64_ST_TYPE(i) ((i)&0xf) +#define ELF64_ST_INFO(b,t) (((b)<<4)+((t)&0xf)) + +#define STB_LOCAL 0 +#define STB_GLOBAL 1 +#define STB_WEAK 2 +#define STB_LOPROC 13 +#define STB_HIPROC 15 + +#define STT_NOTYPE 0 +#define STT_OBJECT 1 +#define STT_FUNC 2 +#define STT_SECTION 3 +#define STT_FILE 4 +#define STT_LOPROC 13 +#define STT_HIPROC 15 + +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/aa64.h b/include/elf/aa64.h new file mode 100644 index 000000000..95f9317ca --- /dev/null +++ b/include/elf/aa64.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf-aa64.h - aa64-specific thingimawhatsits for elf + * Copyright Peter Jones + */ + +#pragma once +#if defined(__aarch64__) + +#include "elf64.h" + +#define R_AARCH64_NONE 0 +#define REL_ARCH_NONE R_AARCH64_NONE +#define R_AARCH64_RELATIVE 1027 +#define REL_ARCH_RELATIVE R_AARCH64_RELATIVE + +#endif /* !__aarch64__ */ +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/arm.h b/include/elf/arm.h new file mode 100644 index 000000000..9e130c517 --- /dev/null +++ b/include/elf/arm.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf-arm.h - arm-specific thingimawhatsits for elf + * Copyright Peter Jones + */ + +#pragma once +#if defined(__arm__) + +#include "elf32.h" + +#define R_ARM_NONE 0 +#define REL_ARCH_NONE R_ARM_NONE +#define R_ARM_RELATIVE 8 +#define REL_ARCH_RELATIVE R_ARM_RELATIVE + +#endif /* !__arm__ */ +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/elf32.h b/include/elf/elf32.h new file mode 100644 index 000000000..1f5190757 --- /dev/null +++ b/include/elf/elf32.h @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf32.h - elf-ish definitions for 32-bit arches + * Copyright Peter Jones + */ + +#pragma once + +typedef elf32_half elf_half; +typedef elf32_word elf_word; +typedef elf32_sword elf_sword; +typedef elf32_xword elf_xword; +typedef elf32_sxword elf_sxword; +typedef elf32_addr elf_addr; + +typedef struct elf32_rel elf_rel; +typedef struct elf32_rela elf_rela; + +typedef struct elf32_sym elf_sym; + +#define ELF_R_SYM(i) ELF32_R_SYM(i) +#define ELF_R_TYPE(i) ELF32_R_TYPE(i) +#define ELF_R_INFO(sym, type) ELF32_R_INFO(sym, type) + +#define ELF_ST_BIND(i) ELF32_ST_BIND(i) +#define ELF_ST_TYPE(i) ELF32_ST_TYPE(i) +#define ELF_ST_INFO(b,t) ELF32_ST_INFO(b,t) + +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/elf64.h b/include/elf/elf64.h new file mode 100644 index 000000000..24faa02ea --- /dev/null +++ b/include/elf/elf64.h @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf64.h - elf-ish definitions for 64-bit arches + * Copyright Peter Jones + */ + +#pragma once + +typedef elf64_half elf_half; +typedef elf64_word elf_word; +typedef elf64_sword elf_sword; +typedef elf64_xword elf_xword; +typedef elf64_sxword elf_sxword; +typedef elf64_addr elf_addr; + +typedef struct elf64_rel elf_rel; +typedef struct elf64_rela elf_rela; + +typedef struct elf64_sym elf_sym; + +#define ELF_R_SYM(i) ELF64_R_SYM(i) +#define ELF_R_TYPE(i) ELF64_R_TYPE(i) +#define ELF_R_INFO(sym, type) ELF64_R_INFO(sym, type) + +#define ELF_ST_BIND(i) ELF64_ST_BIND(i) +#define ELF_ST_TYPE(i) ELF64_ST_TYPE(i) +#define ELF_ST_INFO(b,t) ELF64_ST_INFO(b,t) + +// vim:fenc=utf-8:tw=75:noet + diff --git a/include/elf/ia32.h b/include/elf/ia32.h new file mode 100644 index 000000000..e918a1dfa --- /dev/null +++ b/include/elf/ia32.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf-ia32.h - ia32-specific thingimawhatsits for elf + * Copyright Peter Jones + */ + +#pragma once +#if defined(__i386__) + +#include "elf32.h" + +#define R_386_NONE 0 +#define REL_ARCH_NONE R_386_NONE +#define R_386_RELATIVE 8 +#define REL_ARCH_RELATIVE R_386_RELATIVE + +#endif /* !__i386__ */ +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/riscv64.h b/include/elf/riscv64.h new file mode 100644 index 000000000..26fd24a0d --- /dev/null +++ b/include/elf/riscv64.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf-riscv64.h - riscv64-specific thingimawhatsits for elf + * Copyright Peter Jones + */ + +#pragma once +#if (defined(__riscv__) || defined(__riscv)) && defined(__LP64__) + +#include "elf64.h" + +#define R_RISCV_NONE 0 +#define REL_ARCH_NONE R_RISCV_NONE +#define R_RISCV_RELATIVE 3 +#define REL_ARCH_RELATIVE R_RISCV_RELATIVE + +#endif /* ! (defined(__riscv__) || defined(__riscv)) && defined(__LP64__) */ +// vim:fenc=utf-8:tw=75:noet diff --git a/include/elf/x64.h b/include/elf/x64.h new file mode 100644 index 000000000..910e46270 --- /dev/null +++ b/include/elf/x64.h @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * elf-x64.h - x64-specific thingimawhatsits for elf + * Copyright Peter Jones + */ + +#pragma once +#if defined(__x86_64__) + +#include "elf64.h" + +#define R_X86_64_NONE 0 +#define REL_ARCH_NONE R_X86_64_NONE +#define R_X86_64_RELATIVE 8 +#define REL_ARCH_RELATIVE R_X86_64_RELATIVE + +#endif /* !__x86_64__ */ +// vim:fenc=utf-8:tw=75:noet diff --git a/shim.h b/shim.h index d3d6aa32a..caaf5e207 100644 --- a/shim.h +++ b/shim.h @@ -174,6 +174,7 @@ #include "include/asm.h" #include "include/compiler.h" +#include "include/backtrace.h" #include "include/list.h" #include "include/configtable.h" #include "include/console.h" From 5da8539d4958b2962a2654d9ba2c4e26311e133c Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Mon, 29 Jun 2026 16:12:21 -0400 Subject: [PATCH 07/14] Implement stack protector. This implements support for using `gcc -fstack-protector...` in all of our .efi objects. If we're built with ENABLE_SHIM_DEVEL, this will also include a backtrace in the message to the user. Signed-off-by: Peter Jones --- Make.defaults | 2 +- Makefile | 4 +++ MokManager.c | 2 ++ fallback.c | 4 +++ include/guid.h | 7 ++++ include/stack-protector.h | 28 +++++++++++++++ lib/guid.c | 1 + shim.c | 3 ++ shim.h | 1 + stack-protector.c | 73 +++++++++++++++++++++++++++++++++++++++ 10 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 include/stack-protector.h create mode 100644 stack-protector.c diff --git a/Make.defaults b/Make.defaults index b22e3d910..21a76b129 100644 --- a/Make.defaults +++ b/Make.defaults @@ -124,7 +124,7 @@ override DEFAULT_FEATUREFLAGS = \ -ffreestanding \ -Wno-frame-address -fno-omit-frame-pointer \ $(shell $(CC) -fmacro-prefix-map=./=./ -E -x c /dev/null >/dev/null 2>&1 && echo -fmacro-prefix-map='$(TOPDIR)/=$(DEBUGSRC)') \ - -fno-stack-protector \ + -fstack-protector-strong -mstack-protector-guard=global \ -fno-strict-aliasing \ -fpic \ -fshort-wchar diff --git a/Makefile b/Makefile index 538bc439d..3cfe70ace 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,7 @@ OBJS = shim.o \ sbat.o \ sbat_data.o \ sbat_var.o \ + stack-protector.o \ time.o \ tpm.o \ utils.o \ @@ -92,6 +93,7 @@ ORIG_SOURCES = shim.c \ pe-relocate.c \ sbat.c \ sbat_var.S \ + stack-protector.c \ shim.h \ time.c \ tpm.c \ @@ -109,6 +111,7 @@ MOK_OBJS = MokManager.o \ hexdump.o \ PasswordCrypt.o \ sbat_data.o \ + stack-protector.o \ time.o \ utils.o \ @@ -124,6 +127,7 @@ FALLBACK_OBJS = fallback.o \ globals.o \ hexdump.o \ sbat_data.o \ + stack-protector.o \ time.o \ tpm.o \ utils.o diff --git a/MokManager.c b/MokManager.c index bb61f00cb..cc69eefe0 100644 --- a/MokManager.c +++ b/MokManager.c @@ -2674,6 +2674,7 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE * systab) EFI_STATUS efi_status; InitializeLib(image_handle, systab); + update_stack_guard(); setup_verbosity(); setup_rand(); @@ -2683,5 +2684,6 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, EFI_SYSTEM_TABLE * systab) efi_status = check_mok_request(image_handle); console_fini(); + BS->Exit(image_handle, efi_status, 0, NULL); return efi_status; } diff --git a/fallback.c b/fallback.c index 2aaa882e5..9fe8b1948 100644 --- a/fallback.c +++ b/fallback.c @@ -1104,6 +1104,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) EFI_STATUS efi_status; InitializeLib(image, systab); + update_stack_guard(); /* * if SHIM_DEBUG is set, wait for a debugger to attach. @@ -1115,6 +1116,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) if (EFI_ERROR(efi_status)) { console_print(L"Error: could not find loaded image: %r\n", efi_status); + BS->Exit(image, efi_status, 0, NULL); return efi_status; } @@ -1126,6 +1128,7 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) if (EFI_ERROR(efi_status)) { console_print(L"Error: could not find boot options: %r\n", efi_status); + BS->Exit(image, efi_status, 0, NULL); return efi_status; } @@ -1174,5 +1177,6 @@ efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab) RT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL); + BS->Exit(image, efi_status, 0, NULL); return EFI_SUCCESS; } diff --git a/include/guid.h b/include/guid.h index 26628d1e2..e72ef96eb 100644 --- a/include/guid.h +++ b/include/guid.h @@ -42,6 +42,13 @@ extern EFI_GUID EFI_TPM2_GUID; extern EFI_GUID EFI_CC_MEASUREMENT_PROTOCOL_GUID; extern EFI_GUID EFI_SECURE_BOOT_DB_GUID; extern EFI_GUID EFI_SIMPLE_FILE_SYSTEM_GUID; +/* + * gnu-efi defines this as a macro, but we want it as a symbol instead. + */ +#ifdef EFI_RNG_PROTOCOL_GUID +#undef EFI_RNG_PROTOCOL_GUID +#endif +extern EFI_GUID EFI_RNG_PROTOCOL_GUID; extern EFI_GUID SECURITY_PROTOCOL_GUID; extern EFI_GUID SECURITY2_PROTOCOL_GUID; extern EFI_GUID EFI_MEMORY_ATTRIBUTE_PROTOCOL_GUID; diff --git a/include/stack-protector.h b/include/stack-protector.h new file mode 100644 index 000000000..e69b558b8 --- /dev/null +++ b/include/stack-protector.h @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * stack-protector.h - support for gcc -fstack-protector + * Copyright Peter Jones + */ + +#pragma once + +extern uintptr_t __stack_chk_guard; +extern uintptr_t stack_protector_init(void); + +/* + * This call must only be called from functions that do not return. That + * is, the caller must call BS->Exit() to end the task rather than + * returning, or else the stack protector will trigger on the function + * postamble. + */ +static inline __attribute__((__always_inline__)) void +update_stack_guard(void) +{ + uintptr_t guard; + + guard = stack_protector_init(); + if (guard) + __stack_chk_guard = guard; +} + +// vim:fenc=utf-8:tw=75:noet diff --git a/lib/guid.c b/lib/guid.c index 1dc90ca90..ade94ec10 100644 --- a/lib/guid.c +++ b/lib/guid.c @@ -29,6 +29,7 @@ EFI_GUID EFI_LOADED_IMAGE_GUID = EFI_LOADED_IMAGE_PROTOCOL_GUID; EFI_GUID EFI_TPM_GUID = { 0xf541796d, 0xa62e, 0x4954, {0xa7, 0x75, 0x95, 0x84, 0xf6, 0x1b, 0x9c, 0xdd } }; EFI_GUID EFI_TPM2_GUID = { 0x607f766c, 0x7455, 0x42be, {0x93, 0x0b, 0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f } }; EFI_GUID EFI_CC_MEASUREMENT_PROTOCOL_GUID = { 0x96751a3d, 0x72f4, 0x41a6, {0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b } }; +EFI_GUID EFI_RNG_PROTOCOL_GUID = { 0x3152bca5, 0xeade, 0x433d, {0x86, 0x2e, 0xc0, 0x1c, 0xdc, 0x29, 0x1f, 0x44} }; EFI_GUID EFI_SECURE_BOOT_DB_GUID = { 0xd719b2cb, 0x3d3a, 0x4596, { 0xa3, 0xbc, 0xda, 0xd0, 0x0e, 0x67, 0x65, 0x6f } }; EFI_GUID EFI_SIMPLE_FILE_SYSTEM_GUID = SIMPLE_FILE_SYSTEM_PROTOCOL; EFI_GUID SECURITY_PROTOCOL_GUID = { 0xA46423E3, 0x4617, 0x49f1, {0xB9, 0xFF, 0xD1, 0xBF, 0xA9, 0x11, 0x58, 0x39 } }; diff --git a/shim.c b/shim.c index 2a92d4b73..a7e01430a 100644 --- a/shim.c +++ b/shim.c @@ -1185,6 +1185,8 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) * Ensure that gnu-efi functions are available */ InitializeLib(image_handle, systab); + update_stack_guard(); + setup_verbosity(); update_watchdog(); @@ -1308,5 +1310,6 @@ efi_main (EFI_HANDLE passed_image_handle, EFI_SYSTEM_TABLE *passed_systab) shim_fini(); devel_egress(EFI_ERROR(efi_status) ? EXIT_FAILURE : EXIT_SUCCESS); + BS->Exit(passed_image_handle, efi_status, 0, NULL); return efi_status; } diff --git a/shim.h b/shim.h index caaf5e207..2f9b9de10 100644 --- a/shim.h +++ b/shim.h @@ -204,6 +204,7 @@ #include "include/security_policy.h" #endif #include "include/simple_file.h" +#include "include/stack-protector.h" #include "include/str.h" #include "include/time.h" #include "include/tpm.h" diff --git a/stack-protector.c b/stack-protector.c new file mode 100644 index 000000000..1868dda3a --- /dev/null +++ b/stack-protector.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * stack-protector.c - support for gcc -fstack-protector + * Copyright Peter Jones + */ + +#include "shim.h" + +/* + * Before the first call to stack_protector_init(), it's important for the + * canary it to have /something nonzero/ and to have some variability. + * urandom would be nice, but it's not that important that it's truly + * random, just fairly hard to predict and not stable across a bunch of + * different machines. + * + * So we're just assigning it to some address that'll get relocated during + * startup. It'll get reset during initialization soon after. + */ +uintptr_t __stack_chk_guard = (uintptr_t)stack_protector_init; + +void NORETURN +__stack_chk_fail(void) +{ + do { + console_print(L"Stack corruption detected. Shutting down in 5 seconds.\n"); + backtrace(0); + usleep(5000000); + RT->ResetSystem(EfiResetShutdown, EFI_SECURITY_VIOLATION, 0, NULL); + + /* + * We should never get here, but if we do, we have to do + * /something/. + */ + wait_for_debug(); + } while (1); +} + +/* + * For reasons unknown, when we build for ia32 using: + * + * x86_64-linux-gnu-gcc -fstack-protector -mstack-protector-guard=global -m32 ... + * + * a few objects wind up referencing __stack_chk_fail_local() instead of + * __stack_chk_fail(). I really don't understand why, but... eh. + */ +void NORETURN HIDDEN +__stack_chk_fail_local (void) +{ + __stack_chk_fail (); +} + +uintptr_t +stack_protector_init(void) +{ + EFI_RNG_PROTOCOL *rng = NULL; + EFI_STATUS efi_status; + uintptr_t guard = __stack_chk_guard; + + efi_status = LibLocateProtocol(&EFI_RNG_PROTOCOL_GUID, (VOID *)&rng); + if (EFI_ERROR(efi_status)) { + perror(L"Could not find EFI RNG protocol: %r\n", efi_status); + return guard; + } + + efi_status = rng->GetRNG(rng, NULL, sizeof(guard)-1, (uint8_t *)&guard); + if (EFI_ERROR(efi_status)) { + perror(L"Could not get random data: %r\n", efi_status); + return guard; + } + return guard; +} + +// vim:fenc=utf-8:tw=75:noet From 6e2107c3729289ce98a356f7610443b84c9b9e9d Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 Jul 2026 15:44:08 -0400 Subject: [PATCH 08/14] XXX: Temporarily print backtraces on stack protector failures. --- backtrace.c | 3 --- include/backtrace.h | 8 -------- 2 files changed, 11 deletions(-) diff --git a/backtrace.c b/backtrace.c index 11e55a9a1..609c4c128 100644 --- a/backtrace.c +++ b/backtrace.c @@ -6,8 +6,6 @@ #include "shim.h" -#if defined(ENABLE_SHIM_DEVEL) - #include #define MAX_STACK_FRAME 102400 @@ -288,6 +286,5 @@ backtrace(unsigned int skip) skip += 1; } while (!done); } -#endif /* !defined(ENABLE_SHIM_DEVEL) */ // vim:fenc=utf-8:tw=75:noet diff --git a/include/backtrace.h b/include/backtrace.h index 988f73e50..5a0531903 100644 --- a/include/backtrace.h +++ b/include/backtrace.h @@ -7,15 +7,7 @@ #pragma once #ifndef SHIM_UNIT_TEST -#if defined(ENABLE_SHIM_DEVEL) extern void backtrace(unsigned int skip); -#else -static inline void UNUSED -backtrace(unsigned int skip UNUSED) -{ - -} -#endif #endif /* SHIM_UNIT_TEST */ // vim:fenc=utf-8:tw=75:noet From 6666b8e22f7489365643827ccd69dac2dea60593 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 15 Jul 2026 16:13:32 -0400 Subject: [PATCH 09/14] backtrace: stub arm out the wrong way... --- backtrace.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backtrace.c b/backtrace.c index 609c4c128..7e600d5ca 100644 --- a/backtrace.c +++ b/backtrace.c @@ -6,6 +6,7 @@ #include "shim.h" +#ifndef __arm__ #include #define MAX_STACK_FRAME 102400 @@ -286,5 +287,12 @@ backtrace(unsigned int skip) skip += 1; } while (!done); } +#else /* __arm__ */ +void +backtrace(unsigned int skip UNUSED) +{ + +} +#endif // vim:fenc=utf-8:tw=75:noet From b17408f49087d89f50c5f4c0574ee69798d835bc Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 09:57:10 -0400 Subject: [PATCH 10/14] make: fix some cleaned object filenames for fuzz-clean "make fuzz-clean" is trying to remove valgrind files that fuzz no longer generates, as well as log files that are no longer in the top-level directory. This removes the valgrind bits and fixes the directory. Signed-off-by: Peter Jones --- include/fuzz.mk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/fuzz.mk b/include/fuzz.mk index 44b0776ef..632039604 100644 --- a/include/fuzz.mk +++ b/include/fuzz.mk @@ -113,8 +113,7 @@ fuzz : $(fuzzers) $(MAKE) -f include/fuzz.mk fuzz-clean fuzz-clean : - @rm -vf random.bin libefi-test.a - @rm -vf vgcore.* fuzz*.log + @rm -vf random.bin libefi-test.a $(wildcard *-corpus/fuzz*.log) clean : fuzz-clean From f9da8c323bb97516949095b75b8c1b6036244118 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 09:58:48 -0400 Subject: [PATCH 11/14] make: clean fuzz and test stuff from the top-level makefile usually During some testing I noticed "make clean" when cross-building for Aarch64 included gcc complaining about "-mstrict-align", which is weird because aarch64-linux-gnu-gcc supports that just fine. Turns out it's because the test makefile is always using the host CC*, and that means it's calling 'gcc ... $(ARCH_CFLAGS) ... -print-file-name=include-fixed' to figure out compiler arguments that don't matter for the clean target. Since we don't actually have any support for using the cross-compiler to build test or fuzz targets, as we would have no way to run them, this will always happen and it'll always be dumb. This change makes it so Instead we just don't descend into those makefiles if we're just cleaning and not actually building/running the tests or fuzzers. [*] Actually was always wrongly using "gcc" and not "$(HOSTCC)" Signed-off-by: Peter Jones --- Makefile | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 3cfe70ace..5ff276b19 100644 --- a/Makefile +++ b/Makefile @@ -477,23 +477,30 @@ else $(PESIGN) -n certdb -i $< -c "shim" -s -o $@ -f endif -fuzz fuzz-clean fuzz-coverage fuzz-lto : +fuzz fuzz-coverage fuzz-lto : @make -f $(TOPDIR)/include/fuzz.mk \ COMPILER="$(COMPILER)" \ CROSS_COMPILE="$(CROSS_COMPILE)" \ CLANG_WARNINGS="$(CLANG_WARNINGS)" \ ARCH_DEFINES="$(ARCH_DEFINES)" \ EFI_INCLUDES="$(EFI_INCLUDES)" \ - fuzz-clean $@ + $@ -test test-clean test-coverage test-lto : generated_sbat_var_defs.h +test test-coverage test-lto : generated_sbat_var_defs.h @make -f $(TOPDIR)/include/test.mk \ COMPILER="$(COMPILER)" \ CROSS_COMPILE="$(CROSS_COMPILE)" \ CLANG_WARNINGS="$(CLANG_WARNINGS)" \ ARCH_DEFINES="$(ARCH_DEFINES)" \ EFI_INCLUDES="$(EFI_INCLUDES)" \ - test-clean $@ + $@ + +fuzz-clean: + @rm -vf random.bin libefi-test.a $(wildcard *-corpus/fuzz*.log) + +test-clean: + @rm -vf test-random.h libefi-test.a + @rm -vf *.gcda *.gcno *.gcov vgcore.* $(patsubst %.c,%,$(wildcard fuzz-*.c)) : @make -f $(TOPDIR)/include/fuzz.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" $@ @@ -502,10 +509,10 @@ $(patsubst %.c,%,$(wildcard test-*.c)) : @make -f $(TOPDIR)/include/test.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" $@ clean-fuzz-objs: - @make -f $(TOPDIR)/include/fuzz.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" clean + @find . -type f -a -perm /111 -a -iname 'fuzz-*' -print -delete clean-test-objs: - @make -f $(TOPDIR)/include/test.mk EFI_INCLUDES="$(EFI_INCLUDES)" ARCH_DEFINES="$(ARCH_DEFINES)" clean + @find . -type f -a -perm /111 -a -iname 'test-*' -print -delete .PHONY : $(patsubst %.c,%,$(wildcard fuzz-*.c)) fuzz .PHONY : $(patsubst %.c,%,$(wildcard test-*.c)) test From 39db2aaa35715d38891919545b380261d7a01e1e Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 13:11:06 -0400 Subject: [PATCH 12/14] make: make 'make fuzz' targets make more sense Currently there are three "make fuzz" targets: "fuzz", "fuzz-lto", and "fuzz-coverage". The last two were copy pasted from "make test" rules, and they make not sense here. Additionally, the "make fuzz" rule itself makes no sense as given, since it'll try to run all the fuzzers, one at a time, each with no time limit. This patch removes the two dumb rules and adds a 60-second limit to the main rule. Signed-off-by: Peter Jones --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5ff276b19..454290790 100644 --- a/Makefile +++ b/Makefile @@ -477,13 +477,14 @@ else $(PESIGN) -n certdb -i $< -c "shim" -s -o $@ -f endif -fuzz fuzz-coverage fuzz-lto : +fuzz: @make -f $(TOPDIR)/include/fuzz.mk \ COMPILER="$(COMPILER)" \ CROSS_COMPILE="$(CROSS_COMPILE)" \ CLANG_WARNINGS="$(CLANG_WARNINGS)" \ ARCH_DEFINES="$(ARCH_DEFINES)" \ EFI_INCLUDES="$(EFI_INCLUDES)" \ + MAX_FUZZ_TIME=60 \ $@ test test-coverage test-lto : generated_sbat_var_defs.h From 917883beaa0063e80ea7e33a8515eb560c1ff3e8 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 09:39:44 -0400 Subject: [PATCH 13/14] make: remove duplicate ARCH_CFLAGS definitions When I added the ARCH_blah definitions in Make.defaults, a couple of arches wound up with multiple ARCH_CFLAGS definitions. They're harmless, but they could wind up being confusing later. This patch removes them. Signed-off-by: Peter Jones --- Make.defaults | 2 -- 1 file changed, 2 deletions(-) diff --git a/Make.defaults b/Make.defaults index 21a76b129..ca9726f3c 100644 --- a/Make.defaults +++ b/Make.defaults @@ -78,7 +78,6 @@ ifeq ($(ARCH),ia32) ARCH_SUFFIX ?= ia32 ARCH_SUFFIX_UPPER ?= IA32 ARCH_LDFLAGS ?= - ARCH_CFLAGS ?= -m32 ARCH_SECTION_ALIGNMENT ?= 0x1000 endif ifeq ($(ARCH),aarch64) @@ -87,7 +86,6 @@ ifeq ($(ARCH),aarch64) ARCH_SUFFIX ?= aa64 ARCH_SUFFIX_UPPER ?= AA64 ARCH_LDFLAGS ?= - ARCH_CFLAGS ?= ARCH_SECTION_ALIGNMENT ?= 0x10000 endif ifeq ($(ARCH),arm) From 482ffa96a35fe4c97759607fb854b9cb60f52562 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 16 Jul 2026 09:41:40 -0400 Subject: [PATCH 14/14] Work around aarch64 build weirdness. For reasons beyond my comprehension, currently when building aarch64 debuginfo binaries, we get this: aarch64-linux-gnu-objcopy: shimaa64.efi.debug: not enough room for program headers, try linking with -N aarch64-linux-gnu-objcopy: shimaa64.efi.debug[.eh_frame]: bad value make: *** [../Makefile:458: shimaa64.efi.debug] Error 1 This sounds like it's about the space for the program headers before the first section, but adding more space changes nothing. What *does* change something is adding another section later on. This patch adds .data.ident to the objcopy section list, which should make absolutely no meaningful difference either in the link map or in using the debuginfo, but results in objcopy working just fine. With regret, Signed-off-by: Peter Jones --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 454290790..a05bfdc9d 100644 --- a/Makefile +++ b/Makefile @@ -455,7 +455,7 @@ endif ifneq ($(OBJCOPY_GTE224),1) $(error objcopy >= 2.24 is required) endif - $(OBJCOPY) -D -j .text -j .sdata -j .data \ + $(OBJCOPY) -D -j .text -j .sdata -j .data* \ -j .dynamic -j .rodata -j .rel* \ -j .rela* -j .dyn* -j .reloc -j .eh_frame -j .sbat \ -j .sbatlevel \