-----Original Message-----
From: Michael Kubacki <mikuback@linux.microsoft.com>
Sent: Friday, February 3, 2023 8:24 AM
To: devel@edk2.groups.io;
ardb@kernel.org; Kinney, Michael D
<michael.d.kinney@intel.com>
Cc: Gao, Liming <gaoliming@byosoft.com.cn>; Yao, Jiewen
<jiewen.yao@intel.com>; Kubacki, Michael <michael.kubacki@microsoft.com>;
Sean Brogan <sean.brogan@microsoft.com>; Rebecca Cran
<quic_rcran@quicinc.com>; Leif Lindholm <quic_llindhol@quicinc.com>; Sami
Mujawar <sami.mujawar@arm.com>; Taylor Beebe <t@taylorbeebe.com>
Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge
CFI in mem attributes table
Ard, I am still actively tracking this for the PE/COFF spec.
Unfortunately, I don't have more firm info right now but I suggest
holding off on alternatives for the time being and I will reply back as
soon as the next steps are known.
Thanks,
Michael
On 2/2/2023 2:00 PM, Ard Biesheuvel wrote:
On Thu, 2 Feb 2023 at 19:49, Kinney, Michael D
<michael.d.kinney@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 <ardb@kernel.org>
Sent: Thursday, February 2, 2023 10:04 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ardb@kernel.org>; Kinney, Michael D
<michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Yao,
Jiewen <jiewen.yao@intel.com>; Kubacki, Michael
<michael.kubacki@microsoft.com>; Sean Brogan
<sean.brogan@microsoft.com>; Rebecca
Cran <quic_rcran@quicinc.com>; Leif Lindholm
<quic_llindhol@quicinc.com>; Sami Mujawar <sami.mujawar@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 <ardb@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|0x30
001055
+ ## 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|FAL
SE|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