From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by mx.groups.io with SMTP id smtpd.web09.7428.1581692713965167461 for ; Fri, 14 Feb 2020 07:05:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=Vecv8ge0; spf=pass (domain: linaro.org, ip: 209.85.221.67, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f67.google.com with SMTP id y17so11263064wrh.5 for ; Fri, 14 Feb 2020 07:05:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=CB56ZGGnfQRymEi6px/cwYokak6rl/VobdGMulxxhUE=; b=Vecv8ge0D05+0i3LhuYevl3hCB48lchfSzsaED9LNCkHb8y7wXS5XEsm56+uH7iLjx N+ji/wHT+J5RVfNNPqsl68PQDDv0U/DAbBG1JtEn6tlFzFwsvawASNsW3XhBTmsBagji 4OpG8uU/4bcztf96N65FgXW1lIUj3BUR3DmshedUEiA885lLsC3xUkc7jcJq9HrsE3fY BoAAK0XalVSZ+5s3ebdvbQsw9hIkoU9RsnMGLHUSsxuF7lPzFZyo1V5UUDr+Waa74M6L y3fDP5/97AV88Qw5OVJVg77MupOX14crhzd0AcmhCjou/VbSuiryHqztNS0gUe4FM2qz kcZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=CB56ZGGnfQRymEi6px/cwYokak6rl/VobdGMulxxhUE=; b=Zz5zX+weaucT/6qjX1RX0aceEEvR8MGpjfL291dxwPy9VLptIDj6H4PpGQ8Uzz71de KhKhXVy8vfHLbBskr2XBM2lcr/wMVZeSDLj4GeNElqRIUX8dbR1bqs/4e96+oc1l9dET dm2yyPGfpDyQCKLwpfp5nDlI6YeHKMfUwY9Or5cGeOqkRYBUxd1wJVh+0papYeryrnvL cj1dexWD5HvltEdqj5w6HjsB7jiC8om/diycW5+L1376r2iWFK8b+zbUt8e4hPFOT6Aj P+8WQu46cC77djrF0kLvebMyjc6YrexTxU0S6WQUa/BsMb7xSpQOCcAglz/VQPouBJby /cSA== X-Gm-Message-State: APjAAAU+P5UNRTSRoxYXGY/VgXzFquj+uPN8zsHC1YXHEhp23Fz0v/W5 Hco95SmHZgc9N0cALQJi4g9ZBE93HyqpmP6JV1nJFVWzfOZtzg== X-Google-Smtp-Source: APXvYqxLG9xtE3tjvhMRt1XN81hdCX4XgrTYjhVef027AHWiVxA5XDzXRNGy+zZpBeA2Wtbbb0Ca3nMjW4xXqXPIiOk= X-Received: by 2002:a5d:65cf:: with SMTP id e15mr4432590wrw.126.1581692711947; Fri, 14 Feb 2020 07:05:11 -0800 (PST) MIME-Version: 1.0 References: <20200214114155.22859-1-ard.biesheuvel@arm.com> <8f18a788-91bd-63c7-447a-c28fa3613e6d@redhat.com> In-Reply-To: <8f18a788-91bd-63c7-447a-c28fa3613e6d@redhat.com> From: "Ard Biesheuvel" Date: Fri, 14 Feb 2020 16:05:00 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg IA32: add support for loading X64 Linux images To: edk2-devel-groups-io , Laszlo Ersek Cc: Leif Lindholm , Peter Jones , Matthew Garrett , Alexander Graf , Daniel Kiper , Arvind Sankar Content-Type: text/plain; charset="UTF-8" On Fri, 14 Feb 2020 at 15:45, Laszlo Ersek wrote: > > On 02/14/20 12:41, Ard Biesheuvel wrote: > > This is the UEFI counterpart to my Linux series [0] which generalizes > > mixed mode support into a feature that requires very little internal > > knowledge about the architecture specifics of booting Linux on the > > part of the bootloader or firmware. > > > > Instead, we add a .compat PE/COFF section containing an array of > > PE_COMPAT nodes containing tuples that > > describe alternate entrypoints into the image for different native > > machine types, e.g., IA-32 in a 64-bit image so it can be booted > > from IA-32 firmware. > > > > This patch implements the PE/COFF emulator protocol to take this new > > section into account, so that such images can simply be loaded via > > LoadImage/StartImage, e.g., straight from the UEFI shell. > > (1) Please mention that this driver / protocol implementation ties in > with the larger PE/COFF emulator feature that is based on e.g. your > commit 57df17fe26cd ("MdeModulePkg/DxeCore: invoke the emulator protocol > for foreign images", 2019-04-14). > OK > (2) I think we're now in the soft feature freeze, so I believe this > patch will have to wait until the next dev cycle opens up. I hope that's > OK with you. > Sure, that is fine. I just want the different pieces of the puzzle to be public on a mailing list somewhere so people can see what the opposite end looks like (seen from the Linux side :-)) > > > > [0] https://lore.kernel.org/linux-arm-kernel/20200213145928.7047-1-ardb@kernel.org/ > > > > Cc: lersek@redhat.com > > Cc: leif@nuviainc.com > > Cc: pjones@redhat.com > > Cc: mjg59@google.com > > Cc: agraf@csgraf.de > > Cc: daniel.kiper@oracle.com > > Cc: nivedita@alum.mit.edu > > Signed-off-by: Ard Biesheuvel > > --- > > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c | 151 ++++++++++++++++++++ > > OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf | 36 +++++ > > OvmfPkg/OvmfPkgIa32.dsc | 2 + > > OvmfPkg/OvmfPkgIa32.fdf | 2 + > > 4 files changed, 191 insertions(+) > > > > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > > new file mode 100644 > > index 000000000000..a9c960c64be9 > > --- /dev/null > > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.c > > @@ -0,0 +1,151 @@ > > +/** @file > > + * PE/COFF emulator protocol implementation to start Linux kernel > > + * images from non-native firmware > > + * > > + * Copyright (c) 2020, ARM Ltd. All rights reserved.
> > + * > > + * SPDX-License-Identifier: BSD-2-Clause-Patent > > + * > > + */ > > + > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > + > > +#pragma pack(1) > > +typedef struct { > > + UINT8 Type; > > + UINT8 Size; > > + UINT16 MachineType; > > + UINT32 EntryPoint; > > +} PE_COMPAT_TYPE1; > > +#pragma pack() > > + > > +STATIC > > +BOOLEAN > > +EFIAPI > > +IsImageSupported ( > > + IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *This, > > + IN UINT16 ImageType, > > + IN EFI_DEVICE_PATH_PROTOCOL *DevicePath OPTIONAL > > + ) > > +{ > > + if (ImageType != EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION && > > + ImageType != EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER) { > > + return FALSE; > > + } > > + return TRUE; > > +} > > + > > +STATIC > > +EFI_IMAGE_ENTRY_POINT > > +EFIAPI > > +GetCompatEntryPoint ( > > + IN EFI_PHYSICAL_ADDRESS ImageBase > > + ) > > +{ > > + EFI_IMAGE_DOS_HEADER *DosHdr; > > + UINTN PeCoffHeaderOffset; > > + EFI_IMAGE_NT_HEADERS32 *Pe32; > > + EFI_IMAGE_SECTION_HEADER *Section; > > + UINTN NumberOfSections; > > + PE_COMPAT_TYPE1 *PeCompat; > > + > > + DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase; > > + if (DosHdr->e_magic != EFI_IMAGE_DOS_SIGNATURE) { > > + return NULL; > > + } > > + > > + PeCoffHeaderOffset = DosHdr->e_lfanew; > > + Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageBase + PeCoffHeaderOffset); > > + > > + Section = (EFI_IMAGE_SECTION_HEADER *)((UINTN)ImageBase + PeCoffHeaderOffset + > > + sizeof(UINT32) + > > + sizeof(EFI_IMAGE_FILE_HEADER) + > > + Pe32->FileHeader.SizeOfOptionalHeader); > > + NumberOfSections = (UINTN)Pe32->FileHeader.NumberOfSections; > > + > > + while (NumberOfSections--) { > > + if (!CompareMem (Section->Name, ".compat", sizeof (Section->Name))) { > > + // > > + // Dereference the section contents to find the mixed mode entry point > > + // > > + PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)ImageBase + Section->VirtualAddress); > > + > > + while (PeCompat->Type != 0) { > > + if (PeCompat->Type == 1 && > > + PeCompat->Size >= sizeof (PE_COMPAT_TYPE1) && > > + EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeCompat->MachineType)) { > > + > > + return (EFI_IMAGE_ENTRY_POINT)((UINTN)ImageBase + PeCompat->EntryPoint); > > + } > > + PeCompat = (PE_COMPAT_TYPE1 *)((UINTN)PeCompat + PeCompat->Size); > > + } > > + } > > + Section++; > > + } > > + return NULL; > > +} > > + > > +STATIC > > +EFI_STATUS > > +EFIAPI > > +Unsupported ( > > + IN EFI_HANDLE ImageHandle, > > + IN EFI_SYSTEM_TABLE *SystemTable > > + ) > > +{ > > + return EFI_UNSUPPORTED; > > +} > > + > > +STATIC > > +EFI_STATUS > > +EFIAPI > > +RegisterImage ( > > + IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *This, > > + IN EFI_PHYSICAL_ADDRESS ImageBase, > > + IN UINT64 ImageSize, > > + IN OUT EFI_IMAGE_ENTRY_POINT *EntryPoint > > + ) > > +{ > > + *EntryPoint = GetCompatEntryPoint (ImageBase) ?: &Unsupported; > > (3) Operator "?:" is a GNU-ism, please replace it. > OK > > + > > + return EFI_SUCCESS; > > +} > > + > > +STATIC > > +EFI_STATUS > > +EFIAPI > > +UnregisterImage ( > > + IN EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL *This, > > + IN EFI_PHYSICAL_ADDRESS ImageBase > > + ) > > +{ > > + return EFI_SUCCESS; > > +} > > + > > +STATIC EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL mCompatLoaderPeCoffEmuProtocol = { > > + IsImageSupported, > > + RegisterImage, > > + UnregisterImage, > > + EDKII_PECOFF_IMAGE_EMULATOR_VERSION, > > + EFI_IMAGE_MACHINE_X64 > > +}; > > + > > +EFI_STATUS > > +EFIAPI > > +CompatImageLoaderDxeEntryPoint ( > > + IN EFI_HANDLE ImageHandle, > > + IN EFI_SYSTEM_TABLE *SystemTable > > + ) > > +{ > > + return gBS->InstallProtocolInterface (&ImageHandle, > > + &gEdkiiPeCoffImageEmulatorProtocolGuid, > > + EFI_NATIVE_INTERFACE, > > + &mCompatLoaderPeCoffEmuProtocol); > > +} > > diff --git a/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > > new file mode 100644 > > index 000000000000..82369384fbe6 > > --- /dev/null > > +++ b/OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > > @@ -0,0 +1,36 @@ > > +## @file > > +# PE/COFF emulator protocol implementation to start Linux kernel > > +# images from non-native firmware > > +# > > +# Copyright (c) 2020, ARM Ltd. All rights reserved.
> > +# > > +# SPDX-License-Identifier: BSD-2-Clause-Patent > > +# > > +## > > + > > +[Defines] > > + INF_VERSION = 1.27 > > + BASE_NAME = CompatImageLoaderDxe > > + FILE_GUID = 1019f54a-2560-41b2-87b0-6750b98f3eff > > + MODULE_TYPE = DXE_DRIVER > > + VERSION_STRING = 1.0 > > + ENTRY_POINT = CompatImageLoaderDxeEntryPoint > > + > > +[Sources] > > + CompatImageLoaderDxe.c > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + MdeModulePkg/MdeModulePkg.dec > > + > > +[LibraryClasses] > > + BaseMemoryLib > > + PeCoffLib > > + UefiBootServicesTableLib > > + UefiDriverEntryPoint > > + > > +[Protocols] > > + gEdkiiPeCoffImageEmulatorProtocolGuid ## PRODUCES > > + > > +[Depex] > > + TRUE > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > > index 3ffaa9b3388f..66522a4bc555 100644 > > --- a/OvmfPkg/OvmfPkgIa32.dsc > > +++ b/OvmfPkg/OvmfPkgIa32.dsc > > @@ -916,3 +916,5 @@ [Components] > > NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf > > } > > !endif > > + > > + OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > > diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf > > index 586bbff08585..3411e4ca676e 100644 > > --- a/OvmfPkg/OvmfPkgIa32.fdf > > +++ b/OvmfPkg/OvmfPkgIa32.fdf > > @@ -350,6 +350,8 @@ [FV.DXEFV] > > !endif > > !endif > > > > +INF OvmfPkg/CompatImageLoaderDxe/CompatImageLoaderDxe.inf > > + > > ################################################################################ > > > > [FV.FVMAIN_COMPACT] > > > > (4) Please introduce a new build flag (such as -D > LOAD_X64_ON_IA32_ENABLE) for gating this feature. It's fine if the > default value of the flag is TRUE, I'd just like a possibility to > exclude this driver from the firmware without out-of-tree patches. > OK > (5) Can you please explain how EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL > relates to Secure Boot and/or Trusted Boot? > > (5a) Is the ".compat" section included in the image hashing? > Yes. > (5b) Does the DXE core subject such non-native images to verification / > measurement at all? (Sorry I didn't follow your original work on > EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL.) > Yes. They are treated entirely like ordinary images, with all the policy checks regarding authentication and measurement. This is actually the whole point of this exercise: there is a whole stack of secure boot and measured boot related patches trying to make its way upstream that are based on circumventing LoadImage and StartImage, open coding authentication checks using a protocol exposed by shim, and taking its own measurements of the kernel as well. I fully understand how this came about, but I strongly believe that we should not extrapolate this to other architectures. What I would like to see instead is shim hooking the LoadImage and StartImage boot services (which it already does btw, but not for exposing its augmented functionality to the caller), so that other components can be agnostic about whether shim is being used or not. There are currently two reasons why we cannot use LoadImage/StartImage: - initrd loading (hence the other set of changes that I proposed) - mixed mode support With those two out of the way, we should be able to converge on a generic EFI booting protocol for Linux that is identical across architectures, with the only Linux specific pieces being this .compat section handling and the LoadFile2 protocol for initrd. Note that mixed mode on actual existing hardware still requires special handling, given that their firmware predates this PE/COFF emu protocol, but this handling may be something we could fold into shim, given the amount of PE/COFF parsing it does already, which makes it trivial to just jump to the IA32 entry point in the X64 binary in its implementation of StartImage()