From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.120600.1680659901563538233 for ; Tue, 04 Apr 2023 18:58:21 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=KgrNDtSU; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id A9AC7210DEA4; Tue, 4 Apr 2023 18:58:19 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com A9AC7210DEA4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1680659901; bh=HoJt7a2rXYTL6sdNnt4zm2jLL/KtN6a/WvQdCwP9JGM=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=KgrNDtSUorLnnTDku5ZKvpxXfflZUYEdtFUKgmDJ55GBxbg2e5lA7SivRod2FDI1X qhlparbf5kOoXpfAK1E/daOwQOvM3SmpLJ9WaOW62PEmAbjDbtRUCTRnXt1ZpFDnn6 C8fCECmNNKkHPh826/DI76WFDW9tz0uNgxe8wBYc= Message-ID: Date: Tue, 4 Apr 2023 21:58:18 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.9.1 Subject: Re: [edk2-devel] [PATCH v3 2/4] BaseTools/GenFw: Add DllCharacteristicsEx field to debug data To: devel@edk2.groups.io, ardb@kernel.org Cc: Michael Kinney , Liming Gao , Jiewen Yao , Michael Kubacki , Sean Brogan , Rebecca Cran , Leif Lindholm , Sami Mujawar , Taylor Beebe , =?UTF-8?Q?Marvin_H=c3=a4user?= , Bob Feng , Oliver Smith-Denny References: <20230404154022.2776035-1-ardb@kernel.org> <20230404154022.2776035-3-ardb@kernel.org> From: "Michael Kubacki" In-Reply-To: <20230404154022.2776035-3-ardb@kernel.org> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Reviewed-by: Michael Kubacki 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 > Reviewed-by: Leif Lindholm > Reviewed-by: Oliver Smith-Denny > --- > 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 > > // >