On 10/30/26 07:14, Ana Bobillo wrote:
From: Ana Bobillo <[email protected]>
efi_load_pe() copies the PE headers to the start of the allocated image.
The size it copies is not the size of the headers: it adds the size of a
whole 32bit NT header, which already holds an optional header, to
SizeOfOptionalHeader, and assumes that the NT header follows the DOS
header directly. Nor is it checked against the size of the allocation,
which only covers the sections. An image with many empty section
headers but little section data makes the copy write past the
allocation, and the copy reads past the end of a small image.
For secure boot we are looking at all regions. Shouldn't we make
Copy the headers up to the end of the section table, which is already
checked to lie within the image, and reject an image whose headers do
not fit into its virtual size before allocating it.
This runs before the image is rejected when Secure Boot fails to verify
it.
Fixes: 8458bf648fd7 ("efi_loader: set image_base and image_size to correct
values")
Signed-off-by: Ana Bobillo <[email protected]>
---
lib/efi_loader/efi_image_loader.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/lib/efi_loader/efi_image_loader.c
b/lib/efi_loader/efi_image_loader.c
index 7fd03ac6211..f9cd2cb274c 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -1010,7 +1010,6 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj
*handle,
struct efi_loaded_image *loaded_image_info)
{
IMAGE_NT_HEADERS32 *nt;
- IMAGE_DOS_HEADER *dos;
IMAGE_SECTION_HEADER *sections;
int num_sections;
void *efi_reloc;
@@ -1020,6 +1019,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj
*handle,
int rel_idx = IMAGE_DIRECTORY_ENTRY_BASERELOC;
uint64_t image_base;
unsigned long virt_size = 0;
+ size_t hdr_size;
int supported = 0;
efi_status_t ret;
@@ -1085,6 +1085,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle,
virt_size = max_t(unsigned long, virt_size, end);
}
+ /*
+ * The PE headers, up to the end of the section table, are copied to
+ * the start of the loaded image and must fit into it.
+ */
+ hdr_size = (void *)(sections + num_sections) - efi;
+ if (hdr_size > virt_size) {
+ log_err("PE headers do not fit into the image\n");
+ return EFI_LOAD_ERROR;
+ }
+
Shouldn't we ensure that we always allocate this amount of memory
instead of failing here?
Best regards
Heinrich
/* Read 32/64bit specific header bits */
if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
IMAGE_NT_HEADERS64 *nt64 = (void *)nt;
@@ -1141,11 +1151,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj
*handle,
#endif
/* Copy PE headers */
- memcpy(efi_reloc, efi,
- sizeof(*dos)
- + sizeof(*nt)
- + nt->FileHeader.SizeOfOptionalHeader
- + num_sections * sizeof(IMAGE_SECTION_HEADER));
+ memcpy(efi_reloc, efi, hdr_size);
/* Load sections into RAM */
for (i = num_sections - 1; i >= 0; i--) {
