From: Matthew Garrett Date: Fri, 30 Jun 2017 18:27:47 +0000 (-0700) Subject: sd-boot: stub: Obtain PE section offsets from RAM, not disk (#6250) X-Git-Tag: v234~46 X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fsystemd.git;a=commitdiff_plain;h=d4cbada2a95667c4d5d4310298bfcb446b1357b5 sd-boot: stub: Obtain PE section offsets from RAM, not disk (#6250) In a Secure Boot scenario the stub loader will have been validated before execution. A malicious drive could then change the data returned in future reads, resulting in the loader obtaining incorrect section offsets and (for instance) allowing the command line to be modified. Pull that information out of the in-RAM representation of the loader instead in order to avoid this. Fixes: #6230 (Lennart did some minor coding style fixes, and renamed pefile.c → pe.c, as suggested by Kay, given that the file now contains a function whose name doesn't match the filename as prefix anymore.) --- diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index d429a125226..1e990b38251 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -20,10 +20,10 @@ #include "disk.h" #include "graphics.h" #include "linux.h" -#include "pefile.h" -#include "util.h" #include "measure.h" +#include "pe.h" #include "shim.h" +#include "util.h" #ifndef EFI_OS_INDICATIONS_BOOT_TO_FW_UI #define EFI_OS_INDICATIONS_BOOT_TO_FW_UI 0x0000000000000001ULL @@ -1543,7 +1543,7 @@ static VOID config_entry_add_linux( Config *config, EFI_LOADED_IMAGE *loaded_ima continue; /* look for .osrel and .cmdline sections in the .efi binary */ - err = pefile_locate_sections(linux_dir, f->FileName, sections, addrs, offs, szs); + err = pe_file_locate_sections(linux_dir, f->FileName, sections, addrs, offs, szs); if (EFI_ERROR(err)) continue; diff --git a/src/boot/efi/meson.build b/src/boot/efi/meson.build index 9e06e22ff47..5ef5b2d20b5 100644 --- a/src/boot/efi/meson.build +++ b/src/boot/efi/meson.build @@ -4,7 +4,7 @@ efi_headers = files(''' graphics.h linux.h measure.h - pefile.h + pe.h splash.h util.h shim.h @@ -14,7 +14,7 @@ common_sources = ''' disk.c graphics.c measure.c - pefile.c + pe.c util.c '''.split() diff --git a/src/boot/efi/pefile.c b/src/boot/efi/pe.c similarity index 60% rename from src/boot/efi/pefile.c rename to src/boot/efi/pe.c index 77fff77b69f..054e8edbc63 100644 --- a/src/boot/efi/pefile.c +++ b/src/boot/efi/pe.c @@ -15,7 +15,7 @@ #include #include -#include "pefile.h" +#include "pe.h" #include "util.h" struct DosFileHeader { @@ -52,6 +52,11 @@ struct PeFileHeader { UINT16 Characteristics; } __attribute__((packed)); +struct PeHeader { + UINT8 Magic[4]; + struct PeFileHeader FileHeader; +} __attribute__((packed)); + struct PeSectionHeader { UINT8 Name[8]; UINT32 VirtualSize; @@ -65,15 +70,61 @@ struct PeSectionHeader { UINT32 Characteristics; } __attribute__((packed)); +EFI_STATUS pe_memory_locate_sections(CHAR8 *base, CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes) { + struct DosFileHeader *dos; + struct PeHeader *pe; + UINTN i; + UINTN offset; + + dos = (struct DosFileHeader *)base; + + if (CompareMem(dos->Magic, "MZ", 2) != 0) + return EFI_LOAD_ERROR; + + pe = (struct PeHeader *)&base[dos->ExeHeader]; + if (CompareMem(pe->Magic, "PE\0\0", 4) != 0) + return EFI_LOAD_ERROR; + + /* PE32+ Subsystem type */ + if (pe->FileHeader.Machine != PE_HEADER_MACHINE_X64 && + pe->FileHeader.Machine != PE_HEADER_MACHINE_I386) + return EFI_LOAD_ERROR; + + if (pe->FileHeader.NumberOfSections > 96) + return EFI_LOAD_ERROR; + + offset = dos->ExeHeader + sizeof(*pe) + pe->FileHeader.SizeOfOptionalHeader; + + for (i = 0; i < pe->FileHeader.NumberOfSections; i++) { + struct PeSectionHeader *sect; + UINTN j; + + sect = (struct PeSectionHeader *)&base[offset]; + for (j = 0; sections[j]; j++) { + if (CompareMem(sect->Name, sections[j], strlena(sections[j])) != 0) + continue; -EFI_STATUS pefile_locate_sections(EFI_FILE *dir, CHAR16 *path, CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes) { + if (addrs) + addrs[j] = (UINTN)sect->VirtualAddress; + if (offsets) + offsets[j] = (UINTN)sect->PointerToRawData; + if (sizes) + sizes[j] = (UINTN)sect->VirtualSize; + } + offset += sizeof(*sect); + } + + return EFI_SUCCESS; +} + +EFI_STATUS pe_file_locate_sections(EFI_FILE *dir, CHAR16 *path, CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes) { EFI_FILE_HANDLE handle; struct DosFileHeader dos; - uint8_t magic[4]; - struct PeFileHeader pe; + struct PeHeader pe; UINTN len; - UINTN i; + UINTN headerlen; EFI_STATUS err; + CHAR8 *header = NULL; err = uefi_call_wrapper(dir->Open, 5, dir, &handle, path, EFI_FILE_MODE_READ, 0ULL); if (EFI_ERROR(err)) @@ -89,30 +140,10 @@ EFI_STATUS pefile_locate_sections(EFI_FILE *dir, CHAR16 *path, CHAR8 **sections, goto out; } - if (CompareMem(dos.Magic, "MZ", 2) != 0) { - err = EFI_LOAD_ERROR; - goto out; - } - err = uefi_call_wrapper(handle->SetPosition, 2, handle, dos.ExeHeader); if (EFI_ERROR(err)) goto out; - /* PE header */ - len = sizeof(magic); - err = uefi_call_wrapper(handle->Read, 3, handle, &len, &magic); - if (EFI_ERROR(err)) - goto out; - if (len != sizeof(magic)) { - err = EFI_LOAD_ERROR; - goto out; - } - - if (CompareMem(magic, "PE\0\0", 2) != 0) { - err = EFI_LOAD_ERROR; - goto out; - } - len = sizeof(pe); err = uefi_call_wrapper(handle->Read, 3, handle, &len, &pe); if (EFI_ERROR(err)) @@ -122,49 +153,30 @@ EFI_STATUS pefile_locate_sections(EFI_FILE *dir, CHAR16 *path, CHAR8 **sections, goto out; } - /* PE32+ Subsystem type */ - if (pe.Machine != PE_HEADER_MACHINE_X64 && - pe.Machine != PE_HEADER_MACHINE_I386) { - err = EFI_LOAD_ERROR; + headerlen = sizeof(dos) + sizeof(pe) + pe.FileHeader.SizeOfOptionalHeader + pe.FileHeader.NumberOfSections * sizeof(struct PeSectionHeader); + header = AllocatePool(headerlen); + if (!header) { + err = EFI_OUT_OF_RESOURCES; goto out; } + len = headerlen; + err = uefi_call_wrapper(handle->SetPosition, 2, handle, 0); + if (EFI_ERROR(err)) + goto out; - if (pe.NumberOfSections > 96) { - err = EFI_LOAD_ERROR; + err = uefi_call_wrapper(handle->Read, 3, handle, &len, header); + if (EFI_ERROR(err)) { goto out; } - - /* the sections start directly after the headers */ - err = uefi_call_wrapper(handle->SetPosition, 2, handle, dos.ExeHeader + sizeof(magic) + sizeof(pe) + pe.SizeOfOptionalHeader); - if (EFI_ERROR(err)) + if (len != headerlen) { + err = EFI_LOAD_ERROR; goto out; - - for (i = 0; i < pe.NumberOfSections; i++) { - struct PeSectionHeader sect; - UINTN j; - - len = sizeof(sect); - err = uefi_call_wrapper(handle->Read, 3, handle, &len, §); - if (EFI_ERROR(err)) - goto out; - if (len != sizeof(sect)) { - err = EFI_LOAD_ERROR; - goto out; - } - for (j = 0; sections[j]; j++) { - if (CompareMem(sect.Name, sections[j], strlena(sections[j])) != 0) - continue; - - if (addrs) - addrs[j] = (UINTN)sect.VirtualAddress; - if (offsets) - offsets[j] = (UINTN)sect.PointerToRawData; - if (sizes) - sizes[j] = (UINTN)sect.VirtualSize; - } } + err = pe_memory_locate_sections(header, sections, addrs, offsets, sizes); out: + if (header) + FreePool(header); uefi_call_wrapper(handle->Close, 1, handle); return err; } diff --git a/src/boot/efi/pefile.h b/src/boot/efi/pe.h similarity index 67% rename from src/boot/efi/pefile.h rename to src/boot/efi/pe.h index 2e445ede178..fa8feea758d 100644 --- a/src/boot/efi/pefile.h +++ b/src/boot/efi/pe.h @@ -15,6 +15,8 @@ #ifndef __SDBOOT_PEFILE_H #define __SDBOOT_PEFILE_H -EFI_STATUS pefile_locate_sections(EFI_FILE *dir, CHAR16 *path, - CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes); +EFI_STATUS pe_memory_locate_sections(CHAR8 *base, + CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes); +EFI_STATUS pe_file_locate_sections(EFI_FILE *dir, CHAR16 *path, + CHAR8 **sections, UINTN *addrs, UINTN *offsets, UINTN *sizes); #endif diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 98730a5d3d8..bab5d46de9a 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -17,10 +17,10 @@ #include "disk.h" #include "graphics.h" #include "linux.h" -#include "pefile.h" +#include "measure.h" +#include "pe.h" #include "splash.h" #include "util.h" -#include "measure.h" /* magic string to find in the binary image */ static const char __attribute__((used)) magic[] = "#### LoaderInfo: systemd-stub " PACKAGE_VERSION " ####"; @@ -29,8 +29,6 @@ static const EFI_GUID global_guid = EFI_GLOBAL_VARIABLE; EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { EFI_LOADED_IMAGE *loaded_image; - EFI_FILE *root_dir; - CHAR16 *loaded_image_path; CHAR8 *b; UINTN size; BOOLEAN secure = FALSE; @@ -59,22 +57,12 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { return err; } - root_dir = LibOpenRoot(loaded_image->DeviceHandle); - if (!root_dir) { - Print(L"Unable to open root directory: %r ", err); - uefi_call_wrapper(BS->Stall, 1, 3 * 1000 * 1000); - return EFI_LOAD_ERROR; - } - - loaded_image_path = DevicePathToStr(loaded_image->FilePath); - if (efivar_get_raw(&global_guid, L"SecureBoot", &b, &size) == EFI_SUCCESS) { if (*b > 0) secure = TRUE; FreePool(b); } - - err = pefile_locate_sections(root_dir, loaded_image_path, sections, addrs, offs, szs); + err = pe_memory_locate_sections(loaded_image->ImageBase, sections, addrs, offs, szs); if (EFI_ERROR(err)) { Print(L"Unable to locate embedded .linux section: %r ", err); uefi_call_wrapper(BS->Stall, 1, 3 * 1000 * 1000);