public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Enable BTI support in memory attributes table
@ 2023-04-04 15:40 Ard Biesheuvel
  2023-04-04 15:40 ` [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 15:40 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
	Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
	Sami Mujawar, Taylor Beebe, Marvin Häuser, Bob Feng,
	Oliver Smith-Denny

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

Implement version 2 of the memory attributes table, which now contains a
flag informing the OS whether or not code regions may be mapped with CFI
mitigations such as IBT or BTI enabled.

This series covers the remaining parts after the AArch64 specific
changes were merged:

- Update the BaseTools to emit the appropriate PE/COFF annotation when a
  BTI/IBT compatible ELF executable is converted to PE/COFF
- Take this PE/COFF annotation into account when populating the memory
  attributes table in the DXE core

TODO:
- X64 changes to make the code IBT compatible and emit the ELF note
- Figure out how to generate such executables with native PE toolchains
- Implement BTI/IBT enforcement at boot time - this is something I
  intend to look into next.

Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
linker, version 3.8 or newer) with the following build options.

[BuildOptions]
  GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
  GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld -Wl,--no-relax,--no-pie,-z,bti-report=error

Changes since v2:
- increase DllCharacteristicsEx field to 4 bytes
- add Oliver's Rb

If no comments or objections have been raised by the end of the week, I
will go ahead and merge this - thanks.

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>
Cc: Bob Feng <bob.c.feng@intel.com>
Cc: Oliver Smith-Denny <osde@linux.microsoft.com>

Ard Biesheuvel (4):
  BaseTools/GenFw: Parse IBT/BTI support status from ELF note
  BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
  MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
  MdeModulePkg: Enable forward edge CFI in mem attributes table

 BaseTools/Source/C/GenFw/Elf64Convert.c               | 104 +++++++++++++++++---
 BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
 BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
 BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
 MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
 MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
 MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
 MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46 ++++++---
 10 files changed, 186 insertions(+), 28 deletions(-)

-- 
2.39.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
@ 2023-04-04 15:40 ` Ard Biesheuvel
  2023-04-05  1:57   ` [edk2-devel] " Michael Kubacki
  2023-04-04 15:40 ` [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Ard Biesheuvel
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 15:40 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
	Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
	Sami Mujawar, Taylor Beebe, Marvin Häuser, Bob Feng,
	Oliver Smith-Denny

When performing ELF to PE/COFF conversion, parse any notes sections to
decide whether the image supports forward CFI landing pads. This will be
used to set the associated DllCharacteristicsEx flag in a subsequent
patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
 BaseTools/Source/C/GenFw/Elf64Convert.c | 50 ++++++++++++++++++++
 BaseTools/Source/C/GenFw/elf_common.h   |  9 ++++
 2 files changed, 59 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 8b50774beb1eebda..2a810e835d4a4a66 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -770,6 +770,49 @@ WriteSectionRiscV64 (
   }
 }
 
+STATIC UINT16 mDllCharacteristicsEx;
+
+STATIC
+VOID
+ParseNoteSection (
+  CONST Elf_Shdr  *Shdr
+  )
+{
+  CONST Elf_Note *Note;
+  CONST UINT32   *Prop;
+  UINT32         Prop0;
+  UINT32         Prop2;
+
+  Note = (Elf_Note *)((UINT8 *)mEhdr + Shdr->sh_offset);
+
+  if ((Note->n_type == NT_GNU_PROPERTY_TYPE_0) &&
+      (Note->n_namesz == sizeof ("GNU")) &&
+      (strcmp ((CHAR8 *)(Note + 1), "GNU") == 0) &&
+      (Note->n_descsz > sizeof (UINT32[2]))) {
+    Prop = (UINT32 *)((UINT8 *)(Note + 1) + sizeof("GNU"));
+
+    switch (mEhdr->e_machine) {
+    case EM_AARCH64:
+      Prop0 = GNU_PROPERTY_AARCH64_FEATURE_1_AND;
+      Prop2 = GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
+      break;
+
+    case EM_X86_64:
+      Prop0 = GNU_PROPERTY_X86_FEATURE_1_AND;
+      Prop2 = GNU_PROPERTY_X86_FEATURE_1_IBT;
+      break;
+
+    default:
+      return;
+    }
+    if ((Prop[0] == Prop0) &&
+        (Prop[1] >= sizeof (UINT32)) &&
+        ((Prop[2] & Prop2) != 0)) {
+      mDllCharacteristicsEx |= EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT;
+    }
+  }
+}
+
 //
 // Elf functions interface implementation
 //
@@ -826,6 +869,13 @@ ScanSections64 (
     }
   }
 
+  for (i = 0; i < mEhdr->e_shnum; i++) {
+    Elf_Shdr *shdr = GetShdrByIndex(i);
+    if (shdr->sh_type == SHT_NOTE) {
+      ParseNoteSection (shdr);
+    }
+  }
+
   //
   // Check if mCoffAlignment is larger than MAX_COFF_ALIGNMENT
   //
diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
index 7b7fdeb3290dfa88..ccd32804b090a226 100644
--- a/BaseTools/Source/C/GenFw/elf_common.h
+++ b/BaseTools/Source/C/GenFw/elf_common.h
@@ -59,6 +59,15 @@ typedef struct {
   UINT32  n_type;    /* Type of this note. */
 } Elf_Note;
 
+#define NT_GNU_PROPERTY_TYPE_0  5
+
+#define GNU_PROPERTY_X86_FEATURE_1_AND  0xc0000002
+#define GNU_PROPERTY_X86_FEATURE_1_IBT  0x1
+
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND  0xc0000000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI  0x1
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC  0x2
+
 /* Indexes into the e_ident array.  Keep synced with
    http://www.sco.com/developers/gabi/latest/ch4.eheader.html */
 #define EI_MAG0    0  /* Magic number, byte 0. */
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
  2023-04-04 15:40 ` [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
@ 2023-04-04 15:40 ` Ard Biesheuvel
  2023-04-05  1:58   ` [edk2-devel] " Michael Kubacki
  2023-04-04 15:40 ` [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context Ard Biesheuvel
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 15:40 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
	Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
	Sami Mujawar, Taylor Beebe, Marvin Häuser, Bob Feng,
	Oliver Smith-Denny

The PE/COFF spec describes an additional DllCharacteristics field
implemented as a debug directory entry, which carries flags related to
which control flow integrity (CFI) features are supported by the binary.

So let's add this entry when doing ELF to PE/COFF conversion - we will
add support for setting the flags in a subsequent patch.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
 BaseTools/Source/C/GenFw/Elf64Convert.c               | 54 +++++++++++++++-----
 BaseTools/Source/C/GenFw/GenFw.c                      |  3 +-
 BaseTools/Source/C/Include/IndustryStandard/PeImage.h | 13 ++++-
 3 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 2a810e835d4a4a66..9c17c90b16602421 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -992,6 +992,16 @@ ScanSections64 (
                 sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
                 strlen(mInImageName) + 1;
 
+  //
+  // Add more space in the .debug data region for the DllCharacteristicsEx
+  // field.
+  //
+  if (mDllCharacteristicsEx != 0) {
+    mCoffOffset = DebugRvaAlign(mCoffOffset) +
+                  sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
+                  sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+  }
+
   mCoffOffset = CoffAlign(mCoffOffset);
   if (SectionCount == 0) {
     mDataOffset = mCoffOffset;
@@ -2244,29 +2254,47 @@ WriteDebug64 (
   VOID
   )
 {
-  UINT32                              Len;
-  EFI_IMAGE_OPTIONAL_HEADER_UNION     *NtHdr;
-  EFI_IMAGE_DATA_DIRECTORY            *DataDir;
-  EFI_IMAGE_DEBUG_DIRECTORY_ENTRY     *Dir;
-  EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY *Nb10;
+  UINT32                                      Len;
+  EFI_IMAGE_OPTIONAL_HEADER_UNION             *NtHdr;
+  EFI_IMAGE_DATA_DIRECTORY                    *DataDir;
+  EFI_IMAGE_DEBUG_DIRECTORY_ENTRY             *Dir;
+  EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY         *Nb10;
+  EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY *DllEntry;
 
   Len = strlen(mInImageName) + 1;
 
+  NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset);
+  DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG];
+  DataDir->VirtualAddress = mDebugOffset;
+  DataDir->Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+
   Dir = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
+
+  if (mDllCharacteristicsEx != 0) {
+    DataDir->Size  += sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+
+    Dir->Type       = EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS;
+    Dir->SizeOfData = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+    Dir->FileOffset = mDebugOffset + DataDir->Size +
+                      sizeof (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
+                      DebugRvaAlign(Len);
+    Dir->RVA        = Dir->FileOffset;
+
+    DllEntry = (VOID *)(mCoffFile + Dir->FileOffset);
+
+    DllEntry->DllCharacteristicsEx = mDllCharacteristicsEx;
+
+    Dir++;
+  }
+
   Dir->Type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
   Dir->SizeOfData = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
-  Dir->RVA = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
-  Dir->FileOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+  Dir->RVA = mDebugOffset + DataDir->Size;
+  Dir->FileOffset = mDebugOffset + DataDir->Size;
 
   Nb10 = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1);
   Nb10->Signature = CODEVIEW_SIGNATURE_NB10;
   strcpy ((char *)(Nb10 + 1), mInImageName);
-
-
-  NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset);
-  DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG];
-  DataDir->VirtualAddress = mDebugOffset;
-  DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
 }
 
 STATIC
diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 6f61f16788cd2a0a..d0e52ccc26431380 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2932,7 +2932,8 @@ Routine Description:
       if (mIsConvertXip) {
         DebugEntry->FileOffset = DebugEntry->RVA;
       }
-      if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
+      if ((ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) &&
+          (DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS)) {
         memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData);
         memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY));
       }
diff --git a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
index 77ded3f611398278..22161edf443d0e22 100644
--- a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
+++ b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
@@ -615,7 +615,8 @@ typedef struct {
 ///
 /// Debug Format
 ///
-#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW 2
+#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW               2
+#define EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS  20
 
 typedef struct {
   UINT32  Characteristics;
@@ -664,6 +665,16 @@ typedef struct {
   //
 } EFI_IMAGE_DEBUG_CODEVIEW_MTOC_ENTRY;
 
+///
+/// Extended DLL Characteristics
+///
+#define EFI_IMAGE_DLLCHARACTERISTICS_EX_CET_COMPAT          0x0001
+#define EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT  0x0040
+
+typedef struct {
+  UINT32  DllCharacteristicsEx;
+} EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY;
+
 //
 // .pdata entries for X64
 //
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
  2023-04-04 15:40 ` [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
  2023-04-04 15:40 ` [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Ard Biesheuvel
@ 2023-04-04 15:40 ` Ard Biesheuvel
  2023-04-05  1:58   ` [edk2-devel] " Michael Kubacki
  2023-04-04 15:40 ` [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard Biesheuvel
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 15:40 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
	Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
	Sami Mujawar, Taylor Beebe, Marvin Häuser, Bob Feng,
	Oliver Smith-Denny

When loading a PE/COFF image, capture the DLL characteristics fields of
the header into our image context structure so we can refer to them when
mapping the image.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
 MdePkg/Include/IndustryStandard/PeImage.h | 13 +++++-
 MdePkg/Include/Library/PeCoffLib.h        |  6 +++
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 46 +++++++++++++++-----
 3 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/PeImage.h b/MdePkg/Include/IndustryStandard/PeImage.h
index dd4cc25483bc4bcf..8646ff22b55faff0 100644
--- a/MdePkg/Include/IndustryStandard/PeImage.h
+++ b/MdePkg/Include/IndustryStandard/PeImage.h
@@ -625,7 +625,8 @@ typedef struct {
   UINT32    FileOffset;  ///< The file pointer to the debug data.
 } EFI_IMAGE_DEBUG_DIRECTORY_ENTRY;
 
-#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW  2    ///< The Visual C++ debug information.
+#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW               2    ///< The Visual C++ debug information.
+#define EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS  20
 
 ///
 /// Debug Data Structure defined in Microsoft C++.
@@ -669,6 +670,16 @@ typedef struct {
   //
 } EFI_IMAGE_DEBUG_CODEVIEW_MTOC_ENTRY;
 
+///
+/// Extended DLL Characteristics
+///
+#define EFI_IMAGE_DLLCHARACTERISTICS_EX_CET_COMPAT          0x0001
+#define EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT  0x0040
+
+typedef struct {
+  UINT32    DllCharacteristicsEx;
+} EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY;
+
 ///
 /// Resource format.
 ///
diff --git a/MdePkg/Include/Library/PeCoffLib.h b/MdePkg/Include/Library/PeCoffLib.h
index b45879453785c77d..74cceb37bf39ffc6 100644
--- a/MdePkg/Include/Library/PeCoffLib.h
+++ b/MdePkg/Include/Library/PeCoffLib.h
@@ -171,6 +171,12 @@ typedef struct {
   ///
   UINT16                      ImageType;
   ///
+  /// Set by PeCoffLoaderGetImageInfo() to the DLL flags stored in the PE/COFF header and
+  /// in the DllCharacteristicsEx debug table.
+  ///
+  UINT16                      DllCharacteristics;
+  UINT32                      DllCharacteristicsEx;
+  ///
   /// Set by PeCoffLoaderGetImageInfo() to TRUE if the PE/COFF image does not contain
   /// relocation information.
   ///
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 97a8aaf8c73d3e3c..4b71176a0c7c2ed0 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -308,10 +308,11 @@ PeCoffLoaderGetPeHeader (
       //
       // Use PE32 offset
       //
-      ImageContext->ImageType        = Hdr.Pe32->OptionalHeader.Subsystem;
-      ImageContext->ImageSize        = (UINT64)Hdr.Pe32->OptionalHeader.SizeOfImage;
-      ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
-      ImageContext->SizeOfHeaders    = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
+      ImageContext->ImageType          = Hdr.Pe32->OptionalHeader.Subsystem;
+      ImageContext->ImageSize          = (UINT64)Hdr.Pe32->OptionalHeader.SizeOfImage;
+      ImageContext->SectionAlignment   = Hdr.Pe32->OptionalHeader.SectionAlignment;
+      ImageContext->SizeOfHeaders      = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
+      ImageContext->DllCharacteristics = Hdr.Pe32->OptionalHeader.DllCharacteristics;
     } else if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
       //
       // 1. Check FileHeader.NumberOfRvaAndSizes filed.
@@ -429,10 +430,11 @@ PeCoffLoaderGetPeHeader (
       //
       // Use PE32+ offset
       //
-      ImageContext->ImageType        = Hdr.Pe32Plus->OptionalHeader.Subsystem;
-      ImageContext->ImageSize        = (UINT64)Hdr.Pe32Plus->OptionalHeader.SizeOfImage;
-      ImageContext->SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
-      ImageContext->SizeOfHeaders    = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders;
+      ImageContext->ImageType          = Hdr.Pe32Plus->OptionalHeader.Subsystem;
+      ImageContext->ImageSize          = (UINT64)Hdr.Pe32Plus->OptionalHeader.SizeOfImage;
+      ImageContext->SectionAlignment   = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
+      ImageContext->SizeOfHeaders      = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders;
+      ImageContext->DllCharacteristics = Hdr.Pe32Plus->OptionalHeader.DllCharacteristics;
     } else {
       ImageContext->ImageError = IMAGE_ERROR_INVALID_MACHINE_TYPE;
       return RETURN_UNSUPPORTED;
@@ -545,8 +547,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,
+  DllCharacteristics, DllCharacteristicsEx and DebugDirectoryEntryRva 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.
@@ -752,7 +755,28 @@ PeCoffLoaderGetImageInfo (
               ImageContext->ImageSize += DebugEntry.SizeOfData;
             }
 
-            return RETURN_SUCCESS;
+            continue;
+          }
+
+          if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS) {
+            Size     = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+            ReadSize = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+            Status   = ImageContext->ImageRead (
+                                       ImageContext->Handle,
+                                       DebugEntry.FileOffset,
+                                       &Size,
+                                       &ImageContext->DllCharacteristicsEx
+                                       );
+            if (RETURN_ERROR (Status) || (Size != ReadSize)) {
+              ImageContext->ImageError = IMAGE_ERROR_IMAGE_READ;
+              if (Size != ReadSize) {
+                Status = RETURN_UNSUPPORTED;
+              }
+
+              return Status;
+            }
+
+            continue;
           }
         }
       }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2023-04-04 15:40 ` [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context Ard Biesheuvel
@ 2023-04-04 15:40 ` Ard Biesheuvel
  2023-04-05  1:59   ` [edk2-devel] " Michael Kubacki
  2023-04-04 16:19 ` [PATCH v3 0/4] Enable BTI support in memory " Marvin Häuser
  2023-04-06  1:33 ` 回复: [edk2-devel] " gaoliming
  5 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 15:40 UTC (permalink / raw)
  To: devel
  Cc: Ard Biesheuvel, Michael Kinney, Liming Gao, Jiewen Yao,
	Michael Kubacki, Sean Brogan, Rebecca Cran, Leif Lindholm,
	Sami Mujawar, Taylor Beebe, Marvin Häuser, Bob Feng,
	Oliver Smith-Denny

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>
Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
 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;
+  }
+
   //
   // 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 82fa026bceb990e5..fd127ee167e1ac9a 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,7 +183,12 @@ 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));
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2023-04-04 15:40 ` [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard Biesheuvel
@ 2023-04-04 16:19 ` Marvin Häuser
  2023-04-04 16:29   ` Ard Biesheuvel
  2023-04-06  1:33 ` 回复: [edk2-devel] " gaoliming
  5 siblings, 1 reply; 15+ messages in thread
From: Marvin Häuser @ 2023-04-04 16:19 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel, Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Bob Feng, Oliver Smith-Denny

FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>

An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).

Best regards,
Marvin

> On 4. Apr 2023, at 17:40, Ard Biesheuvel <ardb@kernel.org> wrote:
> Implement version 2 of the memory attributes table, which now contains a
> flag informing the OS whether or not code regions may be mapped with CFI
> mitigations such as IBT or BTI enabled.
> 
> This series covers the remaining parts after the AArch64 specific
> changes were merged:
> 
> - Update the BaseTools to emit the appropriate PE/COFF annotation when a
>  BTI/IBT compatible ELF executable is converted to PE/COFF
> - Take this PE/COFF annotation into account when populating the memory
>  attributes table in the DXE core
> 
> TODO:
> - X64 changes to make the code IBT compatible and emit the ELF note
> - Figure out how to generate such executables with native PE toolchains
> - Implement BTI/IBT enforcement at boot time - this is something I
>  intend to look into next.
> 
> Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
> linker, version 3.8 or newer) with the following build options.
> 
> [BuildOptions]
>  GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
>  GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
>  GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld -Wl,--no-relax,--no-pie,-z,bti-report=error
> 
> Changes since v2:
> - increase DllCharacteristicsEx field to 4 bytes
> - add Oliver's Rb
> 
> If no comments or objections have been raised by the end of the week, I
> will go ahead and merge this - thanks.
> 
> 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>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> 
> Ard Biesheuvel (4):
>  BaseTools/GenFw: Parse IBT/BTI support status from ELF note
>  BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
>  MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
>  MdeModulePkg: Enable forward edge CFI in mem attributes table
> 
> BaseTools/Source/C/GenFw/Elf64Convert.c               | 104 +++++++++++++++++---
> BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
> BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
> BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
> MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
> MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
> MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
> MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
> MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46 ++++++---
> 10 files changed, 186 insertions(+), 28 deletions(-)
> 
> -- 
> 2.39.2


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-04 16:19 ` [PATCH v3 0/4] Enable BTI support in memory " Marvin Häuser
@ 2023-04-04 16:29   ` Ard Biesheuvel
  2023-04-04 16:42     ` Marvin Häuser
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 16:29 UTC (permalink / raw)
  To: Marvin Häuser
  Cc: devel, Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Bob Feng, Oliver Smith-Denny

On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
>
> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
>

It *can* be expanded in the future. The debug directory entry includes
a size field, and once flags get defined that are not present, they
just default to unset.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-04 16:29   ` Ard Biesheuvel
@ 2023-04-04 16:42     ` Marvin Häuser
  2023-04-04 16:53       ` Ard Biesheuvel
  0 siblings, 1 reply; 15+ messages in thread
From: Marvin Häuser @ 2023-04-04 16:42 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel, Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Bob Feng, Oliver Smith-Denny


> On 4. Apr 2023, at 18:29, Ard Biesheuvel <ardb@kernel.org> wrote:
> On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
>> 
>> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
>> 
>> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
> 
> It *can* be expanded in the future. The debug directory entry includes
> a size field, and once flags get defined that are not present, they
> just default to unset.

Ugh, I should have known this… sorry for my terrible remark! :)

Though doesn’t that mean the size field should ideally be checked in 3/4 (not regarding the extensibility point, but well-formedness of the data)?

Best regards,
Marvin

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-04 16:42     ` Marvin Häuser
@ 2023-04-04 16:53       ` Ard Biesheuvel
  0 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-04 16:53 UTC (permalink / raw)
  To: Marvin Häuser
  Cc: devel, Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Bob Feng, Oliver Smith-Denny

On Tue, 4 Apr 2023 at 18:42, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
>
> > On 4. Apr 2023, at 18:29, Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Tue, 4 Apr 2023 at 18:19, Marvin Häuser <mhaeuser@posteo.de> wrote:
> >>
> >> FWIW, Reviewed-by: Marvin Häuser <mhaeuser@posteo.de>
> >>
> >> An off-topic remark, but I find it ominous that when adding a hack like the DllCharacteristicsEx debug entry, the opportunity is not used to turn it into something that can be expanded in the future without introducing yet another hack like this (I know 31 more Bits look plenty now, but if an address, offset, or size will be needed… ouch).
> >
> > It *can* be expanded in the future. The debug directory entry includes
> > a size field, and once flags get defined that are not present, they
> > just default to unset.
>
> Ugh, I should have known this… sorry for my terrible remark! :)
>
> Though doesn’t that mean the size field should ideally be checked in 3/4 (not regarding the extensibility point, but well-formedness of the data)?
>

Ideally, yes, we shouldn't read more bytes than the debug entry
describes, and zero out the rest.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note
  2023-04-04 15:40 ` [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
@ 2023-04-05  1:57   ` Michael Kubacki
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Kubacki @ 2023-04-05  1:57 UTC (permalink / raw)
  To: devel, ardb
  Cc: Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Marvin Häuser, Bob Feng, Oliver Smith-Denny

Acked-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 4/4/2023 11:40 AM, Ard Biesheuvel wrote:
> When performing ELF to PE/COFF conversion, parse any notes sections to
> decide whether the image supports forward CFI landing pads. This will be
> used to set the associated DllCharacteristicsEx flag in a subsequent
> patch.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> ---
>   BaseTools/Source/C/GenFw/Elf64Convert.c | 50 ++++++++++++++++++++
>   BaseTools/Source/C/GenFw/elf_common.h   |  9 ++++
>   2 files changed, 59 insertions(+)
> 
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 8b50774beb1eebda..2a810e835d4a4a66 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -770,6 +770,49 @@ WriteSectionRiscV64 (
>     }
> 
>   }
> 
>   
> 
> +STATIC UINT16 mDllCharacteristicsEx;
> 
> +
> 
> +STATIC
> 
> +VOID
> 
> +ParseNoteSection (
> 
> +  CONST Elf_Shdr  *Shdr
> 
> +  )
> 
> +{
> 
> +  CONST Elf_Note *Note;
> 
> +  CONST UINT32   *Prop;
> 
> +  UINT32         Prop0;
> 
> +  UINT32         Prop2;
> 
> +
> 
> +  Note = (Elf_Note *)((UINT8 *)mEhdr + Shdr->sh_offset);
> 
> +
> 
> +  if ((Note->n_type == NT_GNU_PROPERTY_TYPE_0) &&
> 
> +      (Note->n_namesz == sizeof ("GNU")) &&
> 
> +      (strcmp ((CHAR8 *)(Note + 1), "GNU") == 0) &&
> 
> +      (Note->n_descsz > sizeof (UINT32[2]))) {
> 
> +    Prop = (UINT32 *)((UINT8 *)(Note + 1) + sizeof("GNU"));
> 
> +
> 
> +    switch (mEhdr->e_machine) {
> 
> +    case EM_AARCH64:
> 
> +      Prop0 = GNU_PROPERTY_AARCH64_FEATURE_1_AND;
> 
> +      Prop2 = GNU_PROPERTY_AARCH64_FEATURE_1_BTI;
> 
> +      break;
> 
> +
> 
> +    case EM_X86_64:
> 
> +      Prop0 = GNU_PROPERTY_X86_FEATURE_1_AND;
> 
> +      Prop2 = GNU_PROPERTY_X86_FEATURE_1_IBT;
> 
> +      break;
> 
> +
> 
> +    default:
> 
> +      return;
> 
> +    }
> 
> +    if ((Prop[0] == Prop0) &&
> 
> +        (Prop[1] >= sizeof (UINT32)) &&
> 
> +        ((Prop[2] & Prop2) != 0)) {
> 
> +      mDllCharacteristicsEx |= EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT;
> 
> +    }
> 
> +  }
> 
> +}
> 
> +
> 
>   //
> 
>   // Elf functions interface implementation
> 
>   //
> 
> @@ -826,6 +869,13 @@ ScanSections64 (
>       }
> 
>     }
> 
>   
> 
> +  for (i = 0; i < mEhdr->e_shnum; i++) {
> 
> +    Elf_Shdr *shdr = GetShdrByIndex(i);
> 
> +    if (shdr->sh_type == SHT_NOTE) {
> 
> +      ParseNoteSection (shdr);
> 
> +    }
> 
> +  }
> 
> +
> 
>     //
> 
>     // Check if mCoffAlignment is larger than MAX_COFF_ALIGNMENT
> 
>     //
> 
> diff --git a/BaseTools/Source/C/GenFw/elf_common.h b/BaseTools/Source/C/GenFw/elf_common.h
> index 7b7fdeb3290dfa88..ccd32804b090a226 100644
> --- a/BaseTools/Source/C/GenFw/elf_common.h
> +++ b/BaseTools/Source/C/GenFw/elf_common.h
> @@ -59,6 +59,15 @@ typedef struct {
>     UINT32  n_type;    /* Type of this note. */
> 
>   } Elf_Note;
> 
>   
> 
> +#define NT_GNU_PROPERTY_TYPE_0  5
> 
> +
> 
> +#define GNU_PROPERTY_X86_FEATURE_1_AND  0xc0000002
> 
> +#define GNU_PROPERTY_X86_FEATURE_1_IBT  0x1
> 
> +
> 
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND  0xc0000000
> 
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI  0x1
> 
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC  0x2
> 
> +
> 
>   /* Indexes into the e_ident array.  Keep synced with
> 
>      http://www.sco.com/developers/gabi/latest/ch4.eheader.html */
> 
>   #define EI_MAG0    0  /* Magic number, byte 0. */
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
  2023-04-04 15:40 ` [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Ard Biesheuvel
@ 2023-04-05  1:58   ` Michael Kubacki
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Kubacki @ 2023-04-05  1:58 UTC (permalink / raw)
  To: devel, ardb
  Cc: Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Marvin Häuser, Bob Feng, Oliver Smith-Denny

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 4/4/2023 11:40 AM, Ard Biesheuvel wrote:
> The PE/COFF spec describes an additional DllCharacteristics field
> implemented as a debug directory entry, which carries flags related to
> which control flow integrity (CFI) features are supported by the binary.
> 
> So let's add this entry when doing ELF to PE/COFF conversion - we will
> add support for setting the flags in a subsequent patch.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> ---
>   BaseTools/Source/C/GenFw/Elf64Convert.c               | 54 +++++++++++++++-----
>   BaseTools/Source/C/GenFw/GenFw.c                      |  3 +-
>   BaseTools/Source/C/Include/IndustryStandard/PeImage.h | 13 ++++-
>   3 files changed, 55 insertions(+), 15 deletions(-)
> 
> diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
> index 2a810e835d4a4a66..9c17c90b16602421 100644
> --- a/BaseTools/Source/C/GenFw/Elf64Convert.c
> +++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
> @@ -992,6 +992,16 @@ ScanSections64 (
>                   sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
> 
>                   strlen(mInImageName) + 1;
> 
>   
> 
> +  //
> 
> +  // Add more space in the .debug data region for the DllCharacteristicsEx
> 
> +  // field.
> 
> +  //
> 
> +  if (mDllCharacteristicsEx != 0) {
> 
> +    mCoffOffset = DebugRvaAlign(mCoffOffset) +
> 
> +                  sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) +
> 
> +                  sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
> 
> +  }
> 
> +
> 
>     mCoffOffset = CoffAlign(mCoffOffset);
> 
>     if (SectionCount == 0) {
> 
>       mDataOffset = mCoffOffset;
> 
> @@ -2244,29 +2254,47 @@ WriteDebug64 (
>     VOID
> 
>     )
> 
>   {
> 
> -  UINT32                              Len;
> 
> -  EFI_IMAGE_OPTIONAL_HEADER_UNION     *NtHdr;
> 
> -  EFI_IMAGE_DATA_DIRECTORY            *DataDir;
> 
> -  EFI_IMAGE_DEBUG_DIRECTORY_ENTRY     *Dir;
> 
> -  EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY *Nb10;
> 
> +  UINT32                                      Len;
> 
> +  EFI_IMAGE_OPTIONAL_HEADER_UNION             *NtHdr;
> 
> +  EFI_IMAGE_DATA_DIRECTORY                    *DataDir;
> 
> +  EFI_IMAGE_DEBUG_DIRECTORY_ENTRY             *Dir;
> 
> +  EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY         *Nb10;
> 
> +  EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY *DllEntry;
> 
>   
> 
>     Len = strlen(mInImageName) + 1;
> 
>   
> 
> +  NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset);
> 
> +  DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG];
> 
> +  DataDir->VirtualAddress = mDebugOffset;
> 
> +  DataDir->Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> 
> +
> 
>     Dir = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
> 
> +
> 
> +  if (mDllCharacteristicsEx != 0) {
> 
> +    DataDir->Size  += sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> 
> +
> 
> +    Dir->Type       = EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS;
> 
> +    Dir->SizeOfData = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
> 
> +    Dir->FileOffset = mDebugOffset + DataDir->Size +
> 
> +                      sizeof (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
> 
> +                      DebugRvaAlign(Len);
> 
> +    Dir->RVA        = Dir->FileOffset;
> 
> +
> 
> +    DllEntry = (VOID *)(mCoffFile + Dir->FileOffset);
> 
> +
> 
> +    DllEntry->DllCharacteristicsEx = mDllCharacteristicsEx;
> 
> +
> 
> +    Dir++;
> 
> +  }
> 
> +
> 
>     Dir->Type = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
> 
>     Dir->SizeOfData = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
> 
> -  Dir->RVA = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> 
> -  Dir->FileOffset = mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> 
> +  Dir->RVA = mDebugOffset + DataDir->Size;
> 
> +  Dir->FileOffset = mDebugOffset + DataDir->Size;
> 
>   
> 
>     Nb10 = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1);
> 
>     Nb10->Signature = CODEVIEW_SIGNATURE_NB10;
> 
>     strcpy ((char *)(Nb10 + 1), mInImageName);
> 
> -
> 
> -
> 
> -  NtHdr = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset);
> 
> -  DataDir = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG];
> 
> -  DataDir->VirtualAddress = mDebugOffset;
> 
> -  DataDir->Size = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> 
>   }
> 
>   
> 
>   STATIC
> 
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 6f61f16788cd2a0a..d0e52ccc26431380 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2932,7 +2932,8 @@ Routine Description:
>         if (mIsConvertXip) {
> 
>           DebugEntry->FileOffset = DebugEntry->RVA;
> 
>         }
> 
> -      if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> 
> +      if ((ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) &&
> 
> +          (DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS)) {
> 
>           memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData);
> 
>           memset (DebugEntry, 0, sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY));
> 
>         }
> 
> diff --git a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> index 77ded3f611398278..22161edf443d0e22 100644
> --- a/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> +++ b/BaseTools/Source/C/Include/IndustryStandard/PeImage.h
> @@ -615,7 +615,8 @@ typedef struct {
>   ///
> 
>   /// Debug Format
> 
>   ///
> 
> -#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW 2
> 
> +#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW               2
> 
> +#define EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS  20
> 
>   
> 
>   typedef struct {
> 
>     UINT32  Characteristics;
> 
> @@ -664,6 +665,16 @@ typedef struct {
>     //
> 
>   } EFI_IMAGE_DEBUG_CODEVIEW_MTOC_ENTRY;
> 
>   
> 
> +///
> 
> +/// Extended DLL Characteristics
> 
> +///
> 
> +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_CET_COMPAT          0x0001
> 
> +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT  0x0040
> 
> +
> 
> +typedef struct {
> 
> +  UINT32  DllCharacteristicsEx;
> 
> +} EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY;
> 
> +
> 
>   //
> 
>   // .pdata entries for X64
> 
>   //
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
  2023-04-04 15:40 ` [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context Ard Biesheuvel
@ 2023-04-05  1:58   ` Michael Kubacki
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Kubacki @ 2023-04-05  1:58 UTC (permalink / raw)
  To: devel, ardb
  Cc: Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Marvin Häuser, Bob Feng, Oliver Smith-Denny

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 4/4/2023 11:40 AM, Ard Biesheuvel wrote:
> When loading a PE/COFF image, capture the DLL characteristics fields of
> the header into our image context structure so we can refer to them when
> mapping the image.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> ---
>   MdePkg/Include/IndustryStandard/PeImage.h | 13 +++++-
>   MdePkg/Include/Library/PeCoffLib.h        |  6 +++
>   MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 46 +++++++++++++++-----
>   3 files changed, 53 insertions(+), 12 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/PeImage.h b/MdePkg/Include/IndustryStandard/PeImage.h
> index dd4cc25483bc4bcf..8646ff22b55faff0 100644
> --- a/MdePkg/Include/IndustryStandard/PeImage.h
> +++ b/MdePkg/Include/IndustryStandard/PeImage.h
> @@ -625,7 +625,8 @@ typedef struct {
>     UINT32    FileOffset;  ///< The file pointer to the debug data.
> 
>   } EFI_IMAGE_DEBUG_DIRECTORY_ENTRY;
> 
>   
> 
> -#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW  2    ///< The Visual C++ debug information.
> 
> +#define EFI_IMAGE_DEBUG_TYPE_CODEVIEW               2    ///< The Visual C++ debug information.
> 
> +#define EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS  20
> 
>   
> 
>   ///
> 
>   /// Debug Data Structure defined in Microsoft C++.
> 
> @@ -669,6 +670,16 @@ typedef struct {
>     //
> 
>   } EFI_IMAGE_DEBUG_CODEVIEW_MTOC_ENTRY;
> 
>   
> 
> +///
> 
> +/// Extended DLL Characteristics
> 
> +///
> 
> +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_CET_COMPAT          0x0001
> 
> +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT  0x0040
> 
> +
> 
> +typedef struct {
> 
> +  UINT32    DllCharacteristicsEx;
> 
> +} EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY;
> 
> +
> 
>   ///
> 
>   /// Resource format.
> 
>   ///
> 
> diff --git a/MdePkg/Include/Library/PeCoffLib.h b/MdePkg/Include/Library/PeCoffLib.h
> index b45879453785c77d..74cceb37bf39ffc6 100644
> --- a/MdePkg/Include/Library/PeCoffLib.h
> +++ b/MdePkg/Include/Library/PeCoffLib.h
> @@ -171,6 +171,12 @@ typedef struct {
>     ///
> 
>     UINT16                      ImageType;
> 
>     ///
> 
> +  /// Set by PeCoffLoaderGetImageInfo() to the DLL flags stored in the PE/COFF header and
> 
> +  /// in the DllCharacteristicsEx debug table.
> 
> +  ///
> 
> +  UINT16                      DllCharacteristics;
> 
> +  UINT32                      DllCharacteristicsEx;
> 
> +  ///
> 
>     /// Set by PeCoffLoaderGetImageInfo() to TRUE if the PE/COFF image does not contain
> 
>     /// relocation information.
> 
>     ///
> 
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 97a8aaf8c73d3e3c..4b71176a0c7c2ed0 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -308,10 +308,11 @@ PeCoffLoaderGetPeHeader (
>         //
> 
>         // Use PE32 offset
> 
>         //
> 
> -      ImageContext->ImageType        = Hdr.Pe32->OptionalHeader.Subsystem;
> 
> -      ImageContext->ImageSize        = (UINT64)Hdr.Pe32->OptionalHeader.SizeOfImage;
> 
> -      ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> 
> -      ImageContext->SizeOfHeaders    = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
> 
> +      ImageContext->ImageType          = Hdr.Pe32->OptionalHeader.Subsystem;
> 
> +      ImageContext->ImageSize          = (UINT64)Hdr.Pe32->OptionalHeader.SizeOfImage;
> 
> +      ImageContext->SectionAlignment   = Hdr.Pe32->OptionalHeader.SectionAlignment;
> 
> +      ImageContext->SizeOfHeaders      = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
> 
> +      ImageContext->DllCharacteristics = Hdr.Pe32->OptionalHeader.DllCharacteristics;
> 
>       } else if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> 
>         //
> 
>         // 1. Check FileHeader.NumberOfRvaAndSizes filed.
> 
> @@ -429,10 +430,11 @@ PeCoffLoaderGetPeHeader (
>         //
> 
>         // Use PE32+ offset
> 
>         //
> 
> -      ImageContext->ImageType        = Hdr.Pe32Plus->OptionalHeader.Subsystem;
> 
> -      ImageContext->ImageSize        = (UINT64)Hdr.Pe32Plus->OptionalHeader.SizeOfImage;
> 
> -      ImageContext->SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> 
> -      ImageContext->SizeOfHeaders    = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders;
> 
> +      ImageContext->ImageType          = Hdr.Pe32Plus->OptionalHeader.Subsystem;
> 
> +      ImageContext->ImageSize          = (UINT64)Hdr.Pe32Plus->OptionalHeader.SizeOfImage;
> 
> +      ImageContext->SectionAlignment   = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> 
> +      ImageContext->SizeOfHeaders      = Hdr.Pe32Plus->OptionalHeader.SizeOfHeaders;
> 
> +      ImageContext->DllCharacteristics = Hdr.Pe32Plus->OptionalHeader.DllCharacteristics;
> 
>       } else {
> 
>         ImageContext->ImageError = IMAGE_ERROR_INVALID_MACHINE_TYPE;
> 
>         return RETURN_UNSUPPORTED;
> 
> @@ -545,8 +547,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,
> 
> +  DllCharacteristics, DllCharacteristicsEx and DebugDirectoryEntryRva 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.
> 
> @@ -752,7 +755,28 @@ PeCoffLoaderGetImageInfo (
>                 ImageContext->ImageSize += DebugEntry.SizeOfData;
> 
>               }
> 
>   
> 
> -            return RETURN_SUCCESS;
> 
> +            continue;
> 
> +          }
> 
> +
> 
> +          if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS) {
> 
> +            Size     = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
> 
> +            ReadSize = sizeof (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
> 
> +            Status   = ImageContext->ImageRead (
> 
> +                                       ImageContext->Handle,
> 
> +                                       DebugEntry.FileOffset,
> 
> +                                       &Size,
> 
> +                                       &ImageContext->DllCharacteristicsEx
> 
> +                                       );
> 
> +            if (RETURN_ERROR (Status) || (Size != ReadSize)) {
> 
> +              ImageContext->ImageError = IMAGE_ERROR_IMAGE_READ;
> 
> +              if (Size != ReadSize) {
> 
> +                Status = RETURN_UNSUPPORTED;
> 
> +              }
> 
> +
> 
> +              return Status;
> 
> +            }
> 
> +
> 
> +            continue;
> 
>             }
> 
>           }
> 
>         }
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table
  2023-04-04 15:40 ` [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard Biesheuvel
@ 2023-04-05  1:59   ` Michael Kubacki
  0 siblings, 0 replies; 15+ messages in thread
From: Michael Kubacki @ 2023-04-05  1:59 UTC (permalink / raw)
  To: devel, ardb
  Cc: Michael Kinney, Liming Gao, Jiewen Yao, Michael Kubacki,
	Sean Brogan, Rebecca Cran, Leif Lindholm, Sami Mujawar,
	Taylor Beebe, Marvin Häuser, Bob Feng, Oliver Smith-Denny

Reviewed-by: Michael Kubacki <michael.kubacki@microsoft.com>

On 4/4/2023 11:40 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>
> Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>
> Reviewed-by: Oliver Smith-Denny <osde@linux.microsoft.com>
> ---
>   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;
> 
> +  }
> 
> +
> 
>     //
> 
>     // 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 82fa026bceb990e5..fd127ee167e1ac9a 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,7 +183,12 @@ 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));
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* 回复: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2023-04-04 16:19 ` [PATCH v3 0/4] Enable BTI support in memory " Marvin Häuser
@ 2023-04-06  1:33 ` gaoliming
  2023-04-07 12:40   ` Ard Biesheuvel
  5 siblings, 1 reply; 15+ messages in thread
From: gaoliming @ 2023-04-06  1:33 UTC (permalink / raw)
  To: devel, ardb
  Cc: 'Michael Kinney', 'Jiewen Yao',
	'Michael Kubacki', 'Sean Brogan',
	'Rebecca Cran', 'Leif Lindholm',
	'Sami Mujawar', 'Taylor Beebe',
	'Marvin Häuser', 'Bob Feng',
	'Oliver Smith-Denny'

Ard:
 Can you submit one BZ for this new feature? I will add it into the stable tag feature planning. 

 For this patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2023年4月4日 23:40
> 收件人: devel@edk2.groups.io
> 抄送: Ard Biesheuvel <ardb@kernel.org>; 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>; Oliver
> Smith-Denny <osde@linux.microsoft.com>
> 主题: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes
> table
> 
> Implement version 2 of the memory attributes table, which now contains a
> 
> flag informing the OS whether or not code regions may be mapped with CFI
> 
> mitigations such as IBT or BTI enabled.
> 
> 
> 
> This series covers the remaining parts after the AArch64 specific
> 
> changes were merged:
> 
> 
> 
> - Update the BaseTools to emit the appropriate PE/COFF annotation when a
> 
>   BTI/IBT compatible ELF executable is converted to PE/COFF
> 
> - Take this PE/COFF annotation into account when populating the memory
> 
>   attributes table in the DXE core
> 
> 
> 
> TODO:
> 
> - X64 changes to make the code IBT compatible and emit the ELF note
> 
> - Figure out how to generate such executables with native PE toolchains
> 
> - Implement BTI/IBT enforcement at boot time - this is something I
> 
>   intend to look into next.
> 
> 
> 
> Can be tested with the CLANG38 toolchain (both Clang compiler and LLD
> 
> linker, version 3.8 or newer) with the following build options.
> 
> 
> 
> [BuildOptions]
> 
>   GCC:*_*_AARCH64_PP_FLAGS = -mbranch-protection=bti
> 
>   GCC:*_*_AARCH64_CC_FLAGS = -mbranch-protection=bti
> 
>   GCC:*_*_AARCH64_DLINK_FLAGS = -fuse-ld=lld
> -Wl,--no-relax,--no-pie,-z,bti-report=error
> 
> 
> 
> Changes since v2:
> 
> - increase DllCharacteristicsEx field to 4 bytes
> 
> - add Oliver's Rb
> 
> 
> 
> If no comments or objections have been raised by the end of the week, I
> 
> will go ahead and merge this - thanks.
> 
> 
> 
> 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>
> 
> Cc: Bob Feng <bob.c.feng@intel.com>
> 
> Cc: Oliver Smith-Denny <osde@linux.microsoft.com>
> 
> 
> 
> Ard Biesheuvel (4):
> 
>   BaseTools/GenFw: Parse IBT/BTI support status from ELF note
> 
>   BaseTools/GenFw: Add DllCharacteristicsEx field to debug data
> 
>   MdePkg/PeCoffLib: Capture DLL characteristics fields in image context
> 
>   MdeModulePkg: Enable forward edge CFI in mem attributes table
> 
> 
> 
>  BaseTools/Source/C/GenFw/Elf64Convert.c               | 104
> +++++++++++++++++---
> 
>  BaseTools/Source/C/GenFw/GenFw.c                      |   3 +-
> 
>  BaseTools/Source/C/GenFw/elf_common.h                 |   9 ++
> 
>  BaseTools/Source/C/Include/IndustryStandard/PeImage.h |  13 ++-
> 
>  MdeModulePkg/Core/Dxe/DxeMain.h                       |   2 +
> 
>  MdeModulePkg/Core/Dxe/Image/Image.c                   |  10 ++
> 
>  MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c    |   8 +-
> 
>  MdePkg/Include/IndustryStandard/PeImage.h             |  13 ++-
> 
>  MdePkg/Include/Library/PeCoffLib.h                    |   6 ++
> 
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c             |  46
> ++++++---
> 
>  10 files changed, 186 insertions(+), 28 deletions(-)
> 
> 
> 
> --
> 
> 2.39.2
> 
> 
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#102491):
> https://edk2.groups.io/g/devel/message/102491
> Mute This Topic: https://groups.io/mt/98062730/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 




^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [edk2-devel] [PATCH v3 0/4] Enable BTI support in memory attributes table
  2023-04-06  1:33 ` 回复: [edk2-devel] " gaoliming
@ 2023-04-07 12:40   ` Ard Biesheuvel
  0 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2023-04-07 12:40 UTC (permalink / raw)
  To: devel, gaoliming
  Cc: Michael Kinney, Jiewen Yao, Michael Kubacki, Sean Brogan,
	Rebecca Cran, Leif Lindholm, Sami Mujawar, Taylor Beebe,
	Marvin Häuser, Bob Feng, Oliver Smith-Denny

On Thu, 6 Apr 2023 at 03:34, gaoliming via groups.io
<gaoliming=byosoft.com.cn@groups.io> wrote:
>
> Ard:
>  Can you submit one BZ for this new feature? I will add it into the stable tag feature planning.
>

https://bugzilla.tianocore.org/show_bug.cgi?id=4405

>  For this patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>

Thanks, I've queued these up now.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-04-07 12:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-04 15:40 [PATCH v3 0/4] Enable BTI support in memory attributes table Ard Biesheuvel
2023-04-04 15:40 ` [PATCH v3 1/4] BaseTools/GenFw: Parse IBT/BTI support status from ELF note Ard Biesheuvel
2023-04-05  1:57   ` [edk2-devel] " Michael Kubacki
2023-04-04 15:40 ` [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Ard Biesheuvel
2023-04-05  1:58   ` [edk2-devel] " Michael Kubacki
2023-04-04 15:40 ` [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context Ard Biesheuvel
2023-04-05  1:58   ` [edk2-devel] " Michael Kubacki
2023-04-04 15:40 ` [PATCH v3 4/4] MdeModulePkg: Enable forward edge CFI in mem attributes table Ard Biesheuvel
2023-04-05  1:59   ` [edk2-devel] " Michael Kubacki
2023-04-04 16:19 ` [PATCH v3 0/4] Enable BTI support in memory " Marvin Häuser
2023-04-04 16:29   ` Ard Biesheuvel
2023-04-04 16:42     ` Marvin Häuser
2023-04-04 16:53       ` Ard Biesheuvel
2023-04-06  1:33 ` 回复: [edk2-devel] " gaoliming
2023-04-07 12:40   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox