On Tue, 4 Apr 2023 at 17:00, Oliver Smith-Denny
<o...@linux.microsoft.com> wrote:
>
> On 4/4/2023 3:41 AM, Ard Biesheuvel wrote:
> > On Mon, 3 Apr 2023 at 17:48, Oliver Smith-Denny
> > <o...@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 <a...@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.
>
> I see, thanks for the explanation, makes sense to me.
>
> For the patchset (sorry, with the email change I don't have the top
> level entry): Reviewed-by: Oliver Smith-Denny <o...@linux.microsoft.com>
>
Thanks!
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102490): https://edk2.groups.io/g/devel/message/102490
Mute This Topic: https://groups.io/mt/97879305/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
