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 d429a12522628615902945d64ec62442620114f4..1e990b38251261dc5bedb5540f1bde16c6d30541 100644 (file)
--- 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 9e06e22ff47927ed21d40c8bd497faa5d01e25c2..5ef5b2d20b5afcb6233fead233f0ec1f02e76b50 100644 (file)
--- 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 77fff77b69fa50384398b25c439bf8bcad78dc42..054e8edbc63fbfc1ff60750ef4773a020f1303c6 100644 (file)
--- a/src/boot/efi/pefile.c
+++ b/src/boot/efi/pe.c
@@ -15,7 +15,7 @@
 #include <efi.h>
 #include <efilib.h>
 
-#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, &sect);
-                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 2e445ede178ec3518284ee2d9382bd45f29c79a2..fa8feea758d176ad2a4c6f956a8f9db5fdce5d55 100644 (file)
--- 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 98730a5d3d8417ad337b8adaebf89e4e731f223a..bab5d46de9a6bc882ebbda3f27bc2076e307e3ea 100644 (file)
--- 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);