From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.8719.1675383849498308944 for ; Thu, 02 Feb 2023 16:24:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=siTxEPYM; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id C122D20B74F7; Thu, 2 Feb 2023 16:24:07 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com C122D20B74F7 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1675383848; bh=4gWJb3yPX95rPKijverzLZ3eQcPaEy/Hfvnu2tn/muo=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=siTxEPYMHYydcju7ABPyyBrPAIxrm/e2k5A2vlDfD3KnueviiERaESDO1lMDu21vQ LXarTbkHQ6Aaoo2sS0CzdN09xOP+RqIY/dmNY6Cap3pIDcacZIc2XDuglmeJ9TVS3A cUMmKnU0m2oSK9UkeeUKgczx7mNEXhfbCT3u5aII= Message-ID: Date: Thu, 2 Feb 2023 19:24:06 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table To: devel@edk2.groups.io, ardb@kernel.org, "Kinney, Michael D" Cc: "Gao, Liming" , "Yao, Jiewen" , "Kubacki, Michael" , Sean Brogan , Rebecca Cran , Leif Lindholm , Sami Mujawar , Taylor Beebe References: <20230202180335.2256160-1-ardb@kernel.org> <20230202180335.2256160-3-ardb@kernel.org> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 > 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 >>> Sent: Thursday, February 2, 2023 10:04 AM >>> To: devel@edk2.groups.io >>> Cc: Ard Biesheuvel ; Kinney, Michael D ; Gao, Liming ; Yao, >>> Jiewen ; Kubacki, Michael ; Sean Brogan ; Rebecca >>> Cran ; Leif Lindholm ; Sami Mujawar ; Taylor Beebe >>> >>> 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 >>> --- >>> 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.
>>> >>> + # FALSE - Runtime code does not have forward control flow guards.
>>> >>> + # @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 >> > > > >