Re: [edk2-devel] [PATCH v3 3/4] MdePkg/PeCoffLib: Capture DLL characteristics fields in image context

["Michael Kubacki" <mikuback@linux.microsoft.com>]

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;
> 
>             }
> 
>           }
> 
>         }
>