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.9660.1679694521512457384 for ; Fri, 24 Mar 2023 14:48:41 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=Lw0Qyb3D; 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 D0D0C20FC468; Fri, 24 Mar 2023 14:48:37 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com D0D0C20FC468 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1679694519; bh=IrrooNG+c1kUy+C9Gd1UBUCbTNSlVxA8kf1LGWO/BOQ=; h=Date:Subject:From:To:Cc:References:In-Reply-To:From; b=Lw0Qyb3DiMnYgj9dkPUjYTGH7DZfcr0wMp5tWb1u5fDiFBZ0VQKUx3bvSfdziMwOi YyTLJFOAuUvZ7AWitAPOUfHcEOpKm5W1gamc4rsgf7NfIwmkqO3M8Zr/rUHeUmq/5w MmBEo7Dv9ky6PYIbuEUTIQCNh7+psWC8gAd3DKdU= Message-ID: Date: Fri, 24 Mar 2023 17:48:36 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [edk2-devel] [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in mem attributes table From: "Michael Kubacki" 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> In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable This bit is now available in the PE/COFF spec in the Extended DLL=20 Characteristics section. https://learn.microsoft.com/windows/win32/debug/pe-format#extended-dll-char= acteristics Name: "IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT" Value: "0x0040" On 2/2/2023 7:24 PM, Michael Kubacki wrote: > Ard, I am still actively tracking this for the PE/COFF spec. >=20 > Unfortunately, I don't have more firm info right now but I suggest=20 > holding off on alternatives for the time being and I will reply back as= =20 > soon as the next steps are known. >=20 > Thanks, > Michael >=20 > 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=20 >>> an option >>> to add the information to an FFS file. Either as a new bit in a=20 >>> standard header >>> or as a GUIDed section defined by EDK II? >>> >>> Since an FV may contain content build from source and additional conten= t >>> Integrated as binaries from other vendors, having a PCD that scopes thi= s >>> attribute to all FVs many only work for FW builds that are 100% from=20 >>> 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=20 >>>> ; Gao, Liming=20 >>>> ; Yao, >>>> Jiewen ; Kubacki, Michael=20 >>>> ; Sean Brogan=20 >>>> ; Rebecca >>>> Cran ; Leif Lindholm=20 >>>> ; Sami Mujawar ;=20 >>>> Taylor Beebe >>>> >>>> Subject: [RFC PATCH 2/3] MdeModulePkg: Enable forward edge CFI in=20 >>>> mem attributes table >>>> >>>> The memory attributes table has been extended with a flag that=20 >>>> 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=20 >>>> 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=20 >>>> were >>>> loaded from storage other than the firmware volumes, we must assume=20 >>>> that >>>> setting the CFI flag in the memory attributes table is unsafe. >>>> >>>> Signed-off-by: Ard Biesheuvel >>>> --- >>>> =C2=A0 MdeModulePkg/Core/Dxe/DxeMain.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 |=C2=A0 2 ++ >>>> =C2=A0 MdeModulePkg/Core/Dxe/DxeMain.inf=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |= =C2=A0 1 + >>>> =C2=A0 MdeModulePkg/Core/Dxe/Image/Image.c=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | 11 ++++++= +++++ >>>> =C2=A0 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c |=C2=A0 7 ++= ++++- >>>> =C2=A0 MdeModulePkg/MdeModulePkg.dec=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 |=C2=A0 8 ++++++++ >>>> =C2=A0 5 files changed, 28 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h=20 >>>> 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 =20 >>>> gMemoryTypeInformation[EfiMaxMemoryType + 1] >>>> =C2=A0 extern BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gDispatc= herRunning; >>>> >>>> =C2=A0 extern EFI_RUNTIME_ARCH_PROTOCOL=C2=A0 gRuntimeTemplate; >>>> >>>> >>>> >>>> +extern BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gMemoryAttribu= tesTableForwardCfi; >>>> >>>> + >>>> >>>> =C2=A0 extern EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE =20 >>>> gLoadModuleAtFixAddressConfigurationTable; >>>> >>>> =C2=A0 extern BOOLEAN =20 >>>> gLoadFixedAddressCodeMemoryReady; >>>> >>>> =C2=A0 // >>>> >>>> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf=20 >>>> 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] >>>> =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 ## CONSUMES >>>> >>>> =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ## CONSUMES >>>> >>>> =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ## CONSUMES >>>> >>>> + =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ## CONSUMES >>>> >>>> >>>> >>>> =C2=A0 # [Hob] >>>> >>>> =C2=A0 # RESOURCE_DESCRIPTOR=C2=A0=C2=A0 ## CONSUMES >>>> >>>> diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c=20 >>>> 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 ( >>>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CoreNewDebugImageInfoEntry (EFI_DEBUG_I= MAGE_INFO_TYPE_NORMAL,=20 >>>> &Image->Info, Image->Handle); >>>> >>>> =C2=A0=C2=A0=C2=A0 } >>>> >>>> >>>> >>>> +=C2=A0 // >>>> >>>> +=C2=A0 // If we loaded a runtime DXE driver from something other than= a=20 >>>> FV, it >>>> >>>> +=C2=A0 // was not built as part of the firmware image, and so we cann= ot=20 >>>> assume >>>> >>>> +=C2=A0 // that it was built with IBT/BTI landing pads for forward edg= e=20 >>>> control >>>> >>>> +=C2=A0 // flow integrity. >>>> >>>> +=C2=A0 // >>>> >>>> +=C2=A0 if (!ImageIsFromFv && >>>> >>>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (Image->ImageContext.ImageCodeMemoryTy= pe =3D=3D=20 >>>> EfiRuntimeServicesCode)) { >>>> >>>> +=C2=A0=C2=A0=C2=A0 gMemoryAttributesTableForwardCfi =3D FALSE; >>>> >>>> +=C2=A0 } >>>> >>>> + >>>> >>>> =C2=A0=C2=A0=C2=A0 // >>>> >>>> =C2=A0=C2=A0=C2=A0 // Reinstall loaded image protocol to fire any noti= fications >>>> >>>> =C2=A0=C2=A0=C2=A0 // >>>> >>>> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c=20 >>>> 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 =20 >>>> mMemoryAttributesTableEnable=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D TRUE; >>>> =C2=A0 BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mM= emoryAttributesTableEndOfDxe=C2=A0=C2=A0=C2=A0 =3D=20 >>>> FALSE; >>>> >>>> =C2=A0 EFI_MEMORY_ATTRIBUTES_TABLE=C2=A0 *mMemoryAttributesTable=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D=20 >>>> NULL; >>>> >>>> =C2=A0 BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 mM= emoryAttributesTableReadyToBoot =3D=20 >>>> FALSE; >>>> >>>> +BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gMemoryA= ttributesTableForwardCfi=C2=A0 =3D=20 >>>> FixedPcdGetBool (PcdMemoryAttributesTableForwardCfi); >>>> >>>> >>>> >>>> =C2=A0 /** >>>> >>>> =C2=A0=C2=A0=C2=A0 Install MemoryAttributesTable. >>>> >>>> @@ -182,7 +183,11 @@ InstallMemoryAttributesTable ( >>>> =C2=A0=C2=A0=C2=A0 MemoryAttributesTable->Version=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D=20 >>>> EFI_MEMORY_ATTRIBUTES_TABLE_VERSION; >>>> >>>> =C2=A0=C2=A0=C2=A0 MemoryAttributesTable->NumberOfEntries =3D RuntimeE= ntryCount; >>>> >>>> =C2=A0=C2=A0=C2=A0 MemoryAttributesTable->DescriptorSize=C2=A0 =3D (UI= NT32)DescriptorSize; >>>> >>>> -=C2=A0 MemoryAttributesTable->Reserved=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 =3D 0; >>>> >>>> +=C2=A0 if (gMemoryAttributesTableForwardCfi) { >>>> >>>> +=C2=A0=C2=A0=C2=A0 MemoryAttributesTable->Flags=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D=20 >>>> EFI_MEMORY_ATTRIBUTES_FLAGS_RT_FORWARD_CONTROL_FLOW_GUARD; >>>> >>>> +=C2=A0 } else { >>>> >>>> +=C2=A0=C2=A0=C2=A0 MemoryAttributesTable->Flags=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D 0; >>>> >>>> +=C2=A0 } >>>> >>>> =C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_VERBOSE, "MemoryAttributesTable:\n"))= ; >>>> >>>> =C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_VERBOSE, "=C2=A0 Version=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 - 0x%08x= \n",=20 >>>> MemoryAttributesTable->Version)); >>>> >>>> =C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_VERBOSE, "=C2=A0 NumberOfEntries=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 - 0x%08x\n",=20 >>>> MemoryAttributesTable->NumberOfEntries)); >>>> >>>> diff --git a/MdeModulePkg/MdeModulePkg.dec=20 >>>> b/MdeModulePkg/MdeModulePkg.dec >>>> index 9605c617b7a8..d336a38655a3 100644 >>>> --- a/MdeModulePkg/MdeModulePkg.dec >>>> +++ b/MdeModulePkg/MdeModulePkg.dec >>>> @@ -1093,6 +1093,14 @@ [PcdsFixedAtBuild] >>>> =C2=A0=C2=A0=C2=A0 # @Prompt Enable UEFI Stack Guard. >>>> >>>> =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x300010= 55 >>>> >>>> >>>> >>>> +=C2=A0 ## Indicates whether the EFI memory attributes table will info= rm=20 >>>> the OS that >>>> >>>> +=C2=A0 #=C2=A0 forward edge control flow guards have been inserted in= to the=20 >>>> runtime services >>>> >>>> +=C2=A0 #=C2=A0 code regions. >>>> >>>> +=C2=A0 #=C2=A0=C2=A0 TRUE=C2=A0 - Runtime code has forward control fl= ow guards.
>>>> >>>> +=C2=A0 #=C2=A0=C2=A0 FALSE - Runtime code does not have forward contr= ol flow=20 >>>> guards.
>>>> >>>> +=C2=A0 # @Prompt Enable forward control flow guards in EFI memory=20 >>>> attributes table >>>> >>>> + =20 >>>> gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryAttributesTableForwardCfi|FALS= E|BOOLEAN|0x30001056 >>>> >>>> + >>>> >>>> =C2=A0 [PcdsFixedAtBuild, PcdsPatchableInModule] >>>> >>>> =C2=A0=C2=A0=C2=A0 ## Dynamic type PCD can be registered callback func= tion for Pcd=20 >>>> setting action. >>>> >>>> =C2=A0=C2=A0=C2=A0 #=C2=A0 PcdMaxPeiPcdCallBackNumberPerPcdEntry indic= ates the maximum=20 >>>> number of callback function >>>> >>>> --=20 >>>> 2.39.1 >>> >> >> >>=20 >>