[RFC PATCH v2 0/7] enable IBT/BTI codegen and reporting to the OS

[RFC PATCH v2 0/7] enable IBT/BTI codegen and reporting to the OS

[Ard Biesheuvel]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2373 bytes --]

v2:
- check instruction opcode at image entry point to decide whether the
  image was built with forward edge control flow guards
- add some cleanup patches for BasePeCoffLib while at it

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Rebecca Cran <quic_rcran@quicinc.com>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Taylor Beebe <t@taylorbeebe.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>

Ard Biesheuvel (7):
  MdePkg: Update MemoryAttributesTable to v2.10
  MdePkg/BasePeCoffLib: Move RISC-V definitions out of generic header
  MdePkg/BasePeCoffLib: Clean up stale Itanium references in comments
  MdePkg/BasePeCoffLib: Add generic plumbing to detect IBT/BTI support
  MdePkg/BasePeCoffLib AARCH64: Implement fwd control flow guard
    detection
  MdeModulePkg: Enable forward edge CFI in mem attributes table
  ArmVirtPkg: Implement BTI for runtime regions

 ArmVirtPkg/ArmVirt.dsc.inc                                  |  6 ++++
 MdeModulePkg/Core/Dxe/DxeMain.h                             |  2 ++
 MdeModulePkg/Core/Dxe/Image/Image.c                         |  9 +++++
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c          |  8 ++++-
 MdePkg/Include/Guid/MemoryAttributesTable.h                 |  8 +++--
 MdePkg/Include/Library/PeCoffLib.h                          |  5 +++
 MdePkg/Library/BasePeCoffLib/{ => AArch64}/PeCoffLoaderEx.c | 36 +++++++++++++++-----
 MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c           | 16 +++++++++
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c                   |  9 +++--
 MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf              |  5 ++-
 MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h       | 30 ++++++++--------
 MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c     | 16 +++++++++
 MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c               | 27 +++++++++++----
 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c         | 25 ++++++++++++++
 14 files changed, 167 insertions(+), 35 deletions(-)
 copy MdePkg/Library/BasePeCoffLib/{ => AArch64}/PeCoffLoaderEx.c (64%)

-- 
2.39.1

[RFC PATCH v2 1/7] MdePkg: Update MemoryAttributesTable to v2.10

[Ard Biesheuvel]

UEFI v2.10 introduces a new flag to the memory attributes table to
inform the OS whether or not runtime services code regions were emitted
by the compiler with guard instructions for forward edge control flow
integrity enforcement.

So update our definition accordingly.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
Acked-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 2 +-
 MdePkg/Include/Guid/MemoryAttributesTable.h        | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index e07921371187..82fa026bceb9 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -182,7 +182,7 @@ InstallMemoryAttributesTable (
   MemoryAttributesTable->Version         = EFI_MEMORY_ATTRIBUTES_TABLE_VERSION;
   MemoryAttributesTable->NumberOfEntries = RuntimeEntryCount;
   MemoryAttributesTable->DescriptorSize  = (UINT32)DescriptorSize;
-  MemoryAttributesTable->Reserved        = 0;
+  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/MdePkg/Include/Guid/MemoryAttributesTable.h b/MdePkg/Include/Guid/MemoryAttributesTable.h
index 82f83a67b96d..238c14ff92df 100644
--- a/MdePkg/Include/Guid/MemoryAttributesTable.h
+++ b/MdePkg/Include/Guid/MemoryAttributesTable.h
@@ -17,11 +17,15 @@ typedef struct {
   UINT32    Version;
   UINT32    NumberOfEntries;
   UINT32    DescriptorSize;
-  UINT32    Reserved;
+  UINT32    Flags;
   // EFI_MEMORY_DESCRIPTOR Entry[1];
 } EFI_MEMORY_ATTRIBUTES_TABLE;
 
-#define EFI_MEMORY_ATTRIBUTES_TABLE_VERSION  0x00000001
+#define EFI_MEMORY_ATTRIBUTES_TABLE_VERSION  0x00000002
+
+#define EFI_MEMORY_ATTRIBUTES_FLAGS_RT_FORWARD_CONTROL_FLOW_GUARD  0x1
+// BIT0 implies that Runtime code includes the forward control flow guard
+// instruction, such as X86 CET-IBT or ARM BTI.
 
 extern EFI_GUID  gEfiMemoryAttributesTableGuid;
 
-- 
2.39.1

[RFC PATCH v2 2/7] MdePkg/BasePeCoffLib: Move RISC-V definitions out of generic header

[Ard Biesheuvel]

Generic headers should only contain arch specific definitions if really
needed, which is not the case for the RISC-V based opcode immediate
parsing constants, so move them out of BasePeCoffLibInternals.h.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h | 9 ---------
 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c   | 9 +++++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
index aa86a54850c6..4a43ec236529 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
@@ -17,15 +17,6 @@
 #include <Library/PeCoffExtraActionLib.h>
 #include <IndustryStandard/PeImage.h>
 
-//
-// Macro definitions for RISC-V architecture.
-//
-#define RV_X(x, s, n)  (((x) >> (s)) & ((1<<(n))-1))
-#define RISCV_IMM_BITS   12
-#define RISCV_IMM_REACH  (1LL<<RISCV_IMM_BITS)
-#define RISCV_CONST_HIGH_PART(VALUE) \
-  (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1))
-
 /**
   Performs an Itanium-based specific relocation fixup and is a no-op on other
   instruction sets.
diff --git a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
index adbfe9ccf580..71daf7fe4554 100644
--- a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
@@ -7,6 +7,15 @@
 #include "BasePeCoffLibInternals.h"
 #include <Library/BaseLib.h>
 
+//
+// Macro definitions for RISC-V architecture.
+//
+#define RV_X(x, s, n)  (((x) >> (s)) & ((1<<(n))-1))
+#define RISCV_IMM_BITS   12
+#define RISCV_IMM_REACH  (1LL<<RISCV_IMM_BITS)
+#define RISCV_CONST_HIGH_PART(VALUE) \
+  (((VALUE) + (RISCV_IMM_REACH/2)) & ~(RISCV_IMM_REACH-1))
+
 /**
   Performs an RISC-V specific relocation fixup and is a no-op on
   other instruction sets.
-- 
2.39.1

[RFC PATCH v2 3/7] MdePkg/BasePeCoffLib: Clean up stale Itanium references in comments

[Ard Biesheuvel]

The specialized relocation fixup handlers are used by ARM and RISC-V but
not by IPF/Itanium anymore, so let's clean up the comments referring to
Itanium. No code changes.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  2 +-
 MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h |  8 +++-----
 MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c         | 11 +++++------
 3 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 97a8aaf8c73d..85ada399e303 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -1901,7 +1901,7 @@ PeCoffLoaderRelocateImageForRuntime (
 
           default:
             //
-            // Only Itanium requires ConvertPeImage_Ex
+            // Perform architecture/ISA specific relocation.
             //
             Status = PeHotRelocateImageEx (Reloc, Fixup, &FixupData, Adjust);
             if (RETURN_ERROR (Status)) {
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
index 4a43ec236529..a29a6febe98f 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
@@ -18,8 +18,7 @@
 #include <IndustryStandard/PeImage.h>
 
 /**
-  Performs an Itanium-based specific relocation fixup and is a no-op on other
-  instruction sets.
+  Performs an architecture/ISA specific relocation fixup.
 
   @param  Reloc       The pointer to the relocation record.
   @param  Fixup       The pointer to the address to fix up.
@@ -38,9 +37,8 @@ PeCoffLoaderRelocateImageEx (
   );
 
 /**
-  Performs an Itanium-based specific re-relocation fixup and is a no-op on other
-  instruction sets. This is used to re-relocated the image into the EFI virtual
-  space for runtime calls.
+  Performs an architecture/ISA specific re-relocation fixup. This is used to
+  re-relocate the image into the EFI virtual space for runtime calls.
 
   @param  Reloc       The pointer to the relocation record.
   @param  Fixup       The pointer to the address to fix up.
diff --git a/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
index 1a806dd62db6..f7cade4d7d4e 100644
--- a/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
@@ -1,5 +1,6 @@
 /** @file
-  Specific relocation fixups for none Itanium architecture.
+  NULL implementations of architecture/ISA Specific relocation handlers,
+  for architectures/ISAs that only use the generic PE/COFF relocation types.
 
   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -9,8 +10,7 @@
 #include "BasePeCoffLibInternals.h"
 
 /**
-  Performs an Itanium-based specific relocation fixup and is a no-op on other
-  instruction sets.
+  Performs an architecture/ISA specific relocation fixup.
 
   @param  Reloc       The pointer to the relocation record.
   @param  Fixup       The pointer to the address to fix up.
@@ -59,9 +59,8 @@ PeCoffLoaderImageFormatSupported (
 }
 
 /**
-  Performs an Itanium-based specific re-relocation fixup and is a no-op on other
-  instruction sets. This is used to re-relocated the image into the EFI virtual
-  space for runtime calls.
+  Performs an architecture/ISA specific re-relocation fixup. This is used to
+  re-relocate the image into the EFI virtual space for runtime calls.
 
   @param  Reloc       The pointer to the relocation record.
   @param  Fixup       The pointer to the address to fix up.
-- 
2.39.1

[RFC PATCH v2 4/7] MdePkg/BasePeCoffLib: Add generic plumbing to detect IBT/BTI support

[Ard Biesheuvel]

Add an internal helper that detects whether or not a loaded PE/COFF
image was built with support for forward edge control flow guards.

The default implementation will return FALSE, architectures can
specialize this based on arch specific criteria.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdePkg/Include/Library/PeCoffLib.h                      |  5 +++++
 MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c       | 16 ++++++++++++++++
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c               |  7 +++++--
 MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h   | 13 +++++++++++++
 MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c | 16 ++++++++++++++++
 MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c           | 16 ++++++++++++++++
 MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c     | 16 ++++++++++++++++
 7 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/PeCoffLib.h b/MdePkg/Include/Library/PeCoffLib.h
index b45879453785..98988e566001 100644
--- a/MdePkg/Include/Library/PeCoffLib.h
+++ b/MdePkg/Include/Library/PeCoffLib.h
@@ -182,6 +182,11 @@ typedef struct {
   ///
   BOOLEAN                     IsTeImage;
   ///
+  /// Set by PeCoffLoaderGetImageInfo() to TRUE if the image's entrypoint has
+  /// a forward control flow guard instruction, such as ENDBR on X86 for IBT.
+  ///
+  BOOLEAN                     HasForwardControlFlowGuards;
+  ///
   /// Set by PeCoffLoaderLoadImage() to the HII resource offset
   /// if the image contains a custom PE/COFF resource with the type 'HII'.
   /// Otherwise, the entry remains to be 0.
diff --git a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
index 595377bed661..82d9f548ca54 100644
--- a/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/Arm/PeCoffLoaderEx.c
@@ -234,3 +234,19 @@ PeHotRelocateImageEx (
 
   return RETURN_SUCCESS;
 }
+
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  )
+{
+  return FALSE;
+}
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 85ada399e303..8886b3d3feff 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -545,8 +545,9 @@ PeCoffLoaderGetPeHeader (
   Retrieves information about a PE/COFF image.
 
   Computes the PeCoffHeaderOffset, IsTeImage, ImageType, ImageAddress, ImageSize,
-  DestinationAddress, RelocationsStripped, SectionAlignment, SizeOfHeaders, and
-  DebugDirectoryEntryRva fields of the ImageContext structure.
+  DestinationAddress, RelocationsStripped, SectionAlignment, SizeOfHeaders,
+  DebugDirectoryEntryRva and HasForwardControlFlowGuards fields of the
+  ImageContext structure.
   If ImageContext is NULL, then return RETURN_INVALID_PARAMETER.
   If the PE/COFF image accessed through the ImageRead service in the ImageContext
   structure is not a supported PE/COFF image type, then return RETURN_UNSUPPORTED.
@@ -1429,6 +1430,8 @@ PeCoffLoaderLoadImage (
                                                           );
   }
 
+  ImageContext->HasForwardControlFlowGuards = PeCoffLoaderCheckForwardControlFlowGuards (ImageContext);
+
   //
   // Determine the size of the fixup data
   //
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
index a29a6febe98f..3bf1b7f535fd 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoffLibInternals.h
@@ -119,4 +119,17 @@ PeCoffLoaderImageAddress (
   IN     UINTN                         TeStrippedOffset
   );
 
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  );
+
 #endif
diff --git a/MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c
index 417096f33493..b3d01f0a4be9 100644
--- a/MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/LoongArch/PeCoffLoaderEx.c
@@ -135,3 +135,19 @@ PeHotRelocateImageEx (
   // To check
   return PeCoffLoaderRelocateImageEx (Reloc, Fixup, FixupData, Adjust);
 }
+
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  )
+{
+  return FALSE;
+}
diff --git a/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
index f7cade4d7d4e..43f346e0aadb 100644
--- a/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/PeCoffLoaderEx.c
@@ -80,3 +80,19 @@ PeHotRelocateImageEx (
 {
   return RETURN_UNSUPPORTED;
 }
+
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  )
+{
+  return FALSE;
+}
diff --git a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
index 71daf7fe4554..88dc9bd9b89e 100644
--- a/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
+++ b/MdePkg/Library/BasePeCoffLib/RiscV/PeCoffLoaderEx.c
@@ -143,3 +143,19 @@ PeHotRelocateImageEx (
 {
   return RETURN_UNSUPPORTED;
 }
+
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  )
+{
+  return FALSE;
+}
-- 
2.39.1

[RFC PATCH v2 5/7] MdePkg/BasePeCoffLib AARCH64: Implement fwd control flow guard detection

[Ard Biesheuvel]

Add the check to infer from the instruction opcode at the image
entrypoint whether or not forward edge control flow guards were emitted
by the compiler at build time. Given that an image entry point is
invoked indirectly by construction, its entrypoint must have such a
guard instruction there if it implements support for forward edge
control flow enforcement such as IBT or BTI.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdePkg/Library/BasePeCoffLib/AArch64/PeCoffLoaderEx.c | 103 ++++++++++++++++++++
 MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf        |   5 +-
 2 files changed, 107 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BasePeCoffLib/AArch64/PeCoffLoaderEx.c b/MdePkg/Library/BasePeCoffLib/AArch64/PeCoffLoaderEx.c
new file mode 100644
index 000000000000..d25b11109dbc
--- /dev/null
+++ b/MdePkg/Library/BasePeCoffLib/AArch64/PeCoffLoaderEx.c
@@ -0,0 +1,103 @@
+/** @file
+  AArch64 implementations of architecture/ISA Specific relocation handlers.
+
+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2023, Google LLC. Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "BasePeCoffLibInternals.h"
+
+/**
+  Performs an architecture/ISA specific relocation fixup.
+
+  @param  Reloc       The pointer to the relocation record.
+  @param  Fixup       The pointer to the address to fix up.
+  @param  FixupData   The pointer to a buffer to log the fixups.
+  @param  Adjust      The offset to adjust the fixup.
+
+  @return Status code.
+
+**/
+RETURN_STATUS
+PeCoffLoaderRelocateImageEx (
+  IN UINT16     *Reloc,
+  IN OUT CHAR8  *Fixup,
+  IN OUT CHAR8  **FixupData,
+  IN UINT64     Adjust
+  )
+{
+  return RETURN_UNSUPPORTED;
+}
+
+/**
+  Returns TRUE if the machine type of PE/COFF image is supported. Supported
+  does not mean the image can be executed it means the PE/COFF loader supports
+  loading and relocating of the image type. It's up to the caller to support
+  the entry point.
+
+  @param  Machine   The machine type from the PE Header.
+
+  @return TRUE if this PE/COFF loader can load the image
+
+**/
+BOOLEAN
+PeCoffLoaderImageFormatSupported (
+  IN  UINT16  Machine
+  )
+{
+  if ((Machine == IMAGE_FILE_MACHINE_I386) || (Machine == IMAGE_FILE_MACHINE_X64) ||
+      (Machine == IMAGE_FILE_MACHINE_EBC) || (Machine == IMAGE_FILE_MACHINE_ARM64))
+  {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+/**
+  Performs an architecture/ISA specific re-relocation fixup. This is used to
+  re-relocate the image into the EFI virtual space for runtime calls.
+
+  @param  Reloc       The pointer to the relocation record.
+  @param  Fixup       The pointer to the address to fix up.
+  @param  FixupData   The pointer to a buffer to log the fixups.
+  @param  Adjust      The offset to adjust the fixup.
+
+  @return Status code.
+
+**/
+RETURN_STATUS
+PeHotRelocateImageEx (
+  IN UINT16     *Reloc,
+  IN OUT CHAR8  *Fixup,
+  IN OUT CHAR8  **FixupData,
+  IN UINT64     Adjust
+  )
+{
+  return RETURN_UNSUPPORTED;
+}
+
+/**
+  Returns whether the image implements forward control flow guards.
+
+  @param  ImageContext      The context of the image being loaded.
+
+  @return TRUE if the image implements forward control flow guards
+
+**/
+BOOLEAN
+PeCoffLoaderCheckForwardControlFlowGuards (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
+  )
+{
+  CONST UINT32  *Opcode;
+
+  Opcode = (UINT32 *)(UINTN)ImageContext->EntryPoint;
+
+  //
+  // Check whether the opcode is BTI C or BTI CJ
+  //
+  return ((*Opcode & 0xffffff7f) == 0xd503245f);
+}
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf b/MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
index 3b8b8eb1917d..8b720bd6e006 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
@@ -39,12 +39,15 @@ [Sources]
   BasePeCoffLibInternals.h
   BasePeCoff.c
 
-[Sources.IA32, Sources.X64, Sources.EBC, Sources.AARCH64]
+[Sources.IA32, Sources.X64, Sources.EBC]
   PeCoffLoaderEx.c
 
 [Sources.ARM]
   Arm/PeCoffLoaderEx.c
 
+[Sources.AARCH64]
+  AArch64/PeCoffLoaderEx.c
+
 [Sources.RISCV64]
   RiscV/PeCoffLoaderEx.c
 
-- 
2.39.1

[RFC PATCH v2 6/7] MdeModulePkg: Enable forward edge CFI in mem attributes table

[Ard Biesheuvel]

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.

The PE/COFF loader will now keep track of whether loaded images are
constructed in the expected manner, so set the new flag if all loaded
runtime images were constructed with forward edge control flow guards.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 MdeModulePkg/Core/Dxe/DxeMain.h                    | 2 ++
 MdeModulePkg/Core/Dxe/Image/Image.c                | 9 +++++++++
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 8 +++++++-
 3 files changed, 18 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/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 06cc6744b8c6..eac0b859a7f7 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -1398,6 +1398,15 @@ CoreLoadImageCommon (
     CoreNewDebugImageInfoEntry (EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL, &Image->Info, Image->Handle);
   }
 
+  //
+  // If we loaded a runtime DXE driver, take into account whether or not it was built
+  // with forward edge control flow guards. We can only expose support for forward edge
+  // control flow to the OS if all loaded runtime images support it.
+  //
+  if (Image->ImageContext.ImageCodeMemoryType == EfiRuntimeServicesCode) {
+    gMemoryAttributesTableForwardCfi &= Image->ImageContext.HasForwardControlFlowGuards;
+  }
+
   //
   // 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 82fa026bceb9..d6983f830452 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  = TRUE;
 
 /**
   Install MemoryAttributesTable.
@@ -182,11 +183,16 @@ InstallMemoryAttributesTable (
   MemoryAttributesTable->Version         = EFI_MEMORY_ATTRIBUTES_TABLE_VERSION;
   MemoryAttributesTable->NumberOfEntries = RuntimeEntryCount;
   MemoryAttributesTable->DescriptorSize  = (UINT32)DescriptorSize;
-  MemoryAttributesTable->Flags           = 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));
   DEBUG ((DEBUG_VERBOSE, "  DescriptorSize       - 0x%08x\n", MemoryAttributesTable->DescriptorSize));
+  DEBUG ((DEBUG_VERBOSE, "  Flags                - 0x%08x\n", MemoryAttributesTable->Flags));
   MemoryAttributesEntry = (EFI_MEMORY_DESCRIPTOR *)(MemoryAttributesTable + 1);
   MemoryMap             = MemoryMapStart;
   for (Index = 0; Index < MemoryMapSize/DescriptorSize; Index++) {
-- 
2.39.1

[RFC PATCH v2 7/7] ArmVirtPkg: Implement BTI for runtime regions

[Ard Biesheuvel]

Add a build option RUNTIME_BTI_ENABLE, and wire it up to the command
line options passed to the compiler to get it to emit BTI landing pads
into all modules. Note that runtime DXE modules may incorporate
libraries of type BASE, UEFI_DRIVER or DXE_DRIVER, so the only safe
option here is to apply the command line option to all types.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 74d98e6314c4..9cb37f3d46a3 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -10,6 +10,7 @@
 
 [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x8000004F
+  DEFINE RUNTIME_BTI_ENABLE      = FALSE
 
 !if $(TARGET) != NOOPT
   DEFINE FD_SIZE_IN_MB    = 2
@@ -33,6 +34,11 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
 
+[BuildOptions]
+!if $(RUNTIME_BTI_ENABLE) == TRUE
+  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
+!endif
+
 [LibraryClasses.common]
 !if $(TARGET) == RELEASE
   DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
-- 
2.39.1

Re: [edk2-devel] [RFC PATCH v2 7/7] ArmVirtPkg: Implement BTI for runtime regions

[Michael Brown]

On 03/02/2023 12:10, Ard Biesheuvel wrote:
> +[BuildOptions]
> +!if $(RUNTIME_BTI_ENABLE) == TRUE
> +  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
> +!endif

Question: as a producer of externally loaded UEFI binaries (e.g. 
ipxe.efi): what would I need to do to take advantage of BTI?

I'm assuming:

- enable -mbranch-protection=bti in my builds (easy)

- wait for PE/COFF specification change and then update my produced 
images to include whatever flag gets decided upon.

Is that correct?

Thanks,

Michael

Re: [edk2-devel] [RFC PATCH v2 7/7] ArmVirtPkg: Implement BTI for runtime regions

[Ard Biesheuvel]

On Fri, 3 Feb 2023 at 13:33, Michael Brown <mcb30@ipxe.org> wrote:
>
> On 03/02/2023 12:10, Ard Biesheuvel wrote:
> > +[BuildOptions]
> > +!if $(RUNTIME_BTI_ENABLE) == TRUE
> > +  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
> > +!endif
>
> Question: as a producer of externally loaded UEFI binaries (e.g.
> ipxe.efi): what would I need to do to take advantage of BTI?
>
> I'm assuming:
>
> - enable -mbranch-protection=bti in my builds (easy)
>
> - wait for PE/COFF specification change and then update my produced
> images to include whatever flag gets decided upon.
>
> Is that correct?
>

First of all, in case you missed this, the series in question only
covers runtime DXE drivers, i.e., the code that persists after
ExitBootServices() and gets mapped by the OS and called to access the
variable store. So iPXE should not be affected at all by these
changes.

So building your code with branch protection enabled is not going to
have any benefit until we decide how to deploy this at boot time, and
I think the conclusion on this thread is already that the only
meaningful way to do this is to introduce a PE/COFF image flag that
indicates whether or not a certain image was built with indirect
branch protection.

I could also imagine that, at boot time, it would be feasible to apply
these protections at image granularity, rather than as a global
switch, given that (at least on ARM) these mitigations can be enabled
on a per-page basis, and there is no need to turn it off completely
when, say, the GOP driver on the video card does not support it.

I am not aware of any discussion around this, though - I hope we can
get the right folks at MS involved to drive the PE/COFF side of this
and then, I am more than happy to take (joint) ownership of this on
the Tianocore side, and hammer something out that works for everyone.

Jiewen, Mike; could you comment on the IBT side? Does x86 permit IBT
enforcement on a per-page basis as well? Could we feasibly add this to
the code/data rx/rw remapping code, to enable indirect branch
protection as each image is loaded by the DXE core?

So to answer your question: yes.

Re: [edk2-devel] [RFC PATCH v2 7/7] ArmVirtPkg: Implement BTI for runtime regions

[Michael Brown]

On 03/02/2023 12:55, Ard Biesheuvel wrote:
>> Question: as a producer of externally loaded UEFI binaries (e.g.
>> ipxe.efi): what would I need to do to take advantage of BTI?
>>
>> I'm assuming:
>>
>> - enable -mbranch-protection=bti in my builds (easy)
>>
>> - wait for PE/COFF specification change and then update my produced
>> images to include whatever flag gets decided upon.
>>
>> Is that correct?
> 
> First of all, in case you missed this, the series in question only
> covers runtime DXE drivers, i.e., the code that persists after
> ExitBootServices() and gets mapped by the OS and called to access the
> variable store. So iPXE should not be affected at all by these
> changes.

I was not paying close attention to this patch series and had missed 
that detail: thank you for clarifying.

> So to answer your question: yes.

Thank you!

Michael