Re: [PATCH v2 14/17] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data

["Marvin Häuser" <mhaeuser@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 6273 bytes --]

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.

[1] https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#the-debug-section

> 
> 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               | 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..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
> 
> 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
> 


[-- Attachment #2: Type: text/html, Size: 9310 bytes --]