From c01bf7943283c3a590e7cad0ff7fc2f74aa6afa1 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sat, 10 Sep 2022 08:24:41 +0200 Subject: [PATCH 1/2] boot: Be more explicit where to look for sections It only really makes sense to fetch the location from either one of these fields, depending on whether we are looking at an in-memory (relocated) or a file PE image. --- src/boot/efi/pe.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/boot/efi/pe.c b/src/boot/efi/pe.c index 8d7061d55b..2b260d4212 100644 --- a/src/boot/efi/pe.c +++ b/src/boot/efi/pe.c @@ -142,12 +142,13 @@ static void locate_sections( const PeSectionHeader section_table[], UINTN n_table, const char * const sections[], - UINTN *addrs, UINTN *offsets, - UINTN *sizes) { + UINTN *sizes, + bool in_memory) { assert(section_table); assert(sections); + assert(offsets); assert(sizes); for (UINTN i = 0; i < n_table; i++) { @@ -157,10 +158,7 @@ static void locate_sections( if (memcmp(sect->Name, sections[j], strlen8(sections[j])) != 0) continue; - if (addrs) - addrs[j] = sect->VirtualAddress; - if (offsets) - offsets[j] = sect->PointerToRawData; + offsets[j] = in_memory ? sect->VirtualAddress : sect->PointerToRawData; sizes[j] = sect->VirtualSize; } } @@ -178,8 +176,8 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const pe->FileHeader.NumberOfSections, sections, &addr, - NULL, - &size); + &size, + /*in_memory=*/true); if (size == 0) return 0; @@ -270,8 +268,8 @@ EFI_STATUS pe_memory_locate_sections(const void *base, const char * const sectio pe->FileHeader.NumberOfSections, sections, addrs, - NULL, - sizes); + sizes, + /*in_memory=*/true); return EFI_SUCCESS; } @@ -334,7 +332,7 @@ EFI_STATUS pe_file_locate_sections( return EFI_LOAD_ERROR; locate_sections(section_table, pe.FileHeader.NumberOfSections, - sections, NULL, offsets, sizes); + sections, offsets, sizes, /*in_memory=*/false); return EFI_SUCCESS; } From fa2eebc7a350adc1c61062818acdd1ea4b2aa3c9 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sat, 10 Sep 2022 09:02:35 +0200 Subject: [PATCH 2/2] boot: Try to detect overlapping PE sections This should help finding the cause of boot failures because of badly assembled unified kernel images. --- src/boot/efi/meson.build | 9 ++++++--- src/boot/efi/pe.c | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/boot/efi/meson.build b/src/boot/efi/meson.build index efe056c225..9eefe6571f 100644 --- a/src/boot/efi/meson.build +++ b/src/boot/efi/meson.build @@ -199,9 +199,12 @@ efi_cflags = [ ] ) -# On some distros, sd-boot/-stub may trigger some bug somewhere that will cause -# kernel execution to fail. The cause seems to be purely based on code size and -# always compiling with at least -O1 will work around that. +# Our code size has increased enough to possibly create overlapping PE sections +# at sd-stub runtime, which will often enough prevent the image from booting. +# This only happens because the usual instructions for assembling a unified +# kernel image contain hardcoded addresses for section VMAs added in. Until a +# proper solution is in place, we can at least compile with as least -O1 to +# reduce the likelyhood of this happening. # https://github.com/systemd/systemd/issues/24202 efi_cflags += '-O1' diff --git a/src/boot/efi/pe.c b/src/boot/efi/pe.c index 2b260d4212..852198f895 100644 --- a/src/boot/efi/pe.c +++ b/src/boot/efi/pe.c @@ -151,9 +151,17 @@ static void locate_sections( assert(offsets); assert(sizes); + size_t prev_section_addr = 0; + for (UINTN i = 0; i < n_table; i++) { const PeSectionHeader *sect = section_table + i; + if (in_memory) { + if (prev_section_addr > sect->VirtualAddress) + log_error_stall(u"Overlapping PE sections detected. Boot may fail due to image memory corruption!"); + prev_section_addr = sect->VirtualAddress + sect->VirtualSize; + } + for (UINTN j = 0; sections[j]; j++) { if (memcmp(sect->Name, sections[j], strlen8(sections[j])) != 0) continue;