From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout02.posteo.de (mout02.posteo.de [185.67.36.66]) by mx.groups.io with SMTP id smtpd.web10.39162.1679931977638598211 for ; Mon, 27 Mar 2023 08:46:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@posteo.de header.s=2017 header.b=dVo9qwe3; spf=pass (domain: posteo.de, ip: 185.67.36.66, mailfrom: mhaeuser@posteo.de) Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 43424240281 for ; Mon, 27 Mar 2023 17:46:15 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1679931975; bh=BTAIoVtnEiMsIGrF+tmx8Qw6xkcGlbtppekfOCUs4Ok=; h=From:Subject:Date:Cc:To:From; b=dVo9qwe3qGEbYLtn5XZ6467Ve6uIQwipt76G7P8lNFvV26rIyNi5MrXs5WXu7XFCK ks7oIEdoDPs8k1vr4V2C7ueGy2PzLIP6E8XLD9hRhONv88TQ33v9uohJMp678VyhPd acrapem1ZScaysi3l98CrEUlsfKXRpvXrFh36bAgwrA0CkrwNKTQcWtS1Yh5Pjzk+k Sg6dRggTUVep7ErwzsymvYHNLxNr+4iqnbVn8hRrTs4z7M2PCpqBZjRjcx8JXzV8wt 8k/Pz3yhyFnyVjHMqtkiR9Qdr5/N7Gtok5cU+FYmhNQtZAHuu1jg4EuRnHyCPJeAiJ 0ftDIa+tdg4jQ== Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4Plcc41yX9z9rxB; Mon, 27 Mar 2023 17:46:12 +0200 (CEST) From: =?UTF-8?B?TWFydmluIEjDpHVzZXI=?= Message-Id: <49927D70-11BA-4B5D-93B5-764A853F2ED0@posteo.de> Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.500.231\)) Subject: Re: [PATCH v2 14/17] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data Date: Mon, 27 Mar 2023 15:46:01 +0000 In-Reply-To: <20230327110112.262503-15-ardb@kernel.org> Cc: edk2-devel-groups-io , Michael Kinney , Liming Gao , Jiewen Yao , Michael Kubacki , Sean Brogan , Rebecca Cran , Leif Lindholm , Sami Mujawar , Taylor Beebe , Bob Feng To: Ard Biesheuvel References: <20230327110112.262503-1-ardb@kernel.org> <20230327110112.262503-15-ardb@kernel.org> Content-Type: multipart/alternative; boundary="Apple-Mail=_492D58E0-127F-4554-9686-F954869E94E7" --Apple-Mail=_492D58E0-127F-4554-9686-F954869E94E7 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii Hi Ard, > On 27. Mar 2023, at 13:01, Ard Biesheuvel wrote: >=20 > 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. Out of mere personal interest, is there any reference for this yet? The = "PE format" page [1] doesn't seem to have this yet. [1] = https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-debug-= section >=20 > 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. >=20 > Signed-off-by: Ard Biesheuvel > --- > 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(-) >=20 > 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; >=20 > + // > + // Add more space in the .debug data region for the = DllCharacteristicsEx > + // field. > + // > + if (mDllCharacteristicsEx !=3D 0) { > + mCoffOffset =3D DebugRvaAlign(mCoffOffset) + > + sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) + > + sizeof = (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY); > + } > + > mCoffOffset =3D CoffAlign(mCoffOffset); > if (SectionCount =3D=3D 0) { > mDataOffset =3D 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; >=20 > Len =3D strlen(mInImageName) + 1; >=20 > + NtHdr =3D (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + = mNtHdrOffset); > + DataDir =3D = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DE= BUG]; > + DataDir->VirtualAddress =3D mDebugOffset; > + DataDir->Size =3D sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); > + > Dir =3D (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + = mDebugOffset); > + > + if (mDllCharacteristicsEx !=3D 0) { > + DataDir->Size +=3D sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); > + > + Dir->Type =3D EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS; > + Dir->SizeOfData =3D sizeof = (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY); > + Dir->FileOffset =3D mDebugOffset + DataDir->Size + > + sizeof (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + > + DebugRvaAlign(Len); > + Dir->RVA =3D Dir->FileOffset; > + > + DllEntry =3D (VOID *)(mCoffFile + Dir->FileOffset); > + > + DllEntry->DllCharacteristicsEx =3D mDllCharacteristicsEx; > + > + Dir++; > + } > + > Dir->Type =3D EFI_IMAGE_DEBUG_TYPE_CODEVIEW; > Dir->SizeOfData =3D sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + = Len; > - Dir->RVA =3D mDebugOffset + = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); > - Dir->FileOffset =3D mDebugOffset + = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); > + Dir->RVA =3D mDebugOffset + DataDir->Size; > + Dir->FileOffset =3D mDebugOffset + DataDir->Size; >=20 > Nb10 =3D (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + 1); > Nb10->Signature =3D CODEVIEW_SIGNATURE_NB10; > strcpy ((char *)(Nb10 + 1), mInImageName); > - > - > - NtHdr =3D (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + = mNtHdrOffset); > - DataDir =3D = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DE= BUG]; > - DataDir->VirtualAddress =3D mDebugOffset; > - DataDir->Size =3D sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); > } >=20 > 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 =3D DebugEntry->RVA; > } > - if (ZeroDebugFlag || DebugEntry->Type !=3D = EFI_IMAGE_DEBUG_TYPE_CODEVIEW) { > + if ((ZeroDebugFlag || DebugEntry->Type !=3D = EFI_IMAGE_DEBUG_TYPE_CODEVIEW) && > + (DebugEntry->Type !=3D = 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..5e9428ab98c7f68a 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 >=20 > typedef struct { > UINT32 Characteristics; > @@ -664,6 +665,16 @@ typedef struct { > // > } EFI_IMAGE_DEBUG_CODEVIEW_MTOC_ENTRY; >=20 > +/// > +/// Extended DLL Characteristics > +/// > +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_CET_COMPAT 0x0001 > +#define EFI_IMAGE_DLLCHARACTERISTICS_EX_FORWARD_CFI_COMPAT 0x0040 > + > +typedef struct { > + UINT16 DllCharacteristicsEx; > +} EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY; > + > // > // .pdata entries for X64 > // > --=20 > 2.39.2 >=20 --Apple-Mail=_492D58E0-127F-4554-9686-F954869E94E7 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii Hi = Ard,

On 27. Mar 2023, at = 13:01, Ard Biesheuvel <ardb@kernel.org> 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.

Out of mere = personal interest, is there any reference for this yet? The "PE format" = page [1] doesn't seem to have this = yet.



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>
---
= BaseTools/Source/C/GenFw/Elf64Convert.c =             &n= bsp; | 54 +++++++++++++++-----
BaseTools/Source/C/GenFw/GenFw.c =             &n= bsp;        |  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 (
=             &n= bsp;   sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
=             &n= bsp;   strlen(mInImageName) + 1;

+  //
+ =  // Add more space in the .debug data region for the = DllCharacteristicsEx
+  // field.
+  //
+  if = (mDllCharacteristicsEx !=3D 0) {
+    mCoffOffset =3D = DebugRvaAlign(mCoffOffset) +
+ =             &n= bsp;    sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY) = +
+ =             &n= bsp;    sizeof = (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+  }
+
=   mCoffOffset =3D CoffAlign(mCoffOffset);
  if = (SectionCount =3D=3D 0) {
    mDataOffset =3D = mCoffOffset;
@@ -2244,29 +2254,47 @@ WriteDebug64 (
=   VOID
  )
{
-  UINT32 =             &n= bsp;           &nbs= p;    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 =             &n= bsp;           &nbs= p;            = Len;
+  EFI_IMAGE_OPTIONAL_HEADER_UNION =             *N= tHdr;
+  EFI_IMAGE_DATA_DIRECTORY =             &n= bsp;      *DataDir;
+ =  EFI_IMAGE_DEBUG_DIRECTORY_ENTRY =             *D= ir;
+  EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY =         *Nb10;
+ =  EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY *DllEntry;

=   Len =3D strlen(mInImageName) + 1;

+  NtHdr =3D = (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + mNtHdrOffset);
+ =  DataDir =3D = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_E= NTRY_DEBUG];
+  DataDir->VirtualAddress =3D = mDebugOffset;
+  DataDir->Size =3D sizeof = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+
  Dir =3D = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY*)(mCoffFile + mDebugOffset);
+
+ =  if (mDllCharacteristicsEx !=3D 0) {
+ =    DataDir->Size  +=3D sizeof = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+
+ =    Dir->Type       =3D = EFI_IMAGE_DEBUG_TYPE_EX_DLLCHARACTERISTICS;
+ =    Dir->SizeOfData =3D sizeof = (EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY);
+ =    Dir->FileOffset =3D mDebugOffset + DataDir->Size = +
+ =             &n= bsp;        sizeof = (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) +
+ =             &n= bsp;        DebugRvaAlign(Len);+    Dir->RVA =        =3D = Dir->FileOffset;
+
+    DllEntry =3D (VOID = *)(mCoffFile + Dir->FileOffset);
+
+ =    DllEntry->DllCharacteristicsEx =3D = mDllCharacteristicsEx;
+
+    Dir++;
+ =  }
+
  Dir->Type =3D = EFI_IMAGE_DEBUG_TYPE_CODEVIEW;
  Dir->SizeOfData =3D = sizeof(EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY) + Len;
- =  Dir->RVA =3D mDebugOffset + = sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
-  Dir->FileOffset =3D= mDebugOffset + sizeof(EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+ =  Dir->RVA =3D mDebugOffset + DataDir->Size;
+ =  Dir->FileOffset =3D mDebugOffset + DataDir->Size;

=   Nb10 =3D (EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY*)(Dir + = 1);
  Nb10->Signature =3D CODEVIEW_SIGNATURE_NB10;
=   strcpy ((char *)(Nb10 + 1), mInImageName);
-
-
- =  NtHdr =3D (EFI_IMAGE_OPTIONAL_HEADER_UNION *)(mCoffFile + = mNtHdrOffset);
-  DataDir =3D = &NtHdr->Pe32Plus.OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_E= NTRY_DEBUG];
-  DataDir->VirtualAddress =3D = mDebugOffset;
-  DataDir->Size =3D = 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 = =3D DebugEntry->RVA;
      }
- =      if (ZeroDebugFlag || DebugEntry->Type = !=3D EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
+ =      if ((ZeroDebugFlag || DebugEntry->Type = !=3D EFI_IMAGE_DEBUG_TYPE_CODEVIEW) &&
+ =          (DebugEntry->Type= !=3D 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..5e9428ab98c7f68a 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 =             &n= bsp; 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 {
+  UINT16 =  DllCharacteristicsEx;
+} = EFI_IMAGE_DEBUG_EX_DLLCHARACTERISTICS_ENTRY;
+
//
// .pdata = entries for X64
//
-- =
2.39.2


= --Apple-Mail=_492D58E0-127F-4554-9686-F954869E94E7--