From d06d8a232bf8ff4b27e4b5326dcec9717d89f3bc Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 17:15:38 +0100 Subject: [PATCH 01/10] boot: reject GPT headers with SizeOfPartitionEntry below the minimum Commit 0cf5f816f22c replaced the original lower-bound check if (h->SizeOfPartitionEntry < sizeof(EFI_PARTITION_ENTRY)) return false; with a multiple-of check if ((h->SizeOfPartitionEntry % sizeof(EFI_PARTITION_ENTRY)) != 0) return false; to additionally require the entry size to be a multiple of 128. The modulo test is however also satisfied by SizeOfPartitionEntry == 0, so a GPT header advertising a zero entry size now passes verify_gpt(). Restore the lower bound in addition to the multiple-of check, so the entry size must be at least sizeof(EFI_PARTITION_ENTRY) and a multiple of it (128 bytes). Follow-up for 0cf5f816f22c78740e122dfb6b3942ba4241717b --- src/boot/part-discovery.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/boot/part-discovery.c b/src/boot/part-discovery.c index ab553a6530b..10ba5241254 100644 --- a/src/boot/part-discovery.c +++ b/src/boot/part-discovery.c @@ -60,7 +60,8 @@ static bool verify_gpt(/* const */ GptHeader *h, EFI_LBA lba_expected) { if (h->MyLBA != lba_expected) return false; - if ((h->SizeOfPartitionEntry % sizeof(EFI_PARTITION_ENTRY)) != 0) + if (h->SizeOfPartitionEntry < sizeof(EFI_PARTITION_ENTRY) || + (h->SizeOfPartitionEntry % sizeof(EFI_PARTITION_ENTRY)) != 0) return false; if (h->NumberOfPartitionEntries <= 0 || h->NumberOfPartitionEntries > 1024) From 3bcd707d5650fdf91de19cf6e1ca5bbac7686967 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 17:21:52 +0100 Subject: [PATCH 02/10] boot: make device_path_next_node() robust against malformed zero-length nodes device_path_next_node() advances by the current node's Length field, which per the EFI device path protocol includes the 4-byte node header; a well-formed node is therefore at least sizeof(EFI_DEVICE_PATH) bytes long. A malformed node with Length < sizeof(EFI_DEVICE_PATH), in particular Length == 0, makes the helper return its input pointer unchanged. Advance by at least sizeof(EFI_DEVICE_PATH). Follow-up for 5080a60a719da213fa90964b76cc90bd0d1cb8de --- src/boot/device-path-util.c | 24 ++++++++++++++++++++++++ src/boot/device-path-util.h | 7 +++++++ 2 files changed, 31 insertions(+) diff --git a/src/boot/device-path-util.c b/src/boot/device-path-util.c index 5243e81e5df..73c3933299c 100644 --- a/src/boot/device-path-util.c +++ b/src/boot/device-path-util.c @@ -25,6 +25,10 @@ EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DE const EFI_DEVICE_PATH *end_node = device_path_find_end_node(dp); size_t file_size = strsize16(file); + /* The node Length is a uint16_t, so refuse a path that would not fit. */ + if (file_size > UINT16_MAX - sizeof(FILEPATH_DEVICE_PATH)) + return EFI_INVALID_PARAMETER; + size_t dp_size = (uint8_t *) end_node - (uint8_t *) dp; /* Make a copy that can also hold a file media device path. */ @@ -55,6 +59,9 @@ EFI_STATUS make_url_device_path(const char16_t *url, EFI_DEVICE_PATH **ret) { return EFI_INVALID_PARAMETER; size_t l = strlen8(u); + /* The node Length is a uint16_t, so refuse a URL that would not fit. */ + if (l > UINT16_MAX - offsetof(URI_DEVICE_PATH, Uri)) + return EFI_INVALID_PARAMETER; size_t t = offsetof(URI_DEVICE_PATH, Uri) + l + sizeof(EFI_DEVICE_PATH); EFI_DEVICE_PATH *dp = xmalloc(t); @@ -177,6 +184,23 @@ EFI_DEVICE_PATH *device_path_replace_node( return ret; } +bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size) { + if (!dp) + return false; + + /* Validate against the known size so a truncated/corrupt path can't make us read out of bounds. */ + for (;;) { + if (size < sizeof(EFI_DEVICE_PATH)) + return false; + if (dp->Length < sizeof(EFI_DEVICE_PATH) || dp->Length > size) + return false; + if (device_path_is_end(dp)) + return true; + size -= dp->Length; + dp = (const EFI_DEVICE_PATH *) ((const uint8_t *) dp + dp->Length); + } +} + size_t device_path_size(const EFI_DEVICE_PATH *dp) { const EFI_DEVICE_PATH *i = ASSERT_PTR(dp); diff --git a/src/boot/device-path-util.h b/src/boot/device-path-util.h index b02cf3146a9..8a993f27fe9 100644 --- a/src/boot/device-path-util.h +++ b/src/boot/device-path-util.h @@ -13,6 +13,9 @@ EFI_DEVICE_PATH *device_path_replace_node( static inline EFI_DEVICE_PATH *device_path_next_node(const EFI_DEVICE_PATH *dp) { assert(dp); + /* The node Length includes the 4-byte header, so a well-formed node is at least that long. Paths + * coming from untrusted sources must be checked with device_path_is_valid() before being walked. */ + assert(dp->Length >= sizeof(EFI_DEVICE_PATH)); return (EFI_DEVICE_PATH *) ((uint8_t *) dp + dp->Length); } @@ -30,4 +33,8 @@ static inline bool device_path_is_end(const EFI_DEVICE_PATH *dp) { size_t device_path_size(const EFI_DEVICE_PATH *dp); +/* Validates that a device path is well-formed and fully contained within the given size, terminated by an + * end node. Use on paths from untrusted sources (e.g. EFI variables) before walking them. */ +bool device_path_is_valid(const EFI_DEVICE_PATH *dp, size_t size); + EFI_DEVICE_PATH *device_path_dup(const EFI_DEVICE_PATH *dp); From 7b7599b72d84a948301107b41ae82256e8feb5bb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 17:29:31 +0100 Subject: [PATCH 03/10] boot: initialize return parameters on zero-length EFI variable read When a variable exists but is empty, the initial size-query GetVariable() in efivar_get_raw_full() returns EFI_SUCCESS instead of EFI_BUFFER_TOO_SMALL: the zero-length payload already "fits" the zero-length query buffer. The helper returns success, but does not initialize the return parameters. Handle a couple of corner cases by checking the return size. Follow-up for a40960748907212883f4b7de7367e6870657016e --- src/boot/efi-efivars.c | 13 +++++++++++++ src/boot/vmm.c | 16 +++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/boot/efi-efivars.c b/src/boot/efi-efivars.c index c1ae4252c3c..6af325966f9 100644 --- a/src/boot/efi-efivars.c +++ b/src/boot/efi-efivars.c @@ -191,6 +191,16 @@ EFI_STATUS efivar_get_raw_full( size_t size = 0; err = RT->GetVariable((char16_t *) name, (EFI_GUID *) vendor, NULL, &size, NULL); + if (err == EFI_SUCCESS) { + /* The variable exists but is empty, initialize return parameters */ + if (ret_attributes) + *ret_attributes = 0; + if (ret_data) + *ret_data = NULL; + if (ret_size) + *ret_size = 0; + return EFI_SUCCESS; + } if (err != EFI_BUFFER_TOO_SMALL) return err; @@ -222,6 +232,9 @@ EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, b if (err != EFI_SUCCESS) return err; + if (size == 0) + return EFI_BUFFER_TOO_SMALL; + if (ret) *ret = *b > 0; diff --git a/src/boot/vmm.c b/src/boot/vmm.c index e571a3990de..09c121b9884 100644 --- a/src/boot/vmm.c +++ b/src/boot/vmm.c @@ -62,7 +62,7 @@ bool is_direct_boot(EFI_HANDLE device) { EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { _cleanup_free_ EFI_HANDLE *handles = NULL; size_t n_handles; - EFI_STATUS err, dp_err; + EFI_STATUS err; assert(ret_vmm_dev); assert(ret_vmm_dir); @@ -79,9 +79,15 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { for (size_t order = 0;; order++) { _cleanup_free_ EFI_DEVICE_PATH *dp = NULL; + size_t dp_size = 0; _cleanup_free_ char16_t *order_str = xasprintf("VMMBootOrder%04zx", order); - dp_err = efivar_get_raw(MAKE_GUID_PTR(VMM_BOOT_ORDER), order_str, (void**) &dp, NULL); + err = efivar_get_raw(MAKE_GUID_PTR(VMM_BOOT_ORDER), order_str, (void**) &dp, &dp_size); + + /* Drop the device path from the (untrusted) EFI variable if it doesn't validate, so the + * check below simply has to test whether it is set. */ + if (err == EFI_SUCCESS && !device_path_is_valid(dp, dp_size)) + dp = mfree(dp); for (size_t i = 0; i < n_handles; i++) { _cleanup_file_close_ EFI_FILE *root_dir = NULL, *efi_dir = NULL; @@ -92,8 +98,8 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { if (err != EFI_SUCCESS) return err; - /* check against VMMBootOrderNNNN (if set) */ - if (dp_err == EFI_SUCCESS && !device_path_startswith(fs, dp)) + /* check against VMMBootOrderNNNN (if set and valid) */ + if (dp && !device_path_startswith(fs, dp)) continue; err = open_volume(handles[i], &root_dir); @@ -112,7 +118,7 @@ EFI_STATUS vmm_open(EFI_HANDLE *ret_vmm_dev, EFI_FILE **ret_vmm_dir) { return EFI_SUCCESS; } - if (dp_err != EFI_SUCCESS) + if (!dp) return EFI_NOT_FOUND; } assert_not_reached(); From 7378b51f66e8e7e87aa026b2ba2c7c785a0a509d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 19:41:31 +0100 Subject: [PATCH 04/10] boot: don't unquote an empty value in line_get_key_value() de0da85d41b switched the unquote check to strchr8(QUOTES, value[0]), which is not equivalent to the old explicit comparison for an empty value: strchr8(), like strchr(3), returns a pointer to the haystack's terminating NUL when the needle is '\0', so strchr8(QUOTES, '\0') is non-NULL. For a line whose separator is the last byte (e.g. "ID=") the split leaves value[0] == '\0' and line[linelen - 1] == '\0' too, so both conjuncts hold and value++ steps one byte past the value's terminator. Follow-up for de0da85d41b207b850aa0f68bb2436525389cf2b --- src/boot/efi-string.c | 2 +- src/boot/test-efi-string.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/boot/efi-string.c b/src/boot/efi-string.c index 30644638627..efe1553696d 100644 --- a/src/boot/efi-string.c +++ b/src/boot/efi-string.c @@ -500,7 +500,7 @@ char* line_get_key_value(char *s, const char *sep, size_t *pos, char **ret_key, value++; /* unquote */ - if (strchr8(QUOTES, value[0]) && line[linelen - 1] == value[0]) { + if (value[0] != '\0' && strchr8(QUOTES, value[0]) && line[linelen - 1] == value[0]) { value++; line[linelen - 1] = '\0'; } diff --git a/src/boot/test-efi-string.c b/src/boot/test-efi-string.c index 7633534dd4c..5805cb30241 100644 --- a/src/boot/test-efi-string.c +++ b/src/boot/test-efi-string.c @@ -590,6 +590,7 @@ TEST(line_get_key_value) { " also\tused \r\n" "for \"the conf\"\n" "format\t !!"; + char s3[] = "ID="; size_t pos = 0; char *key, *value; @@ -611,6 +612,11 @@ TEST(line_get_key_value) { ASSERT_TRUE(streq8(value, " stripping # with comments")); ASSERT_NULL(line_get_key_value(s1, "=", &pos, &key, &value)); + pos = 0; + ASSERT_NOT_NULL(line_get_key_value(s3, "=", &pos, &key, &value)); + ASSERT_TRUE(streq8(key, "ID")); + ASSERT_TRUE(streq8(value, "")); + pos = 0; ASSERT_NOT_NULL(line_get_key_value(s2, " \t", &pos, &key, &value)); ASSERT_TRUE(streq8(key, "this")); From ecf3f5056a74058fc60de91a7784ab01ce27dec8 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 19:57:23 +0100 Subject: [PATCH 05/10] boot: reject inner kernel entry point outside the image pe_kernel_info() returned AddressOfEntryPoint (and the .compat section entry_point) straight from the PE header with no check against SizeOfImage. Since cab9c7b5a4 the stub calls the inner kernel directly as ImageBase + entry_point, and only EFI_SIZE_TO_PAGES(SizeOfImage) pages are allocated for it. Follow-up for cab9c7b5a42effa8a45611fc6b8556138c869b5f --- src/boot/pe.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/boot/pe.c b/src/boot/pe.c index 872cb9e6220..685812a488a 100644 --- a/src/boot/pe.c +++ b/src/boot/pe.c @@ -520,6 +520,10 @@ EFI_STATUS pe_kernel_info( return EFI_UNSUPPORTED; if (pe->FileHeader.Machine == TARGET_MACHINE_TYPE) { + /* The entry point is later called as ImageBase + entry_point, and only SizeOfImage + * bytes are allocated for the image, so reject an entry point outside of it. */ + if (pe->OptionalHeader.AddressOfEntryPoint >= size_in_memory) + return EFI_LOAD_ERROR; if (ret_entry_point) *ret_entry_point = pe->OptionalHeader.AddressOfEntryPoint; if (ret_compat_entry_point) @@ -535,6 +539,9 @@ EFI_STATUS pe_kernel_info( if (compat_entry_point == 0) /* Image type not supported and no compat entry found. */ return EFI_UNSUPPORTED; + if (compat_entry_point >= size_in_memory) + /* Same as above: the compat entry point is called as ImageBase + entry_point. */ + return EFI_LOAD_ERROR; if (ret_entry_point) *ret_entry_point = 0; From feeba8fa3b77ae3e8f0c2bedda94e4dab23d85c7 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 20:12:36 +0100 Subject: [PATCH 06/10] boot: bound PE section VirtualSize before zeroing the inner kernel The inner-kernel section loader checks VirtualAddress + SizeOfRawData against kernel_size_in_memory (for the memcpy), but the memzero right after it clears up to VirtualAddress + VirtualSize, and VirtualSize is only constrained to be >= SizeOfRawData. Reject a VirtualAddress + VirtualSize that overflows or exceeds kernel_size_in_memory, mirroring the existing SizeOfRawData checks. Follow-up for cab9c7b5a42effa8a45611fc6b8556138c869b5f --- src/boot/linux.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/boot/linux.c b/src/boot/linux.c index bd71ada4835..d1da46ed4af 100644 --- a/src/boot/linux.c +++ b/src/boot/linux.c @@ -289,6 +289,10 @@ EFI_STATUS linux_exec( return log_error_status(EFI_LOAD_ERROR, "Section would write outside of memory"); if (h->SizeOfRawData > h->VirtualSize) return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, raw data size is greater than virtual size"); + if (UINT32_MAX - h->VirtualAddress < h->VirtualSize) + return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, VirtualSize + VirtualAddress overflows"); + if (h->VirtualAddress + h->VirtualSize > kernel_size_in_memory) + return log_error_status(EFI_LOAD_ERROR, "Section virtual size would write outside of memory"); if (UINT32_MAX - h->PointerToRawData < h->SizeOfRawData) return log_error_status(EFI_LOAD_ERROR, "Invalid PE section, PointerToRawData + SizeOfRawData overflows"); if (h->PointerToRawData + h->SizeOfRawData > kernel->iov_len) From dc55e4a5a00539f39befca4b0350c5e70b067702 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 21:58:57 +0100 Subject: [PATCH 07/10] boot: check PE section against SizeOfImage pe_locate_sections_internal() stores each matching section's VirtualSize and VirtualAddress into PeSectionVector.memory_size/memory_offset with only SIZE_MAX overflow guards, never checking them against the image's SizeOfImage. Wire up the image's SizeOfImage down to pe_locate_sections_internal() and skip any section whose in-memory section does not fit within it. Follow-up for fb974ac485c90f9887d5d21ac25d6d26d452eb3c --- src/boot/boot.c | 13 +++-- src/boot/linux.c | 5 +- src/boot/pe.c | 126 +++++++++++++++++++++++++++++++++++++++-------- src/boot/pe.h | 10 +++- src/boot/stub.c | 8 +-- 5 files changed, 129 insertions(+), 33 deletions(-) diff --git a/src/boot/boot.c b/src/boot/boot.c index dbe7f0c8ab6..c2f7e9b5008 100644 --- a/src/boot/boot.c +++ b/src/boot/boot.c @@ -2046,8 +2046,8 @@ static bool is_sd_boot(EFI_FILE *root_dir, const char16_t *loader_path) { return false; _cleanup_free_ PeSectionHeader *section_table = NULL; - size_t n_section_table; - err = pe_section_table_from_file(handle, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_file(handle, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return false; @@ -2058,6 +2058,7 @@ static bool is_sd_boot(EFI_FILE *root_dir, const char16_t *loader_path) { section_names, /* profile= */ UINT_MAX, /* validate_base= */ 0, + size_in_memory, vector); if (vector[0].memory_size != STRLEN(SD_MAGIC)) return false; @@ -2320,8 +2321,8 @@ static void boot_entry_add_type2( /* Load section table once */ _cleanup_free_ PeSectionHeader *section_table = NULL; - size_t n_section_table; - err = pe_section_table_from_file(handle, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_file(handle, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return; @@ -2333,6 +2334,7 @@ static void boot_entry_add_type2( section_names, /* profile= */ UINT_MAX, /* validate_base= */ 0, + size_in_memory, base_sections); /* and now iterate through possible profiles, and create a menu item for each profile we find */ @@ -2348,6 +2350,7 @@ static void boot_entry_add_type2( section_names, profile, /* validate_base= */ 0, + size_in_memory, sections); if (err != EFI_SUCCESS && profile > 0) /* It's fine if there's no .profile for the first profile */ @@ -3100,7 +3103,7 @@ static EFI_STATUS call_image_start( if (err == EFI_UNSUPPORTED && entry->type == LOADER_LINUX) { uint32_t compat_address; - err = pe_kernel_info(loaded_image->ImageBase, /* ret_entry_point= */ NULL, &compat_address, + err = pe_kernel_info(loaded_image->ImageBase, loaded_image->ImageSize, /* ret_entry_point= */ NULL, &compat_address, /* ret_size_in_memory= */ NULL, /* ret_section_alignment= */ NULL); if (err != EFI_SUCCESS) { diff --git a/src/boot/linux.c b/src/boot/linux.c index d1da46ed4af..584fc051b71 100644 --- a/src/boot/linux.c +++ b/src/boot/linux.c @@ -167,7 +167,7 @@ EFI_STATUS linux_exec( assert(iovec_is_set(kernel)); assert(iovec_is_valid(initrd)); - err = pe_kernel_info(kernel->iov_base, &entry_point, &compat_entry_point, &kernel_size_in_memory, §ion_alignment); + err = pe_kernel_info(kernel->iov_base, kernel->iov_len, &entry_point, &compat_entry_point, &kernel_size_in_memory, §ion_alignment); #if defined(__i386__) || defined(__x86_64__) if (err == EFI_UNSUPPORTED) /* Kernel is too old to support LINUX_INITRD_MEDIA_GUID, try the deprecated EFI handover @@ -259,8 +259,7 @@ EFI_STATUS linux_exec( const PeSectionHeader *headers; size_t n_headers; - /* Do we need to validate anything here? the len? */ - err = pe_section_table_from_base(kernel->iov_base, &headers, &n_headers); + err = pe_section_table_from_base(kernel->iov_base, kernel->iov_len, &headers, &n_headers, /* ret_size_in_memory= */ NULL); if (err != EFI_SUCCESS) return log_error_status(err, "Cannot read sections: %m"); diff --git a/src/boot/pe.c b/src/boot/pe.c index 685812a488a..5e4c07e1610 100644 --- a/src/boot/pe.c +++ b/src/boot/pe.c @@ -173,6 +173,54 @@ static size_t section_table_offset(const DosFileHeader *dos, const PeFileHeader return dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader) + pe->FileHeader.SizeOfOptionalHeader; } +static EFI_STATUS pe_headers_from_base( + const void *base, + size_t base_len, + bool allow_compatibility, + const DosFileHeader **ret_dos, + const PeFileHeader **ret_pe) { + + assert(base); + assert(ret_dos); + assert(ret_pe); + + /* Validate the DOS and PE headers against the buffer length */ + + if (base_len < sizeof(DosFileHeader)) + return EFI_LOAD_ERROR; + + const DosFileHeader *dos = (const DosFileHeader*) base; + if (!verify_dos(dos)) + return EFI_LOAD_ERROR; + + if (dos->ExeHeader > base_len || base_len - dos->ExeHeader < sizeof(PeFileHeader)) + return EFI_LOAD_ERROR; + + const PeFileHeader *pe = (const PeFileHeader*) ((const uint8_t*) base + dos->ExeHeader); + if (!verify_pe(dos, pe, allow_compatibility)) + return EFI_LOAD_ERROR; + + *ret_dos = dos; + *ret_pe = pe; + return EFI_SUCCESS; +} + +static bool pe_section_table_in_bounds( + const DosFileHeader *dos, + const PeFileHeader *pe, + size_t base_len) { + + assert(dos); + assert(pe); + + /* verify_pe() already bounded SizeOfOptionalHeader so this offset cannot overflow, and + * NumberOfSections is a uint16_t so the byte count cannot either. */ + size_t offset = section_table_offset(dos, pe); + size_t bytes = (size_t) pe->FileHeader.NumberOfSections * sizeof(PeSectionHeader); + + return offset <= base_len && base_len - offset >= bytes; +} + static bool pe_section_name_equal(const char *a, const char *b) { if (a == b) @@ -258,6 +306,7 @@ static void pe_locate_sections_internal( size_t n_section_table, const char *const section_names[], size_t validate_base, + size_t size_in_memory, const void *device_table, const Device *device, PeSectionVector sections[]) { @@ -288,6 +337,11 @@ static void pe_locate_sections_internal( if ((size_t) j->VirtualSize > size_max) continue; + /* The section's in-memory range must lie within the image, otherwise consumers + * reading it via memory_offset/memory_size would read past the loaded image. */ + if ((size_t) j->VirtualAddress + (size_t) j->VirtualSize > size_in_memory) + continue; + /* 2nd overflow check: ignore sections that are impossibly large also taking the * loaded base into account. */ if (validate_base != 0) { @@ -360,6 +414,7 @@ static void pe_locate_sections( size_t n_section_table, const char *const section_names[], size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]) { if (!looking_for_dtbauto_or_efifw(section_names)) @@ -368,6 +423,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, /* device_table= */ NULL, /* device= */ NULL, sections); @@ -387,6 +443,7 @@ static void pe_locate_sections( n_section_table, hwid_section_names, validate_base, + size_in_memory, /* device_table= */ NULL, /* device= */ NULL, hwids_section); @@ -403,6 +460,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); @@ -418,6 +476,7 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); @@ -433,12 +492,13 @@ static void pe_locate_sections( n_section_table, section_names, validate_base, + size_in_memory, hwids, device, sections); } -static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const PeFileHeader *pe) { +static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, size_t base_len, const PeFileHeader *pe) { /* The kernel may provide alternative PE entry points for different PE architectures. This allows * booting a 64-bit kernel on 32-bit EFI that is otherwise running on a 64-bit CPU. The locations of any * such compat entry points are located in a special PE section. */ @@ -448,16 +508,29 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const static const char *const section_names[] = { ".compat", NULL }; PeSectionVector vector[1] = {}; + + /* Make sure the section table lies within the buffer before pe_locate_sections() iterates it. */ + if (!pe_section_table_in_bounds(dos, pe, base_len)) + return 0; + pe_locate_sections( (const PeSectionHeader *) ((const uint8_t *) dos + section_table_offset(dos, pe)), pe->FileHeader.NumberOfSections, section_names, PTR_TO_SIZE(dos), + pe->OptionalHeader.SizeOfImage, vector); if (!PE_SECTION_VECTOR_IS_SET(vector)) /* not found */ return 0; + /* pe_locate_sections() bounded the section against SizeOfImage, the in-memory size. Here we read the + * section data straight from the file buffer 'dos', which may be smaller than SizeOfImage, so also + * require the section's data range to lie within base_len before scanning it. */ + if (vector[0].memory_offset > base_len || + vector[0].memory_size > base_len - vector[0].memory_offset) + return 0; + typedef struct { uint8_t type; uint8_t size; @@ -487,19 +560,18 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const EFI_STATUS pe_kernel_info( const void *base, + size_t base_len, uint32_t *ret_entry_point, uint32_t *ret_compat_entry_point, size_t *ret_size_in_memory, uint32_t *ret_section_alignment) { assert(base); - const DosFileHeader *dos = (const DosFileHeader *) base; - if (!verify_dos(dos)) - return EFI_LOAD_ERROR; - - const PeFileHeader *pe = (const PeFileHeader *) ((const uint8_t *) base + dos->ExeHeader); - if (!verify_pe(dos, pe, /* allow_compatibility= */ true)) - return EFI_LOAD_ERROR; + const DosFileHeader *dos; + const PeFileHeader *pe; + EFI_STATUS err = pe_headers_from_base(base, base_len, /* allow_compatibility= */ true, &dos, &pe); + if (err != EFI_SUCCESS) + return err; /* When allocating we need to also consider the virtual/uninitialized data sections, so parse it out * of the SizeOfImage field in the PE header and return it */ @@ -535,7 +607,7 @@ EFI_STATUS pe_kernel_info( return EFI_SUCCESS; } - uint32_t compat_entry_point = get_compatibility_entry_address(dos, pe); + uint32_t compat_entry_point = get_compatibility_entry_address(dos, base_len, pe); if (compat_entry_point == 0) /* Image type not supported and no compat entry found. */ return EFI_UNSUPPORTED; @@ -606,20 +678,20 @@ bool pe_kernel_check_nx_compat(const void *base) { EFI_STATUS pe_section_table_from_base( const void *base, + size_t base_len, const PeSectionHeader **ret_section_table, - size_t *ret_n_section_table) { + size_t *ret_n_section_table, + size_t *ret_size_in_memory) { assert(base); assert(ret_section_table); assert(ret_n_section_table); - const DosFileHeader *dos = (const DosFileHeader*) base; - if (!verify_dos(dos)) - return EFI_LOAD_ERROR; - - const PeFileHeader *pe = (const PeFileHeader*) ((const uint8_t*) base + dos->ExeHeader); - if (!verify_pe(dos, pe, /* allow_compatibility= */ false)) - return EFI_LOAD_ERROR; + const DosFileHeader *dos; + const PeFileHeader *pe; + EFI_STATUS err = pe_headers_from_base(base, base_len, /* allow_compatibility= */ false, &dos, &pe); + if (err != EFI_SUCCESS) + return err; assert_cc(sizeof(pe->FileHeader.NumberOfSections) == sizeof(uint16_t)); /* multiplication below cannot overflow */ @@ -627,14 +699,22 @@ EFI_STATUS pe_section_table_from_base( if (n_section_table * sizeof(PeSectionHeader) > SECTION_TABLE_BYTES_MAX) return EFI_OUT_OF_RESOURCES; + /* Make sure the section table lies within the buffer, so consumers iterating it don't read off the + * end. */ + if (!pe_section_table_in_bounds(dos, pe, base_len)) + return EFI_LOAD_ERROR; + *ret_section_table = (const PeSectionHeader*) ((const uint8_t*) base + section_table_offset(dos, pe)); *ret_n_section_table = n_section_table; + if (ret_size_in_memory) + *ret_size_in_memory = pe->OptionalHeader.SizeOfImage; return EFI_SUCCESS; } EFI_STATUS pe_memory_locate_sections( const void *base, + size_t base_len, const char *const section_names[], PeSectionVector sections[]) { @@ -645,8 +725,8 @@ EFI_STATUS pe_memory_locate_sections( assert(sections); const PeSectionHeader *section_table; - size_t n_section_table; - err = pe_section_table_from_base(base, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_base(base, base_len, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return err; @@ -655,6 +735,7 @@ EFI_STATUS pe_memory_locate_sections( n_section_table, section_names, PTR_TO_SIZE(base), + size_in_memory, sections); return EFI_SUCCESS; @@ -663,7 +744,8 @@ EFI_STATUS pe_memory_locate_sections( EFI_STATUS pe_section_table_from_file( EFI_FILE *handle, PeSectionHeader **ret_section_table, - size_t *ret_n_section_table) { + size_t *ret_n_section_table, + size_t *ret_size_in_memory) { EFI_STATUS err; size_t len; @@ -717,6 +799,8 @@ EFI_STATUS pe_section_table_from_file( *ret_section_table = TAKE_PTR(section_table); *ret_n_section_table = n_section_table; + if (ret_size_in_memory) + *ret_size_in_memory = pe.OptionalHeader.SizeOfImage; return EFI_SUCCESS; } @@ -784,6 +868,7 @@ EFI_STATUS pe_locate_profile_sections( const char* const section_names[], unsigned profile, size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]) { assert(section_table || n_section_table == 0); @@ -805,6 +890,7 @@ EFI_STATUS pe_locate_profile_sections( n, section_names, validate_base, + size_in_memory, sections); return EFI_SUCCESS; diff --git a/src/boot/pe.h b/src/boot/pe.h index 5c8dc86fe93..bc9963799f5 100644 --- a/src/boot/pe.h +++ b/src/boot/pe.h @@ -36,13 +36,16 @@ static inline bool PE_SECTION_VECTOR_IS_SET(const PeSectionVector *v) { EFI_STATUS pe_section_table_from_base( const void *base, + size_t base_len, const PeSectionHeader **ret_section_table, - size_t *ret_n_section_table); + size_t *ret_n_section_table, + size_t *ret_size_in_memory); EFI_STATUS pe_section_table_from_file( EFI_FILE *handle, PeSectionHeader **ret_section_table, - size_t *ret_n_section_table); + size_t *ret_n_section_table, + size_t *ret_size_in_memory); EFI_STATUS pe_locate_profile_sections( const PeSectionHeader section_table[], @@ -50,15 +53,18 @@ EFI_STATUS pe_locate_profile_sections( const char* const section_names[], unsigned profile, size_t validate_base, + size_t size_in_memory, PeSectionVector sections[]); EFI_STATUS pe_memory_locate_sections( const void *base, + size_t base_len, const char *const section_names[], PeSectionVector sections[]); EFI_STATUS pe_kernel_info( const void *base, + size_t base_len, uint32_t *ret_entry_point, uint32_t *ret_compat_entry_point, size_t *ret_size_in_memory, diff --git a/src/boot/stub.c b/src/boot/stub.c index e69faca00b0..94c4ac49e4b 100644 --- a/src/boot/stub.c +++ b/src/boot/stub.c @@ -596,7 +596,7 @@ static EFI_STATUS load_addons( if (err != EFI_SUCCESS) return log_error_status(err, "Failed to find protocol in %ls: %m", items[i]); - err = pe_memory_locate_sections(loaded_addon->ImageBase, unified_sections, sections); + err = pe_memory_locate_sections(loaded_addon->ImageBase, loaded_addon->ImageSize, unified_sections, sections); if (err != EFI_SUCCESS) { log_error_status(err, "Unable to locate embedded .cmdline/.dtb/.dtbauto/.efifw/.initrd/.ucode sections in %ls, ignoring: %m", @@ -1099,8 +1099,8 @@ static EFI_STATUS find_sections( assert(sections); const PeSectionHeader *section_table; - size_t n_section_table; - err = pe_section_table_from_base(loaded_image->ImageBase, §ion_table, &n_section_table); + size_t n_section_table, size_in_memory; + err = pe_section_table_from_base(loaded_image->ImageBase, loaded_image->ImageSize, §ion_table, &n_section_table, &size_in_memory); if (err != EFI_SUCCESS) return log_error_status(err, "Unable to locate PE section table: %m"); @@ -1111,6 +1111,7 @@ static EFI_STATUS find_sections( unified_sections, /* profile= */ UINT_MAX, /* validate_base= */ PTR_TO_SIZE(loaded_image->ImageBase), + size_in_memory, sections); if (err != EFI_SUCCESS) return log_error_status(err, "Unable to locate embedded base PE sections: %m"); @@ -1123,6 +1124,7 @@ static EFI_STATUS find_sections( unified_sections, profile, /* validate_base= */ PTR_TO_SIZE(loaded_image->ImageBase), + size_in_memory, sections); if (err != EFI_SUCCESS && !(err == EFI_NOT_FOUND && profile == 0)) /* the first profile is implied if it doesn't exist */ return log_error_status(err, "Unable to locate embedded per-profile PE sections: %m"); From ba0c1c617e7154bb18e3f0911e07489db2000d45 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 22:10:20 +0100 Subject: [PATCH 08/10] boot: restore RW/RO memory attributes on every error linux_exec() marks code sections RO+X for W^X and reverts them to RW+NX in a loop just before returning, because EDK2 requires freed buffers to be writable and non-executable or FreePages() crashes. Not every error path is currently covered. Switch to a _cleanup_ helper so that every return path is covered. Follow-up for 56d19b633d049035afe3f690fd6c717e06f88597 --- src/boot/linux.c | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/src/boot/linux.c b/src/boot/linux.c index 584fc051b71..56b932ecb56 100644 --- a/src/boot/linux.c +++ b/src/boot/linux.c @@ -153,6 +153,25 @@ static EFI_STATUS memory_mark_rw_nx(EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto, return EFI_SUCCESS; } +typedef struct CleanupNxSections { + EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto; + struct iovec *sections; + size_t n_sections; +} CleanupNxSections; + +static void cleanup_nx_sections(CleanupNxSections *c) { + assert(c); + + /* Restore the code sections that were marked RO+X back to RW+NX before their backing pages are + * freed: EDK2 requires freed buffers to be writable and non-executable (it may overwrite them with + * a fixed pattern), otherwise FreePages() crashes. */ + if (c->memory_proto) + for (size_t i = 0; i < c->n_sections; i++) + (void) memory_mark_rw_nx(c->memory_proto, &c->sections[i]); + + free(c->sections); +} + EFI_STATUS linux_exec( EFI_HANDLE parent_image, const char16_t *cmdline, @@ -242,8 +261,6 @@ EFI_STATUS linux_exec( * https://microsoft.github.io/mu/WhatAndWhy/enhancedmemoryprotection/ * https://www.kraxel.org/blog/2023/12/uefi-nx-linux-boot/ */ EFI_MEMORY_ATTRIBUTE_PROTOCOL *memory_proto = NULL; - _cleanup_free_ struct iovec *nx_sections = NULL; - size_t n_nx_sections = 0; if (pe_kernel_check_nx_compat(kernel->iov_base)) { /* LocateProtocol() is not quite that quick if you have many protocols, so only look for it @@ -276,6 +293,12 @@ EFI_STATUS linux_exec( /* addr= */ 0); uint8_t* loaded_kernel = PHYSICAL_ADDRESS_TO_POINTER(loaded_kernel_pages.addr); + + /* Any code section marked RO+X must be reverted to RW+NX before the backing pages are freed. */ + _cleanup_(cleanup_nx_sections) CleanupNxSections nx_restore = { + .memory_proto = memory_proto, + }; + FOREACH_ARRAY(h, headers, n_headers) { if (h->PointerToRelocations != 0) return log_error_status(EFI_LOAD_ERROR, "Inner kernel image contains sections with relocations, which we do not support."); @@ -304,15 +327,14 @@ EFI_STATUS linux_exec( /* Not a code section? Nothing to do, leave as-is. */ if (memory_proto && (h->Characteristics & (PE_CODE|PE_EXECUTE))) { - nx_sections = xrealloc(nx_sections, n_nx_sections * sizeof(struct iovec), (n_nx_sections + 1) * sizeof(struct iovec)); - nx_sections[n_nx_sections].iov_base = loaded_kernel + h->VirtualAddress; - nx_sections[n_nx_sections].iov_len = h->VirtualSize; + /* Record the section for cleanup before marking it RO+X: if memory_mark_ro_x() + * fails after partially applying the attributes, cleanup still reverts them. */ + nx_restore.sections = xrealloc(nx_restore.sections, nx_restore.n_sections * sizeof(struct iovec), (nx_restore.n_sections + 1) * sizeof(struct iovec)); + nx_restore.sections[nx_restore.n_sections++] = IOVEC_MAKE(loaded_kernel + h->VirtualAddress, h->VirtualSize); - err = memory_mark_ro_x(memory_proto, &nx_sections[n_nx_sections]); + err = memory_mark_ro_x(memory_proto, &nx_restore.sections[nx_restore.n_sections - 1]); if (err != EFI_SUCCESS) return err; - - ++n_nx_sections; } } @@ -352,11 +374,5 @@ EFI_STATUS linux_exec( /* Restore */ *parent_loaded_image = original_parent_loaded_image; - /* On failure we'll free the buffers. EDK2 requires the memory buffers to be writable and - * non-executable, as in some configurations it will overwrite them with a fixed pattern, so if the - * attributes are not restored FreePages() will crash. */ - for (size_t i = 0; i < n_nx_sections; i++) - (void) memory_mark_rw_nx(memory_proto, &nx_sections[i]); - return log_error_status(err, "Error starting kernel image: %m"); } From baad1744bd0bd4302ee438c7f1ba60217e758199 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 22:17:28 +0100 Subject: [PATCH 09/10] boot: restore parent loaded image when initrd registration fails linux_exec() patches the stub's own EFI_LOADED_IMAGE_PROTOCOL to point at the loaded inner kernel, and restores the saved original only after the entry point returns. The initrd_register() failure path returns without restoring, leaving the firmware's protocol pointing to freed data. Follow-up for f4051650657cd337ceba67b773f0e3bf854cbaff --- src/boot/linux.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/boot/linux.c b/src/boot/linux.c index 56b932ecb56..67ed8599931 100644 --- a/src/boot/linux.c +++ b/src/boot/linux.c @@ -355,8 +355,12 @@ EFI_STATUS linux_exec( _cleanup_(cleanup_initrd) EFI_HANDLE initrd_handle = NULL; err = initrd_register(initrd, &initrd_handle); - if (err != EFI_SUCCESS) + if (err != EFI_SUCCESS) { + /* Restore the patched fields before kernel_file_path and loaded_kernel_pages are freed, + * otherwise the stub's own EFI_LOADED_IMAGE_PROTOCOL is left pointing at freed memory. */ + *parent_loaded_image = original_parent_loaded_image; return log_error_status(err, "Error registering initrd: %m"); + } log_wait(); From 918e8f9dd91e5f51cd06b599ef3cbe534aaff28d Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Jun 2026 23:12:54 +0100 Subject: [PATCH 10/10] boot: require a minimum PE optional header size in verify_pe() verify_pe() only checked SizeOfOptionalHeader against a SIZE_MAX wrap (a clause that, given SizeOfOptionalHeader is a uint16_t, can never reject anything) and never read NumberOfRvaAndSizes. But pe_kernel_info(), pe_kernel_check_nx_compat() and pe_kernel_check_no_relocation() then read SizeOfImage, AddressOfEntryPoint, DllCharacteristics and the base relocation data directory entry from the optional header. Require SizeOfOptionalHeader to be large enough to contain everything down to the base relocation data directory entry, and require the image to declare that many data directory entries. Follow-up for bacc2ed0d5bb10de5d37a1df73c061247f005b59 --- src/boot/pe.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/boot/pe.c b/src/boot/pe.c index 5e4c07e1610..b7974b33a50 100644 --- a/src/boot/pe.c +++ b/src/boot/pe.c @@ -140,6 +140,9 @@ typedef struct PeFileHeader { #define SECTION_TABLE_BYTES_MAX (16U * 1024U * 1024U) +/* https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-data-directories-image-only */ +#define BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY 5 + static bool verify_dos(const DosFileHeader *dos) { assert(dos); @@ -163,7 +166,18 @@ static bool verify_pe( (allow_compatibility && pe->FileHeader.Machine == TARGET_MACHINE_TYPE_COMPATIBILITY)) && pe->FileHeader.NumberOfSections > 0 && IN_SET(pe->OptionalHeader.Magic, OPTHDR32_MAGIC, OPTHDR64_MAGIC) && - pe->FileHeader.SizeOfOptionalHeader < SIZE_MAX - (dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader)); + pe->FileHeader.SizeOfOptionalHeader < SIZE_MAX - (dos->ExeHeader + offsetof(PeFileHeader, OptionalHeader)) && + /* The optional header must be large enough to actually contain every field we read from + * it later (the deepest being the base relocation data directory entry), and must declare + * at least that many data directory entries. */ + pe->FileHeader.SizeOfOptionalHeader >= + (pe->OptionalHeader.Magic == OPTHDR32_MAGIC ? + offsetof(PeOptionalHeader, DataDirectory32) : + offsetof(PeOptionalHeader, DataDirectory64)) + + (BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY + 1) * sizeof(PeImageDataDirectory) && + (pe->OptionalHeader.Magic == OPTHDR32_MAGIC ? + pe->OptionalHeader.NumberOfRvaAndSizes32 : + pe->OptionalHeader.NumberOfRvaAndSizes64) > BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY; } static size_t section_table_offset(const DosFileHeader *dos, const PeFileHeader *pe) { @@ -627,9 +641,6 @@ EFI_STATUS pe_kernel_info( return EFI_SUCCESS; } -/* https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#optional-header-data-directories-image-only */ -#define BASE_RELOCATION_TABLE_DATA_DIRECTORY_ENTRY 5 - /* We do not expect PE inner kernels to have any relocations. However that might be wrong for some * architectures, or it might change in the future. If the case of relocation arise, we should transform this * function in a function applying the relocations. However for now, since it would not be exercised and