From: "Ard Biesheuvel" <ardb@kernel.org>
To: Oliver Smith-Denny <osde@linux.microsoft.com>
Cc: devel@edk2.groups.io,
"Michael Kinney" <michael.d.kinney@intel.com>,
"Liming Gao" <gaoliming@byosoft.com.cn>,
"Jiewen Yao" <jiewen.yao@intel.com>,
"Michael Kubacki" <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>,
"Marvin Häuser" <mhaeuser@posteo.de>,
"Bob Feng" <bob.c.feng@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 17/17] MdeModulePkg: Enable forward edge CFI in mem attributes table
Date: Tue, 4 Apr 2023 12:41:26 +0200 [thread overview]
Message-ID: <CAMj1kXEK6q4-v84kh=QSaUmG1miGi3vFbQPiF=tkSwRd55iBEw@mail.gmail.com> (raw)
In-Reply-To: <cad03d93-0d71-8277-fa3c-89e17ab9c475@linux.microsoft.com>
On Mon, 3 Apr 2023 at 17:48, Oliver Smith-Denny
<osde@linux.microsoft.com> wrote:
>
> Turns out my old email was getting sent to a lot of folks spam, so
> resending with hopefully a better email...
>
> On 3/27/2023 4:01 AM, Ard Biesheuvel wrote:
> > 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.
> >
> > Given that the PE/COFF spec now defines a DllCharacteristicsEx flag that
> > indicates whether or not a loaded image is compatible with this, we can
> > wire this up to the flag in the memory attributes table, and set it if
> > all loaded runtime image are compatible with it.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> > MdeModulePkg/Core/Dxe/DxeMain.h | 2 ++
> > MdeModulePkg/Core/Dxe/Image/Image.c | 10 ++++++++++
> > MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 8 +++++++-
> > 3 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
> > index 815a6b4bd844a452..43daa037be441150 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/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
> > index 8704ebea9a7c88c0..9dbfb2a1fad22ced 100644
> > --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> > @@ -1399,6 +1399,16 @@ CoreLoadImageCommon (
> > CoreNewDebugImageInfoEntry (EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL, &Image->Info, Image->Handle);
> >
> > }
> >
> >
> >
> > + //
> >
> > + // Check whether we are loading a runtime image that lacks support for
> >
> > + // IBT/BTI landing pads.
> >
> > + //
> >
> > + if ((Image->ImageContext.ImageCodeMemoryType == EfiRuntimeServicesCode) &&
> >
> > + ((Image->ImageContext.DllCharacteristicsEx & EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT) == 0))
> >
> > + {
> >
> > + gMemoryAttributesTableForwardCfi = FALSE;
> >
> > + }
>
> If I understand this correctly, we are disabling Forward CFI if we
> attempt to load any runtime images that don't support it. Would it make
> sense to have a PCD to determine whether we strictly enforce
> Forward CFI (i.e. don't load this incompatible image) in such a case? We
> have a similar option for non-NX_COMPAT images.
>
These changes only affect what the OS sees, and if the OS wants to
implement a certain policy around this, it is free to do so. I don't
think this belongs in the firmware though,
*However*, if/when we wire up forward CFI enforcement at boot time, it
would be appropriate to have a configurable policy around this, and
reject 3rd party images that do not implement forward CFI if the
firmware is configured for strict enforcement.
I intend to look into that next, but given how tedious and painful it
is to get changes reviewed, I'm not sure this will be anytime soon.
next prev parent reply other threads:[~2023-04-04 10:41 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 11:00 [PATCH v2 00/17] Enable BTI support in memory attributes table Ard Biesheuvel
2023-03-27 11:00 ` [PATCH v2 01/17] MdePkg/ProcessorBind AARCH64: Add asm macro to emit GNU BTI note Ard Biesheuvel
2023-03-27 11:52 ` Leif Lindholm
2023-03-27 12:15 ` Ard Biesheuvel
2023-03-27 12:56 ` Leif Lindholm
2023-03-27 12:45 ` Leif Lindholm
2023-03-27 12:46 ` [edk2-devel] " Ard Biesheuvel
2023-03-27 12:49 ` Leif Lindholm
2023-03-27 14:12 ` Pedro Falcato
2023-03-27 14:24 ` Leif Lindholm
2023-03-30 7:28 ` Ard Biesheuvel
2023-03-30 10:47 ` Leif Lindholm
2023-03-27 11:00 ` [PATCH v2 02/17] MdePkg/BaseCpuLib AARCH64: Make asm files BTI compatible Ard Biesheuvel
2023-03-27 11:00 ` [PATCH v2 03/17] MdePkg/BaseIoLibIntrinsic " Ard Biesheuvel
2023-03-27 11:00 ` [PATCH v2 04/17] MdePkg/BaseLib AARCH64: Make LongJump() " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 05/17] MdePkg/BaseLib AARCH64: Make asm files " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 06/17] MdePkg/BaseMemoryLibOptDxe " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 07/17] MdePkg/BaseSynchronizationLib " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 08/17] MdePkg/BaseRngLib " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 09/17] ArmPkg: Emit BTI opcodes when BTI codegen is enabled Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 10/17] ArmPkg/GccLto AARCH64: Add BTI note to LTO helper library Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 11/17] ArmPkg, BaseTools AARCH64: Add BTI ELF note to .hii objects Ard Biesheuvel
2023-03-27 13:09 ` Leif Lindholm
2023-03-27 13:16 ` [edk2-devel] " Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 12/17] ArmPlatformPkg/PrePeiCore: Make vector table object BTI compatible Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 13/17] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 14/17] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Ard Biesheuvel
2023-03-27 15:46 ` Marvin Häuser
2023-03-27 16:41 ` Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 15/17] MdePkg: Update MemoryAttributesTable to v2.10 Ard Biesheuvel
2023-03-27 13:29 ` Leif Lindholm
2023-03-29 16:47 ` [edk2-devel] " Ard Biesheuvel
2023-03-29 18:07 ` Leif Lindholm
2023-03-27 11:01 ` [PATCH v2 16/17] MdePkg/PeCoffLib: Capture DLL characteristics fieldis in image context Ard Biesheuvel
2023-03-30 14:44 ` [edk2-devel] " Michael Kubacki
2023-03-30 14:53 ` Ard Biesheuvel
2023-03-27 11:01 ` [PATCH v2 17/17] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard Biesheuvel
2023-03-28 22:59 ` [edk2-devel] " Oliver Smith-Denny
2023-04-03 15:48 ` osde
2023-04-04 10:41 ` Ard Biesheuvel [this message]
2023-04-04 15:00 ` Oliver Smith-Denny
2023-04-04 15:30 ` Ard Biesheuvel
2023-03-27 11:43 ` [PATCH v2 00/17] Enable BTI support in memory " Leif Lindholm
2023-03-27 12:54 ` [edk2-devel] " Ard Biesheuvel
2023-03-27 13:37 ` Gerd Hoffmann
2023-03-28 23:00 ` Oliver Smith-Denny
2023-03-29 16:31 ` Leif Lindholm
2023-03-30 7:41 ` [edk2-devel] " Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMj1kXEK6q4-v84kh=QSaUmG1miGi3vFbQPiF=tkSwRd55iBEw@mail.gmail.com' \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox