On Thu, 2 Feb 2023 at 19:49, Kinney, Michael D
<michael.d.kin...@intel.com> wrote:
>
> Hi Ard,
>
> Since the PE/COFF image does not contain this information, is there an option
> to add the information to an FFS file. Either as a new bit in a standard
> header
> or as a GUIDed section defined by EDK II?
>
> Since an FV may contain content build from source and additional content
> Integrated as binaries from other vendors, having a PCD that scopes this
> attribute to all FVs many only work for FW builds that are 100% from sources.
>
I didn't quite consider that scenario tbh.
It would be nice if we could avoid another PI spec dance for this, and
get some kind of IBT/BTI annotation added to the PE/COFF spec. That
way, we cover this issue, as well as clear the way for the use if
IBT/BTI when running under the firmware.
Are TE images a concern here as well? I.e., are those likely to be
used for runtime DXE images in firmware volumes?
>
> > -----Original Message-----
> > From: Ard Biesheuvel <a...@kernel.org>
> > Sent: Thursday, February 2, 2023 10:04 AM
> > To: devel@edk2.groups.io
> > Cc: Ard Biesheuvel <a...@kernel.org>; Kinney, Michael D
> > <michael.d.kin...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>; Yao,
> > Jiewen <jiewen....@intel.com>; Kubacki, Michael
> > <michael.kuba...@microsoft.com>; Sean Brogan <sean.bro...@microsoft.com>;
> > Rebecca
> > Cran <quic_rc...@quicinc.com>; Leif Lindholm <quic_llind...@quicinc.com>;
> > Sami Mujawar <sami.muja...@arm.com>; Taylor Beebe
> > <t...@taylorbeebe.com>
> > Subject: [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem
> > attributes table
> >
> > The memory attributes table has been extended with a flag that indicates
> > whether or not the OS is permitted to map the EFI runtime code regions
> > with strict enforcement for IBT/BTI landing pad instructions.
> >
> > This is generally a property of the firmware build, and so we can permit
> > a platform to set this property using a PCD, and put the burden on the
> > platform definition to set the toolchain options accordingly.
> >
> > There is one snag, however: PE/COFF does not expose whether or not the
> > code was generated with landing pads, so if any runtime DXE drivers were
> > loaded from storage other than the firmware volumes, we must assume that
> > setting the CFI flag in the memory attributes table is unsafe.
> >
> > Signed-off-by: Ard Biesheuvel <a...@kernel.org>
> > ---
> > MdeModulePkg/Core/Dxe/DxeMain.h | 2 ++
> > MdeModulePkg/Core/Dxe/DxeMain.inf | 1 +
> > MdeModulePkg/Core/Dxe/Image/Image.c | 11 +++++++++++
> > MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 7 ++++++-
> > MdeModulePkg/MdeModulePkg.dec | 8 ++++++++
> > 5 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h
> > b/MdeModulePkg/Core/Dxe/DxeMain.h
> > index 815a6b4bd844..427a5fc78f72 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.h
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.h
> > @@ -280,6 +280,8 @@ extern EFI_MEMORY_TYPE_INFORMATION
> > gMemoryTypeInformation[EfiMaxMemoryType + 1]
> > extern BOOLEAN gDispatcherRunning;
> >
> > extern EFI_RUNTIME_ARCH_PROTOCOL gRuntimeTemplate;
> >
> >
> >
> > +extern BOOLEAN gMemoryAttributesTableForwardCfi;
> >
> > +
> >
> > extern EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE
> > gLoadModuleAtFixAddressConfigurationTable;
> >
> > extern BOOLEAN
> > gLoadFixedAddressCodeMemoryReady;
> >
> > //
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > index 35d5bf0dee6f..e6ff67773a69 100644
> > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > @@ -187,6 +187,7 @@ [Pcd]
> > gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > ## CONSUMES
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> > ## CONSUMES
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth
> > ## CONSUMES
> >
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi
> > ## CONSUMES
> >
> >
> >
> > # [Hob]
> >
> > # RESOURCE_DESCRIPTOR ## CONSUMES
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> > b/MdeModulePkg/Core/Dxe/Image/Image.c
> > index 06cc6744b8c6..181fefdb6657 100644
> > --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> > @@ -1398,6 +1398,17 @@ CoreLoadImageCommon (
> > CoreNewDebugImageInfoEntry (EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL,
> > &Image->Info, Image->Handle);
> >
> > }
> >
> >
> >
> > + //
> >
> > + // If we loaded a runtime DXE driver from something other than a FV, it
> >
> > + // was not built as part of the firmware image, and so we cannot assume
> >
> > + // that it was built with IBT/BTI landing pads for forward edge control
> >
> > + // flow integrity.
> >
> > + //
> >
> > + if (!ImageIsFromFv &&
> >
> > + (Image->ImageContext.ImageCodeMemoryType == EfiRuntimeServicesCode))
> > {
> >
> > + gMemoryAttributesTableForwardCfi = FALSE;
> >
> > + }
> >
> > +
> >
> > //
> >
> > // Reinstall loaded image protocol to fire any notifications
> >
> > //
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
> > b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
> > index e07921371187..cdd35ade0a8a 100644
> > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
> > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
> > @@ -89,6 +89,7 @@ BOOLEAN mMemoryAttributesTableEnable
> > = TRUE;
> > BOOLEAN mMemoryAttributesTableEndOfDxe = FALSE;
> >
> > EFI_MEMORY_ATTRIBUTES_TABLE *mMemoryAttributesTable = NULL;
> >
> > BOOLEAN mMemoryAttributesTableReadyToBoot = FALSE;
> >
> > +BOOLEAN gMemoryAttributesTableForwardCfi =
> > FixedPcdGetBool (PcdMemoryAttributesTableForwardCfi);
> >
> >
> >
> > /**
> >
> > Install MemoryAttributesTable.
> >
> > @@ -182,7 +183,11 @@ InstallMemoryAttributesTable (
> > MemoryAttributesTable->Version =
> > EFI_MEMORY_ATTRIBUTES_TABLE_VERSION;
> >
> > MemoryAttributesTable->NumberOfEntries = RuntimeEntryCount;
> >
> > MemoryAttributesTable->DescriptorSize = (UINT32)DescriptorSize;
> >
> > - MemoryAttributesTable->Reserved = 0;
> >
> > + if (gMemoryAttributesTableForwardCfi) {
> >
> > + MemoryAttributesTable->Flags =
> > EFI_MEMORY_ATTRIBUTES_FLAGS_RT_FORWARD_CONTROL_FLOW_GUARD;
> >
> > + } else {
> >
> > + MemoryAttributesTable->Flags = 0;
> >
> > + }
> >
> > DEBUG ((DEBUG_VERBOSE, "MemoryAttributesTable:\n"));
> >
> > DEBUG ((DEBUG_VERBOSE, " Version - 0x%08x\n",
> > MemoryAttributesTable->Version));
> >
> > DEBUG ((DEBUG_VERBOSE, " NumberOfEntries - 0x%08x\n",
> > MemoryAttributesTable->NumberOfEntries));
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> > index 9605c617b7a8..d336a38655a3 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -1093,6 +1093,14 @@ [PcdsFixedAtBuild]
> > # @Prompt Enable UEFI Stack Guard.
> >
> > gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055
> >
> >
> >
> > + ## Indicates whether the EFI memory attributes table will inform the OS
> > that
> >
> > + # forward edge control flow guards have been inserted into the runtime
> > services
> >
> > + # code regions.
> >
> > + # TRUE - Runtime code has forward control flow guards.<BR>
> >
> > + # FALSE - Runtime code does not have forward control flow guards.<BR>
> >
> > + # @Prompt Enable forward control flow guards in EFI memory attributes
> > table
> >
> > +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi|FALSE|BOOLEAN|0x30001056
> >
> > +
> >
> > [PcdsFixedAtBuild, PcdsPatchableInModule]
> >
> > ## Dynamic type PCD can be registered callback function for Pcd setting
> > action.
> >
> > # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of
> > callback function
> >
> > --
> > 2.39.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#99500): https://edk2.groups.io/g/devel/message/99500
Mute This Topic: https://groups.io/mt/96705496/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
