* [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack
@ 2018-09-06 13:45 Ard Biesheuvel
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
` (4 more replies)
0 siblings, 5 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 13:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Star Zeng, Jian J Wang, Michael D Kinney,
Liming Gao, Chao Zhang, Jiewen Yao, Laszlo Ersek, Leif Lindholm
Now that Itanium support has been dropped from EDK2, we can remove all 11 (!)
occurrences of the ELILO PE/COFF loader hack from the code base.
Note that SecurityPkg appears to have four mostly identical implementations
of the PE/COFF measuring routine, so this may be another area for cleanup
later.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Ard Biesheuvel (4):
MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on
IPF
SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
.../Library/BasePeCoffLib/BasePeCoff.c | 60 +++---------------
.../Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---------
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +-----
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 17 +-----
.../Core/PiSmmCore/MemoryAttributesTable.c | 17 +-----
.../Core/PiSmmCore/SmramProfileRecord.c | 31 +---------
MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++----------------
.../DxeImageVerificationLib.c | 47 +++-----------
.../DxeTpmMeasureBootLib.c | 27 ++------
SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 27 ++------
.../SecureBootConfigImpl.c | 25 ++------
11 files changed, 47 insertions(+), 313 deletions(-)
--
2.18.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
@ 2018-09-06 13:45 ` Ard Biesheuvel
2018-09-06 16:29 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
2018-09-06 13:45 ` [PATCH 2/4] MdePkg/BasePeCoffLib: " Ard Biesheuvel
` (3 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 13:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Star Zeng, Jian J Wang, Michael D Kinney,
Liming Gao, Chao Zhang, Jiewen Yao, Laszlo Ersek, Leif Lindholm
Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +-------------------
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +----------
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 17 +----------
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +----------
MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 31 +-------------------
5 files changed, 5 insertions(+), 108 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
index 83ed43a16e95..ff1940431c2f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
+++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
@@ -254,35 +254,6 @@ GetMemoryProfileContext (
return mMemoryProfileContextPtr;
}
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
/**
Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
If Pe32Data is NULL, then ASSERT().
@@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
return Hdr.Te->Subsystem;
} else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
- Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
return Hdr.Pe32->OptionalHeader.Subsystem;
} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 4cd219c88efc..fa8f8fe91ac7 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -406,7 +406,6 @@ ProtectUefiImage (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
BOOLEAN IsAligned;
UINT32 ProtectionPolicy;
@@ -466,21 +465,7 @@ ProtectUefiImage (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a96d442fbc64..05eb4f422b2f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1076,7 +1076,6 @@ InsertImageRecord (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", RuntimeImage));
DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize));
@@ -1126,21 +1125,7 @@ InsertImageRecord (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index 1682d0f9e404..157beb1e67ff 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -1032,7 +1032,6 @@ SmmInsertImageRecord (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%x\n", DriverEntry));
DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%016lx - 0x%08x\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
@@ -1076,21 +1075,7 @@ SmmInsertImageRecord (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
index b586afa7948e..e8d60c2d2357 100644
--- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
+++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
@@ -255,35 +255,6 @@ GetSmramProfileContext (
return mSmramProfileContextPtr;
}
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
/**
Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
If Pe32Data is NULL, then ASSERT().
@@ -320,7 +291,7 @@ InternalPeCoffGetSubsystem (
if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
return Hdr.Te->Subsystem;
} else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
- Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
return Hdr.Pe32->OptionalHeader.Subsystem;
} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
--
2.18.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
@ 2018-09-06 13:45 ` Ard Biesheuvel
2018-09-06 16:31 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
2018-09-06 13:45 ` [PATCH 3/4] SecurityPkg: " Ard Biesheuvel
` (2 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 13:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Star Zeng, Jian J Wang, Michael D Kinney,
Liming Gao, Chao Zhang, Jiewen Yao, Laszlo Ersek, Leif Lindholm
Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-----------------
1 file changed, 9 insertions(+), 52 deletions(-)
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 32eca0ad2ef4..c57816a80887 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
SectionHeader->PointerToRawData -= TeStrippedOffset;
}
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
-
/**
Retrieves the PE or TE Header from a PE/COFF or TE image.
@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
EFI_IMAGE_DOS_HEADER DosHdr;
UINTN Size;
UINTN ReadSize;
- UINT16 Magic;
UINT32 SectionHeaderOffset;
UINT32 Index;
UINT32 HeaderWithoutDataDir;
@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
ImageContext->IsTeImage = FALSE;
ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
//
@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
- } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+ } else if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
//
// 1. Check FileHeader.NumberOfRvaAndSizes filed.
//
@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
EFI_IMAGE_SECTION_HEADER SectionHeader;
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY DebugEntry;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
UINT32 TeStrippedOffset;
if (ImageContext == NULL) {
@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
return Status;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
//
// Retrieve the base address of the image
//
if (!(ImageContext->IsTeImage)) {
TeStrippedOffset = 0;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
}
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
CHAR8 *FixupData;
PHYSICAL_ADDRESS BaseAddress;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
UINT32 TeStrippedOffset;
ASSERT (ImageContext != NULL);
@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
if (!(ImageContext->IsTeImage)) {
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + ImageContext->PeCoffHeaderOffset);
TeStrippedOffset = 0;
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1230,7 +1192,6 @@ PeCoffLoaderLoadImage (
UINTN Size;
UINT32 TempDebugEntryRva;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
EFI_IMAGE_RESOURCE_DIRECTORY *ResourceDirectory;
EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;
EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;
@@ -1404,12 +1365,11 @@ PeCoffLoaderLoadImage (
//
// Get image's entry point
//
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
if (!(ImageContext->IsTeImage)) {
//
// Sizes of AddressOfEntryPoint are different so we need to do this safely
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1444,7 +1404,7 @@ PeCoffLoaderLoadImage (
// the optional header to verify a desired directory entry is there.
//
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1565,7 +1525,7 @@ PeCoffLoaderLoadImage (
//
ImageContext->HiiResourceData = 0;
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1721,7 +1681,6 @@ PeCoffLoaderRelocateImageForRuntime (
CHAR8 *FixupData;
UINTN Adjust;
RETURN_STATUS Status;
- UINT16 Magic;
OldBase = (CHAR8 *)((UINTN)ImageBase);
NewBase = (CHAR8 *)((UINTN)VirtImageBase);
@@ -1750,9 +1709,7 @@ PeCoffLoaderRelocateImageForRuntime (
return ;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
--
2.18.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
2018-09-06 13:45 ` [PATCH 2/4] MdePkg/BasePeCoffLib: " Ard Biesheuvel
@ 2018-09-06 13:45 ` Ard Biesheuvel
2018-09-06 16:47 ` Laszlo Ersek
2018-09-06 13:45 ` [PATCH 4/4] EdkCompatibilityPkg: " Ard Biesheuvel
2018-09-06 14:05 ` [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Yao, Jiewen
4 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 13:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Star Zeng, Jian J Wang, Michael D Kinney,
Liming Gao, Chao Zhang, Jiewen Yao, Laszlo Ersek, Leif Lindholm
Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 47 ++++----------------
SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c | 27 +++--------
SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 27 +++--------
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 25 +++--------
4 files changed, 25 insertions(+), 101 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 0f795c0af125..66d96a9396b9 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -295,7 +295,6 @@ HashPeImage (
)
{
BOOLEAN Status;
- UINT16 Magic;
EFI_IMAGE_SECTION_HEADER *Section;
VOID *HashCtx;
UINTN CtxSize;
@@ -367,33 +366,19 @@ HashPeImage (
// Measuring PE/COFF Image Header;
// But CheckSum field and SECURITY data directory (certificate) are excluded
//
- if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = mNtHeader.Pe32->OptionalHeader.Magic;
- }
//
// 3. Calculate the distance from the base of the image header to the image checksum address.
// 4. Hash the image header from its base to beginning of the image checksum.
//
HashBase = mImageBase;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) HashBase;
NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
- } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+ } else if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
//
// Use PE32+ offset.
//
@@ -420,7 +405,7 @@ HashPeImage (
// 6. Since there is no Cert Directory in optional header, hash everything
// from the end of the checksum to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -444,7 +429,7 @@ HashPeImage (
//
// 7. Hash everything from the end of the checksum to the start of the Cert Directory.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -469,7 +454,7 @@ HashPeImage (
// 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
// 9. Hash everything from the end of the Cert Directory to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -494,7 +479,7 @@ HashPeImage (
//
// 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -577,7 +562,7 @@ HashPeImage (
if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
CertSize = 0;
} else {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -1583,7 +1568,6 @@ DxeImageVerificationHandler (
)
{
EFI_STATUS Status;
- UINT16 Magic;
EFI_IMAGE_DOS_HEADER *DosHdr;
EFI_STATUS VerifyStatus;
EFI_SIGNATURE_LIST *SignatureList;
@@ -1723,22 +1707,7 @@ DxeImageVerificationHandler (
goto Done;
}
- if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = mNtHeader.Pe32->OptionalHeader.Magic;
- }
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index c54ab62e2745..4e4a90f9da62 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -320,7 +320,6 @@ TcgMeasurePeImage (
EFI_IMAGE_SECTION_HEADER *SectionHeader;
UINTN Index;
UINTN Pos;
- UINT16 Magic;
UINT32 EventSize;
UINT32 EventNumber;
EFI_PHYSICAL_ADDRESS EventLogLastEntry;
@@ -418,27 +417,13 @@ TcgMeasurePeImage (
// Measuring PE/COFF Image Header;
// But CheckSum field and SECURITY data directory (certificate) are excluded
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
//
// 3. Calculate the distance from the base of the image header to the image checksum address.
// 4. Hash the image header from its base to beginning of the image checksum.
//
HashBase = (UINT8 *) (UINTN) ImageAddress;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -465,7 +450,7 @@ TcgMeasurePeImage (
// 6. Since there is no Cert Directory in optional header, hash everything
// from the end of the checksum to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -489,7 +474,7 @@ TcgMeasurePeImage (
//
// 7. Hash everything from the end of the checksum to the start of the Cert Directory.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -514,7 +499,7 @@ TcgMeasurePeImage (
// 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
// 9. Hash everything from the end of the Cert Directory to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -539,7 +524,7 @@ TcgMeasurePeImage (
//
// 10. Set the SUM_OF_BYTES_HASHED to the size of the header
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -621,7 +606,7 @@ TcgMeasurePeImage (
if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
CertSize = 0;
} else {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
index 29da2d70e699..61195e6041f7 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
@@ -116,7 +116,6 @@ MeasurePeImageAndExtend (
EFI_IMAGE_SECTION_HEADER *SectionHeader;
UINTN Index;
UINTN Pos;
- UINT16 Magic;
EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
UINT32 NumberOfRvaAndSizes;
UINT32 CertSize;
@@ -181,27 +180,13 @@ MeasurePeImageAndExtend (
// Measuring PE/COFF Image Header;
// But CheckSum field and SECURITY data directory (certificate) are excluded
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
//
// 3. Calculate the distance from the base of the image header to the image checksum address.
// 4. Hash the image header from its base to beginning of the image checksum.
//
HashBase = (UINT8 *) (UINTN) ImageAddress;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -228,7 +213,7 @@ MeasurePeImageAndExtend (
// 6. Since there is no Cert Directory in optional header, hash everything
// from the end of the checksum to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -252,7 +237,7 @@ MeasurePeImageAndExtend (
//
// 7. Hash everything from the end of the checksum to the start of the Cert Directory.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -277,7 +262,7 @@ MeasurePeImageAndExtend (
// 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
// 9. Hash everything from the end of the Cert Directory to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -302,7 +287,7 @@ MeasurePeImageAndExtend (
//
// 10. Set the SUM_OF_BYTES_HASHED to the size of the header
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -384,7 +369,7 @@ MeasurePeImageAndExtend (
if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
CertSize = 0;
} else {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 9acaa7b97507..f96325e978a5 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -1831,7 +1831,6 @@ HashPeImage (
)
{
BOOLEAN Status;
- UINT16 Magic;
EFI_IMAGE_SECTION_HEADER *Section;
VOID *HashCtx;
UINTN CtxSize;
@@ -1874,27 +1873,13 @@ HashPeImage (
// Measuring PE/COFF Image Header;
// But CheckSum field and SECURITY data directory (certificate) are excluded
//
- if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = mNtHeader.Pe32->OptionalHeader.Magic;
- }
//
// 3. Calculate the distance from the base of the image header to the image checksum address.
// 4. Hash the image header from its base to beginning of the image checksum.
//
HashBase = mImageBase;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -1915,7 +1900,7 @@ HashPeImage (
// 6. Get the address of the beginning of the Cert Directory.
// 7. Hash everything from the end of the checksum to the start of the Cert Directory.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -1937,7 +1922,7 @@ HashPeImage (
// 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
// 9. Hash everything from the end of the Cert Directory to the end of image header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1958,7 +1943,7 @@ HashPeImage (
//
// 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
@@ -2032,7 +2017,7 @@ HashPeImage (
//
if (mImageSize > SumOfBytesHashed) {
HashBase = mImageBase + SumOfBytesHashed;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset.
//
--
2.18.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
` (2 preceding siblings ...)
2018-09-06 13:45 ` [PATCH 3/4] SecurityPkg: " Ard Biesheuvel
@ 2018-09-06 13:45 ` Ard Biesheuvel
2018-09-06 16:53 ` Laszlo Ersek
2018-09-06 14:05 ` [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Yao, Jiewen
4 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 13:45 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Star Zeng, Jian J Wang, Michael D Kinney,
Liming Gao, Chao Zhang, Jiewen Yao, Laszlo Ersek, Leif Lindholm
Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c | 60 +++-----------------
1 file changed, 8 insertions(+), 52 deletions(-)
diff --git a/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c b/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
index f99c23e5ee4c..28d39b342afd 100644
--- a/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
+++ b/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
@@ -22,36 +22,6 @@ Abstract:
#include "BasePeCoffLibInternals.h"
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == EFI_IMAGE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
-
/**
Retrieves the PE or TE Header from a PE/COFF or TE image.
@@ -71,7 +41,6 @@ GluePeCoffLoaderGetPeHeader (
RETURN_STATUS Status;
EFI_IMAGE_DOS_HEADER DosHdr;
UINTN Size;
- UINT16 Magic;
//
// Read the DOS image header to check for it's existance
@@ -130,9 +99,7 @@ GluePeCoffLoaderGetPeHeader (
ImageContext->IsTeImage = FALSE;
ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -141,7 +108,7 @@ GluePeCoffLoaderGetPeHeader (
ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
- } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+ } else if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
//
// Use PE32+ offset
//
@@ -209,7 +176,6 @@ GluePeCoffLoaderGetImageInfo (
EFI_IMAGE_SECTION_HEADER SectionHeader;
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY DebugEntry;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
if (NULL == ImageContext) {
return RETURN_INVALID_PARAMETER;
@@ -225,13 +191,11 @@ GluePeCoffLoaderGetImageInfo (
return Status;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
//
// Retrieve the base address of the image
//
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -276,7 +240,7 @@ GluePeCoffLoaderGetImageInfo (
}
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -520,7 +484,6 @@ GluePeCoffLoaderRelocateImage (
CHAR8 *FixupData;
PHYSICAL_ADDRESS BaseAddress;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
ASSERT (ImageContext != NULL);
@@ -552,9 +515,7 @@ GluePeCoffLoaderRelocateImage (
if (!(ImageContext->IsTeImage)) {
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + ImageContext->PeCoffHeaderOffset);
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -777,7 +738,6 @@ GluePeCoffLoaderLoadImage (
UINTN Size;
UINT32 TempDebugEntryRva;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
ASSERT (ImageContext != NULL);
@@ -965,12 +925,11 @@ GluePeCoffLoaderLoadImage (
//
// Get image's entry point
//
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
if (!(ImageContext->IsTeImage)) {
//
// Sizes of AddressOfEntryPoint are different so we need to do this safely
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1004,7 +963,7 @@ GluePeCoffLoaderLoadImage (
// the optional header to verify a desired directory entry is there.
//
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1172,7 +1131,6 @@ PeCoffLoaderRelocateImageForRuntime (
CHAR8 *FixupData;
UINTN Adjust;
RETURN_STATUS Status;
- UINT16 Magic;
OldBase = (CHAR8 *)((UINTN)ImageBase);
NewBase = (CHAR8 *)((UINTN)VirtImageBase);
@@ -1201,9 +1159,7 @@ PeCoffLoaderRelocateImageForRuntime (
return ;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
--
2.18.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
` (3 preceding siblings ...)
2018-09-06 13:45 ` [PATCH 4/4] EdkCompatibilityPkg: " Ard Biesheuvel
@ 2018-09-06 14:05 ` Yao, Jiewen
4 siblings, 0 replies; 18+ messages in thread
From: Yao, Jiewen @ 2018-09-06 14:05 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Zeng, Star, Wang, Jian J, Kinney, Michael D, Gao, Liming,
Zhang, Chao B, Laszlo Ersek, Leif Lindholm
Thanks for the clean up.
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, September 6, 2018 9:45 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Zeng, Star
> <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kinney, Michael
> D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
> Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>
> Subject: [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header
> hack
>
> Now that Itanium support has been dropped from EDK2, we can remove all 11
> (!)
> occurrences of the ELILO PE/COFF loader hack from the code base.
>
> Note that SecurityPkg appears to have four mostly identical implementations
> of the PE/COFF measuring routine, so this may be another area for cleanup
> later.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>
> Ard Biesheuvel (4):
> MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
> MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on
> IPF
> SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
> EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
>
> .../Library/BasePeCoffLib/BasePeCoff.c | 60 +++---------------
> .../Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---------
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +-----
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 17 +-----
> .../Core/PiSmmCore/MemoryAttributesTable.c | 17 +-----
> .../Core/PiSmmCore/SmramProfileRecord.c | 31 +---------
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++----------------
> .../DxeImageVerificationLib.c | 47 +++-----------
> .../DxeTpmMeasureBootLib.c | 27 ++------
> SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 27 ++------
> .../SecureBootConfigImpl.c | 25 ++------
> 11 files changed, 47 insertions(+), 313 deletions(-)
>
> --
> 2.18.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
@ 2018-09-06 16:29 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
1 sibling, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 16:29 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Liming Gao, Jiewen Yao, Star Zeng, Michael D Kinney, Chao Zhang
On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +-------------------
> MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +----------
> MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 17 +----------
> MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +----------
> MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 31 +-------------------
> 5 files changed, 5 insertions(+), 108 deletions(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> index 83ed43a16e95..ff1940431c2f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> @@ -254,35 +254,6 @@ GetMemoryProfileContext (
> return mMemoryProfileContextPtr;
> }
>
> -/**
> - Retrieves the magic value from the PE/COFF header.
> -
> - @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
> -
> - @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> - @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -UINT16
> -InternalPeCoffGetPeHeaderMagicValue (
> - IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
> - )
> -{
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - }
> - //
> - // Return the magic value from the PC/COFF Optional Header
> - //
> - return Hdr.Pe32->OptionalHeader.Magic;
> -}
> -
> /**
> Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
> If Pe32Data is NULL, then ASSERT().
> @@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
> if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
> return Hdr.Te->Subsystem;
> } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
> - Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
> + Magic = Hdr.Pe32->OptionalHeader.Magic;
> if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> return Hdr.Pe32->OptionalHeader.Subsystem;
> } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 4cd219c88efc..fa8f8fe91ac7 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -406,7 +406,6 @@ ProtectUefiImage (
> IMAGE_PROPERTIES_RECORD *ImageRecord;
> CHAR8 *PdbPointer;
> IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> - UINT16 Magic;
> BOOLEAN IsAligned;
> UINT32 ProtectionPolicy;
>
> @@ -466,21 +465,7 @@ ProtectUefiImage (
> //
> // Get SectionAlignment
> //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = Hdr.Pe32->OptionalHeader.Magic;
> - }
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> } else {
> SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index a96d442fbc64..05eb4f422b2f 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -1076,7 +1076,6 @@ InsertImageRecord (
> IMAGE_PROPERTIES_RECORD *ImageRecord;
> CHAR8 *PdbPointer;
> IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> - UINT16 Magic;
>
> DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", RuntimeImage));
> DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize));
> @@ -1126,21 +1125,7 @@ InsertImageRecord (
> //
> // Get SectionAlignment
> //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = Hdr.Pe32->OptionalHeader.Magic;
> - }
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> } else {
> SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> index 1682d0f9e404..157beb1e67ff 100644
> --- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> +++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
> @@ -1032,7 +1032,6 @@ SmmInsertImageRecord (
> IMAGE_PROPERTIES_RECORD *ImageRecord;
> CHAR8 *PdbPointer;
> IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> - UINT16 Magic;
>
> DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%x\n", DriverEntry));
> DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%016lx - 0x%08x\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
> @@ -1076,21 +1075,7 @@ SmmInsertImageRecord (
> //
> // Get SectionAlignment
> //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = Hdr.Pe32->OptionalHeader.Magic;
> - }
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> } else {
> SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
> index b586afa7948e..e8d60c2d2357 100644
> --- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
> +++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
> @@ -255,35 +255,6 @@ GetSmramProfileContext (
> return mSmramProfileContextPtr;
> }
>
> -/**
> - Retrieves the magic value from the PE/COFF header.
> -
> - @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
> -
> - @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> - @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -UINT16
> -InternalPeCoffGetPeHeaderMagicValue (
> - IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
> - )
> -{
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - }
> - //
> - // Return the magic value from the PC/COFF Optional Header
> - //
> - return Hdr.Pe32->OptionalHeader.Magic;
> -}
> -
> /**
> Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
> If Pe32Data is NULL, then ASSERT().
> @@ -320,7 +291,7 @@ InternalPeCoffGetSubsystem (
> if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
> return Hdr.Te->Subsystem;
> } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
> - Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
> + Magic = Hdr.Pe32->OptionalHeader.Magic;
> if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> return Hdr.Pe32->OptionalHeader.Subsystem;
> } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 2/4] MdePkg/BasePeCoffLib: " Ard Biesheuvel
@ 2018-09-06 16:31 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
1 sibling, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 16:31 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Liming Gao, Jiewen Yao, Star Zeng, Michael D Kinney, Chao Zhang
On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-----------------
> 1 file changed, 9 insertions(+), 52 deletions(-)
>
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 32eca0ad2ef4..c57816a80887 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
> SectionHeader->PointerToRawData -= TeStrippedOffset;
> }
>
> -/**
> - Retrieves the magic value from the PE/COFF header.
> -
> - @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
> -
> - @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> - @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -UINT16
> -PeCoffLoaderGetPeHeaderMagicValue (
> - IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
> - )
> -{
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - }
> - //
> - // Return the magic value from the PC/COFF Optional Header
> - //
> - return Hdr.Pe32->OptionalHeader.Magic;
> -}
> -
> -
> /**
> Retrieves the PE or TE Header from a PE/COFF or TE image.
>
> @@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
> EFI_IMAGE_DOS_HEADER DosHdr;
> UINTN Size;
> UINTN ReadSize;
> - UINT16 Magic;
> UINT32 SectionHeaderOffset;
> UINT32 Index;
> UINT32 HeaderWithoutDataDir;
> @@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
> ImageContext->IsTeImage = FALSE;
> ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
>
> - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> -
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
> //
> @@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
> ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
> ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
>
> - } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> + } else if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> //
> // 1. Check FileHeader.NumberOfRvaAndSizes filed.
> //
> @@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
> EFI_IMAGE_SECTION_HEADER SectionHeader;
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY DebugEntry;
> UINT32 NumberOfRvaAndSizes;
> - UINT16 Magic;
> UINT32 TeStrippedOffset;
>
> if (ImageContext == NULL) {
> @@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
> return Status;
> }
>
> - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> -
> //
> // Retrieve the base address of the image
> //
> if (!(ImageContext->IsTeImage)) {
> TeStrippedOffset = 0;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
> }
>
> if (!(ImageContext->IsTeImage)) {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
> CHAR8 *FixupData;
> PHYSICAL_ADDRESS BaseAddress;
> UINT32 NumberOfRvaAndSizes;
> - UINT16 Magic;
> UINT32 TeStrippedOffset;
>
> ASSERT (ImageContext != NULL);
> @@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
> if (!(ImageContext->IsTeImage)) {
> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + ImageContext->PeCoffHeaderOffset);
> TeStrippedOffset = 0;
> - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -1230,7 +1192,6 @@ PeCoffLoaderLoadImage (
> UINTN Size;
> UINT32 TempDebugEntryRva;
> UINT32 NumberOfRvaAndSizes;
> - UINT16 Magic;
> EFI_IMAGE_RESOURCE_DIRECTORY *ResourceDirectory;
> EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;
> EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;
> @@ -1404,12 +1365,11 @@ PeCoffLoaderLoadImage (
> //
> // Get image's entry point
> //
> - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> if (!(ImageContext->IsTeImage)) {
> //
> // Sizes of AddressOfEntryPoint are different so we need to do this safely
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -1444,7 +1404,7 @@ PeCoffLoaderLoadImage (
> // the optional header to verify a desired directory entry is there.
> //
> if (!(ImageContext->IsTeImage)) {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -1565,7 +1525,7 @@ PeCoffLoaderLoadImage (
> //
> ImageContext->HiiResourceData = 0;
> if (!(ImageContext->IsTeImage)) {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -1721,7 +1681,6 @@ PeCoffLoaderRelocateImageForRuntime (
> CHAR8 *FixupData;
> UINTN Adjust;
> RETURN_STATUS Status;
> - UINT16 Magic;
>
> OldBase = (CHAR8 *)((UINTN)ImageBase);
> NewBase = (CHAR8 *)((UINTN)VirtImageBase);
> @@ -1750,9 +1709,7 @@ PeCoffLoaderRelocateImageForRuntime (
> return ;
> }
>
> - Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> -
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 3/4] SecurityPkg: " Ard Biesheuvel
@ 2018-09-06 16:47 ` Laszlo Ersek
2018-09-06 17:25 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 16:47 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Liming Gao, Jiewen Yao, Star Zeng, Michael D Kinney, Chao Zhang
On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 47 ++++----------------
> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c | 27 +++--------
> SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 27 +++--------
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 25 +++--------
> 4 files changed, 25 insertions(+), 101 deletions(-)
>
> diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 0f795c0af125..66d96a9396b9 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -295,7 +295,6 @@ HashPeImage (
> )
> {
> BOOLEAN Status;
> - UINT16 Magic;
> EFI_IMAGE_SECTION_HEADER *Section;
> VOID *HashCtx;
> UINTN CtxSize;
> @@ -367,33 +366,19 @@ HashPeImage (
> // Measuring PE/COFF Image Header;
> // But CheckSum field and SECURITY data directory (certificate) are excluded
> //
> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
> - }
>
> //
> // 3. Calculate the distance from the base of the image header to the image checksum address.
> // 4. Hash the image header from its base to beginning of the image checksum.
> //
> HashBase = mImageBase;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) HashBase;
> NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
> - } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> + } else if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> //
> // Use PE32+ offset.
> //
> @@ -420,7 +405,7 @@ HashPeImage (
> // 6. Since there is no Cert Directory in optional header, hash everything
> // from the end of the checksum to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -444,7 +429,7 @@ HashPeImage (
> //
> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -469,7 +454,7 @@ HashPeImage (
> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
> // 9. Hash everything from the end of the Cert Directory to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -494,7 +479,7 @@ HashPeImage (
> //
> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -577,7 +562,7 @@ HashPeImage (
> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
> CertSize = 0;
> } else {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -1583,7 +1568,6 @@ DxeImageVerificationHandler (
> )
> {
> EFI_STATUS Status;
> - UINT16 Magic;
> EFI_IMAGE_DOS_HEADER *DosHdr;
> EFI_STATUS VerifyStatus;
> EFI_SIGNATURE_LIST *SignatureList;
> @@ -1723,22 +1707,7 @@ DxeImageVerificationHandler (
> goto Done;
> }
>
> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
> - }
> -
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
> index c54ab62e2745..4e4a90f9da62 100644
> --- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
> +++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
> @@ -320,7 +320,6 @@ TcgMeasurePeImage (
> EFI_IMAGE_SECTION_HEADER *SectionHeader;
> UINTN Index;
> UINTN Pos;
> - UINT16 Magic;
> UINT32 EventSize;
> UINT32 EventNumber;
> EFI_PHYSICAL_ADDRESS EventLogLastEntry;
> @@ -418,27 +417,13 @@ TcgMeasurePeImage (
> // Measuring PE/COFF Image Header;
> // But CheckSum field and SECURITY data directory (certificate) are excluded
> //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = Hdr.Pe32->OptionalHeader.Magic;
> - }
>
> //
> // 3. Calculate the distance from the base of the image header to the image checksum address.
> // 4. Hash the image header from its base to beginning of the image checksum.
> //
> HashBase = (UINT8 *) (UINTN) ImageAddress;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -465,7 +450,7 @@ TcgMeasurePeImage (
> // 6. Since there is no Cert Directory in optional header, hash everything
> // from the end of the checksum to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -489,7 +474,7 @@ TcgMeasurePeImage (
> //
> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -514,7 +499,7 @@ TcgMeasurePeImage (
> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
> // 9. Hash everything from the end of the Cert Directory to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -539,7 +524,7 @@ TcgMeasurePeImage (
> //
> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -621,7 +606,7 @@ TcgMeasurePeImage (
> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
> CertSize = 0;
> } else {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
> index 29da2d70e699..61195e6041f7 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
> @@ -116,7 +116,6 @@ MeasurePeImageAndExtend (
> EFI_IMAGE_SECTION_HEADER *SectionHeader;
> UINTN Index;
> UINTN Pos;
> - UINT16 Magic;
> EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
> UINT32 NumberOfRvaAndSizes;
> UINT32 CertSize;
> @@ -181,27 +180,13 @@ MeasurePeImageAndExtend (
> // Measuring PE/COFF Image Header;
> // But CheckSum field and SECURITY data directory (certificate) are excluded
> //
> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = Hdr.Pe32->OptionalHeader.Magic;
> - }
>
> //
> // 3. Calculate the distance from the base of the image header to the image checksum address.
> // 4. Hash the image header from its base to beginning of the image checksum.
> //
> HashBase = (UINT8 *) (UINTN) ImageAddress;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
I think this is a bug. Here "Magic" used to be set based on "Hdr.Pe32->OptionalHeader.Magic", not based on "mNtHeader.Pe32->OptionalHeader.Magic".
And, I'm not convinced (mNtHeader.Pe32 == Hdr.Pe32). In earlier parts of this function, "Hdr.Pe32" is set as follows:
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
where "ImageAddress" is an input parameter to the function.
I wonder why this code compiles; no "mNtHeader" declaration should be in scope here.
$ git grep -w -l mNtHeader
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
More of the same below:
> //
> // Use PE32 offset
> //
> @@ -228,7 +213,7 @@ MeasurePeImageAndExtend (
> // 6. Since there is no Cert Directory in optional header, hash everything
> // from the end of the checksum to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -252,7 +237,7 @@ MeasurePeImageAndExtend (
> //
> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -277,7 +262,7 @@ MeasurePeImageAndExtend (
> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
> // 9. Hash everything from the end of the Cert Directory to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -302,7 +287,7 @@ MeasurePeImageAndExtend (
> //
> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -384,7 +369,7 @@ MeasurePeImageAndExtend (
> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
> CertSize = 0;
> } else {
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
Until here.
For build-testing, I suggest
build -p SecurityPkg/SecurityPkg.dsc ...
It will build all SecurityPkg modules, without putting them in a flash device (FD).
The rest looks good to me.
Thanks,
Laszlo
> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> index 9acaa7b97507..f96325e978a5 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
> @@ -1831,7 +1831,6 @@ HashPeImage (
> )
> {
> BOOLEAN Status;
> - UINT16 Magic;
> EFI_IMAGE_SECTION_HEADER *Section;
> VOID *HashCtx;
> UINTN CtxSize;
> @@ -1874,27 +1873,13 @@ HashPeImage (
> // Measuring PE/COFF Image Header;
> // But CheckSum field and SECURITY data directory (certificate) are excluded
> //
> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> - //
> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> - //
> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> - } else {
> - //
> - // Get the magic value from the PE/COFF Optional Header
> - //
> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
> - }
>
> //
> // 3. Calculate the distance from the base of the image header to the image checksum address.
> // 4. Hash the image header from its base to beginning of the image checksum.
> //
> HashBase = mImageBase;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -1915,7 +1900,7 @@ HashPeImage (
> // 6. Get the address of the beginning of the Cert Directory.
> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -1937,7 +1922,7 @@ HashPeImage (
> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
> // 9. Hash everything from the end of the Cert Directory to the end of image header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
> @@ -1958,7 +1943,7 @@ HashPeImage (
> //
> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
> //
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
> @@ -2032,7 +2017,7 @@ HashPeImage (
> //
> if (mImageSize > SumOfBytesHashed) {
> HashBase = mImageBase + SumOfBytesHashed;
> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset.
> //
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 4/4] EdkCompatibilityPkg: " Ard Biesheuvel
@ 2018-09-06 16:53 ` Laszlo Ersek
2018-09-06 17:27 ` Ard Biesheuvel
0 siblings, 1 reply; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 16:53 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel
Cc: Liming Gao, Jiewen Yao, Star Zeng, Michael D Kinney, Chao Zhang
On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c | 60 +++-----------------
> 1 file changed, 8 insertions(+), 52 deletions(-)
Should we care about EdkCompatibilityPkg at all? Because:
* IPF removal seems not to have occurred to EdkCompatibilityPkg:
$ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
[bunch of hits]
* In <https://bugzilla.tianocore.org/show_bug.cgi?id=816#c7>, you wrote:
> [...] there is a big difference between IPF drivers that are never
> referenced by modern platforms, and workarounds in generic code that
> are present in every modern build for every platform, and are only
> intended for a specific build of ELILO.
> The former is essentially dead code. The latter gets executed many
> times on every boot of every modern UEFI platform in existence.
Under that distinction, I would classify EdkCompatibilityPkg as the
first category, i.e., essentially dead code.
(I'm pointing this out in the hope that it'll save me the review of this
patch! :) )
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 16:47 ` Laszlo Ersek
@ 2018-09-06 17:25 ` Ard Biesheuvel
2018-09-06 18:22 ` Laszlo Ersek
0 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 17:25 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel@lists.01.org, Liming Gao, Jiewen Yao, Star Zeng,
Michael D Kinney, Chao Zhang
On 6 September 2018 at 18:47, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/06/18 15:45, Ard Biesheuvel wrote:
>> Now that Itanium support has been dropped, we can remove the various
>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 47 ++++----------------
>> SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c | 27 +++--------
>> SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c | 27 +++--------
>> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 25 +++--------
>> 4 files changed, 25 insertions(+), 101 deletions(-)
>>
>> diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> index 0f795c0af125..66d96a9396b9 100644
>> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> @@ -295,7 +295,6 @@ HashPeImage (
>> )
>> {
>> BOOLEAN Status;
>> - UINT16 Magic;
>> EFI_IMAGE_SECTION_HEADER *Section;
>> VOID *HashCtx;
>> UINTN CtxSize;
>> @@ -367,33 +366,19 @@ HashPeImage (
>> // Measuring PE/COFF Image Header;
>> // But CheckSum field and SECURITY data directory (certificate) are excluded
>> //
>> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> - //
>> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
>> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> - //
>> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> - } else {
>> - //
>> - // Get the magic value from the PE/COFF Optional Header
>> - //
>> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
>> - }
>>
>> //
>> // 3. Calculate the distance from the base of the image header to the image checksum address.
>> // 4. Hash the image header from its base to beginning of the image checksum.
>> //
>> HashBase = mImageBase;
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) HashBase;
>> NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
>> - } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>> + } else if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>> //
>> // Use PE32+ offset.
>> //
>> @@ -420,7 +405,7 @@ HashPeImage (
>> // 6. Since there is no Cert Directory in optional header, hash everything
>> // from the end of the checksum to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -444,7 +429,7 @@ HashPeImage (
>> //
>> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -469,7 +454,7 @@ HashPeImage (
>> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
>> // 9. Hash everything from the end of the Cert Directory to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -494,7 +479,7 @@ HashPeImage (
>> //
>> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -577,7 +562,7 @@ HashPeImage (
>> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
>> CertSize = 0;
>> } else {
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -1583,7 +1568,6 @@ DxeImageVerificationHandler (
>> )
>> {
>> EFI_STATUS Status;
>> - UINT16 Magic;
>> EFI_IMAGE_DOS_HEADER *DosHdr;
>> EFI_STATUS VerifyStatus;
>> EFI_SIGNATURE_LIST *SignatureList;
>> @@ -1723,22 +1707,7 @@ DxeImageVerificationHandler (
>> goto Done;
>> }
>>
>> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> - //
>> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
>> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> - //
>> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> - } else {
>> - //
>> - // Get the magic value from the PE/COFF Optional Header
>> - //
>> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
>> - }
>> -
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
>> index c54ab62e2745..4e4a90f9da62 100644
>> --- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
>> +++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
>> @@ -320,7 +320,6 @@ TcgMeasurePeImage (
>> EFI_IMAGE_SECTION_HEADER *SectionHeader;
>> UINTN Index;
>> UINTN Pos;
>> - UINT16 Magic;
>> UINT32 EventSize;
>> UINT32 EventNumber;
>> EFI_PHYSICAL_ADDRESS EventLogLastEntry;
>> @@ -418,27 +417,13 @@ TcgMeasurePeImage (
>> // Measuring PE/COFF Image Header;
>> // But CheckSum field and SECURITY data directory (certificate) are excluded
>> //
>> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> - //
>> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
>> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> - //
>> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> - } else {
>> - //
>> - // Get the magic value from the PE/COFF Optional Header
>> - //
>> - Magic = Hdr.Pe32->OptionalHeader.Magic;
>> - }
>>
>> //
>> // 3. Calculate the distance from the base of the image header to the image checksum address.
>> // 4. Hash the image header from its base to beginning of the image checksum.
>> //
>> HashBase = (UINT8 *) (UINTN) ImageAddress;
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -465,7 +450,7 @@ TcgMeasurePeImage (
>> // 6. Since there is no Cert Directory in optional header, hash everything
>> // from the end of the checksum to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -489,7 +474,7 @@ TcgMeasurePeImage (
>> //
>> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -514,7 +499,7 @@ TcgMeasurePeImage (
>> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
>> // 9. Hash everything from the end of the Cert Directory to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -539,7 +524,7 @@ TcgMeasurePeImage (
>> //
>> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -621,7 +606,7 @@ TcgMeasurePeImage (
>> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
>> CertSize = 0;
>> } else {
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
>> index 29da2d70e699..61195e6041f7 100644
>> --- a/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
>> +++ b/SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c
>> @@ -116,7 +116,6 @@ MeasurePeImageAndExtend (
>> EFI_IMAGE_SECTION_HEADER *SectionHeader;
>> UINTN Index;
>> UINTN Pos;
>> - UINT16 Magic;
>> EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr;
>> UINT32 NumberOfRvaAndSizes;
>> UINT32 CertSize;
>> @@ -181,27 +180,13 @@ MeasurePeImageAndExtend (
>> // Measuring PE/COFF Image Header;
>> // But CheckSum field and SECURITY data directory (certificate) are excluded
>> //
>> - if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> - //
>> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
>> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> - //
>> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> - } else {
>> - //
>> - // Get the magic value from the PE/COFF Optional Header
>> - //
>> - Magic = Hdr.Pe32->OptionalHeader.Magic;
>> - }
>>
>> //
>> // 3. Calculate the distance from the base of the image header to the image checksum address.
>> // 4. Hash the image header from its base to beginning of the image checksum.
>> //
>> HashBase = (UINT8 *) (UINTN) ImageAddress;
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>
> I think this is a bug. Here "Magic" used to be set based on "Hdr.Pe32->OptionalHeader.Magic", not based on "mNtHeader.Pe32->OptionalHeader.Magic".
>
> And, I'm not convinced (mNtHeader.Pe32 == Hdr.Pe32). In earlier parts of this function, "Hdr.Pe32" is set as follows:
>
> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINT8 *) (UINTN) ImageAddress + PeCoffHeaderOffset);
>
> where "ImageAddress" is an input parameter to the function.
>
> I wonder why this code compiles; no "mNtHeader" declaration should be in scope here.
>
> $ git grep -w -l mNtHeader
>
> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
>
> More of the same below:
>
>> //
>> // Use PE32 offset
>> //
>> @@ -228,7 +213,7 @@ MeasurePeImageAndExtend (
>> // 6. Since there is no Cert Directory in optional header, hash everything
>> // from the end of the checksum to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -252,7 +237,7 @@ MeasurePeImageAndExtend (
>> //
>> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -277,7 +262,7 @@ MeasurePeImageAndExtend (
>> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
>> // 9. Hash everything from the end of the Cert Directory to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -302,7 +287,7 @@ MeasurePeImageAndExtend (
>> //
>> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -384,7 +369,7 @@ MeasurePeImageAndExtend (
>> if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
>> CertSize = 0;
>> } else {
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>
> Until here.
>
Ah yes. Thanks for spotting that.
> For build-testing, I suggest
>
> build -p SecurityPkg/SecurityPkg.dsc ...
>
> It will build all SecurityPkg modules, without putting them in a flash device (FD).
>
That is what I used to perform the build test, which completed without
error. But the logic is indeed incorrect.
> The rest looks good to me.
>
Thanks
>> diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
>> index 9acaa7b97507..f96325e978a5 100644
>> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
>> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
>> @@ -1831,7 +1831,6 @@ HashPeImage (
>> )
>> {
>> BOOLEAN Status;
>> - UINT16 Magic;
>> EFI_IMAGE_SECTION_HEADER *Section;
>> VOID *HashCtx;
>> UINTN CtxSize;
>> @@ -1874,27 +1873,13 @@ HashPeImage (
>> // Measuring PE/COFF Image Header;
>> // But CheckSum field and SECURITY data directory (certificate) are excluded
>> //
>> - if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> - //
>> - // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
>> - // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>> - // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> - // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> - //
>> - Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> - } else {
>> - //
>> - // Get the magic value from the PE/COFF Optional Header
>> - //
>> - Magic = mNtHeader.Pe32->OptionalHeader.Magic;
>> - }
>>
>> //
>> // 3. Calculate the distance from the base of the image header to the image checksum address.
>> // 4. Hash the image header from its base to beginning of the image checksum.
>> //
>> HashBase = mImageBase;
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -1915,7 +1900,7 @@ HashPeImage (
>> // 6. Get the address of the beginning of the Cert Directory.
>> // 7. Hash everything from the end of the checksum to the start of the Cert Directory.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -1937,7 +1922,7 @@ HashPeImage (
>> // 8. Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)
>> // 9. Hash everything from the end of the Cert Directory to the end of image header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset
>> //
>> @@ -1958,7 +1943,7 @@ HashPeImage (
>> //
>> // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
>> //
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>> @@ -2032,7 +2017,7 @@ HashPeImage (
>> //
>> if (mImageSize > SumOfBytesHashed) {
>> HashBase = mImageBase + SumOfBytesHashed;
>> - if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> + if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> //
>> // Use PE32 offset.
>> //
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 16:53 ` Laszlo Ersek
@ 2018-09-06 17:27 ` Ard Biesheuvel
2018-09-06 18:01 ` Carsey, Jaben
0 siblings, 1 reply; 18+ messages in thread
From: Ard Biesheuvel @ 2018-09-06 17:27 UTC (permalink / raw)
To: Laszlo Ersek
Cc: edk2-devel@lists.01.org, Liming Gao, Jiewen Yao, Star Zeng,
Michael D Kinney, Chao Zhang
On 6 September 2018 at 18:53, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/06/18 15:45, Ard Biesheuvel wrote:
>> Now that Itanium support has been dropped, we can remove the various
>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c | 60 +++-----------------
>> 1 file changed, 8 insertions(+), 52 deletions(-)
>
> Should we care about EdkCompatibilityPkg at all? Because:
>
> * IPF removal seems not to have occurred to EdkCompatibilityPkg:
>
> $ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
> [bunch of hits]
>
> * In <https://bugzilla.tianocore.org/show_bug.cgi?id=816#c7>, you wrote:
>
>> [...] there is a big difference between IPF drivers that are never
>> referenced by modern platforms, and workarounds in generic code that
>> are present in every modern build for every platform, and are only
>> intended for a specific build of ELILO.
>
>> The former is essentially dead code. The latter gets executed many
>> times on every boot of every modern UEFI platform in existence.
>
> Under that distinction, I would classify EdkCompatibilityPkg as the
> first category, i.e., essentially dead code.
>
OK, fair enough. I don't care about EdkCompatibilityPkg at all, I just
wanted to be thorough, but if others don't care either, I'll drop this
from v2.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 17:27 ` Ard Biesheuvel
@ 2018-09-06 18:01 ` Carsey, Jaben
2018-09-06 18:15 ` Laszlo Ersek
0 siblings, 1 reply; 18+ messages in thread
From: Carsey, Jaben @ 2018-09-06 18:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Laszlo Ersek, edk2-devel@lists.01.org, Gao, Liming, Yao, Jiewen,
Zeng, Star, Kinney, Michael D, Zhang, Chao B
We have a BZ to remove the compatibility package... I’d call updating it redundant and not worth your time.
Jaben
> On Sep 6, 2018, at 10:27 AM, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 6 September 2018 at 18:53, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 09/06/18 15:45, Ard Biesheuvel wrote:
>>> Now that Itanium support has been dropped, we can remove the various
>>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>>
>>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c | 60 +++-----------------
>>> 1 file changed, 8 insertions(+), 52 deletions(-)
>>
>> Should we care about EdkCompatibilityPkg at all? Because:
>>
>> * IPF removal seems not to have occurred to EdkCompatibilityPkg:
>>
>> $ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
>> [bunch of hits]
>>
>> * In <https://bugzilla.tianocore.org/show_bug.cgi?id=816#c7>, you wrote:
>>
>>> [...] there is a big difference between IPF drivers that are never
>>> referenced by modern platforms, and workarounds in generic code that
>>> are present in every modern build for every platform, and are only
>>> intended for a specific build of ELILO.
>>
>>> The former is essentially dead code. The latter gets executed many
>>> times on every boot of every modern UEFI platform in existence.
>>
>> Under that distinction, I would classify EdkCompatibilityPkg as the
>> first category, i.e., essentially dead code.
>>
>
> OK, fair enough. I don't care about EdkCompatibilityPkg at all, I just
> wanted to be thorough, but if others don't care either, I'll drop this
> from v2.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 18:01 ` Carsey, Jaben
@ 2018-09-06 18:15 ` Laszlo Ersek
0 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 18:15 UTC (permalink / raw)
To: Carsey, Jaben, Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Gao, Liming, Yao, Jiewen, Zeng, Star,
Kinney, Michael D, Zhang, Chao B
On 09/06/18 20:01, Carsey, Jaben wrote:
> We have a BZ to remove the compatibility package... I’d call updating it redundant and not worth your time.
Ah, very good point!
https://bugzilla.tianocore.org/show_bug.cgi?id=1103
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 17:25 ` Ard Biesheuvel
@ 2018-09-06 18:22 ` Laszlo Ersek
0 siblings, 0 replies; 18+ messages in thread
From: Laszlo Ersek @ 2018-09-06 18:22 UTC (permalink / raw)
To: Ard Biesheuvel, Jiewen Yao
Cc: edk2-devel@lists.01.org, Liming Gao, Star Zeng, Michael D Kinney,
Chao Zhang
On 09/06/18 19:25, Ard Biesheuvel wrote:
> On 6 September 2018 at 18:47, Laszlo Ersek <lersek@redhat.com> wrote:
>> For build-testing, I suggest
>>
>> build -p SecurityPkg/SecurityPkg.dsc ...
>>
>> It will build all SecurityPkg modules, without putting them in a flash device (FD).
>>
>
> That is what I used to perform the build test, which completed without
> error. [...]
That's spooky! In "SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c", no
declaration of "mNtHeader" should be visible. Hm...
Arrgh, if you check "SecurityPkg/SecurityPkg.dsc", the
"SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf" file is listed *only* under
[Components.IA32, Components.X64]
Thus, assuming you built natively on AARCH64, this module was not built. :/
Jiewen, is there any particular reason for not building Tcg2Dxe on
AARCH64? Is it just "historical lack of AARCH64 test environment
providing TPM2 hardware", or something more "architectural"?
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
2018-09-06 16:29 ` Laszlo Ersek
@ 2018-09-07 5:15 ` Zeng, Star
1 sibling, 0 replies; 18+ messages in thread
From: Zeng, Star @ 2018-09-07 5:15 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Gao, Liming, Yao, Jiewen, Kinney, Michael D, Laszlo Ersek,
Zhang, Chao B, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, September 6, 2018 9:45 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming <liming.gao@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [edk2] [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +-------------------
MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +----------
MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 17 +----------
MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +----------
MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c | 31 +-------------------
5 files changed, 5 insertions(+), 108 deletions(-)
diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
index 83ed43a16e95..ff1940431c2f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
+++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
@@ -254,35 +254,6 @@ GetMemoryProfileContext (
return mMemoryProfileContextPtr;
}
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
/**
Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
If Pe32Data is NULL, then ASSERT().
@@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
return Hdr.Te->Subsystem;
} else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
- Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
return Hdr.Pe32->OptionalHeader.Subsystem;
} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 4cd219c88efc..fa8f8fe91ac7 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -406,7 +406,6 @@ ProtectUefiImage (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
BOOLEAN IsAligned;
UINT32 ProtectionPolicy;
@@ -466,21 +465,7 @@ ProtectUefiImage (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a96d442fbc64..05eb4f422b2f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1076,7 +1076,6 @@ InsertImageRecord (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", RuntimeImage));
DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize));
@@ -1126,21 +1125,7 @@ InsertImageRecord (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
index 1682d0f9e404..157beb1e67ff 100644
--- a/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
@@ -1032,7 +1032,6 @@ SmmInsertImageRecord (
IMAGE_PROPERTIES_RECORD *ImageRecord;
CHAR8 *PdbPointer;
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
- UINT16 Magic;
DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%x\n", DriverEntry));
DEBUG ((DEBUG_VERBOSE, "SMM InsertImageRecord - 0x%016lx - 0x%08x\n", DriverEntry->ImageBuffer, DriverEntry->NumberOfPage));
@@ -1076,21 +1075,7 @@ SmmInsertImageRecord (
//
// Get SectionAlignment
//
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- } else {
- //
- // Get the magic value from the PE/COFF Optional Header
- //
- Magic = Hdr.Pe32->OptionalHeader.Magic;
- }
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
} else {
SectionAlignment = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
index b586afa7948e..e8d60c2d2357 100644
--- a/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
+++ b/MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c
@@ -255,35 +255,6 @@ GetSmramProfileContext (
return mSmramProfileContextPtr;
}
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic;
-}
-
/**
Retrieves and returns the Subsystem of a PE/COFF image that has been loaded into system memory.
If Pe32Data is NULL, then ASSERT().
@@ -320,7 +291,7 @@ InternalPeCoffGetSubsystem (
if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
return Hdr.Te->Subsystem;
} else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE) {
- Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+ Magic = Hdr.Pe32->OptionalHeader.Magic;
if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
return Hdr.Pe32->OptionalHeader.Subsystem;
} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
--
2.18.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF
2018-09-06 13:45 ` [PATCH 2/4] MdePkg/BasePeCoffLib: " Ard Biesheuvel
2018-09-06 16:31 ` Laszlo Ersek
@ 2018-09-07 5:15 ` Zeng, Star
2018-09-07 5:36 ` Gao, Liming
1 sibling, 1 reply; 18+ messages in thread
From: Zeng, Star @ 2018-09-07 5:15 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Wang, Jian J, Kinney, Michael D, Gao, Liming, Zhang, Chao B,
Yao, Jiewen, Laszlo Ersek, Leif Lindholm, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, September 6, 2018 9:45 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF
Now that Itanium support has been dropped, we can remove the various occurrences of the ELILO on Itanium PE/COFF header workaround.
Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-----------------
1 file changed, 9 insertions(+), 52 deletions(-)
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 32eca0ad2ef4..c57816a80887 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
SectionHeader->PointerToRawData -= TeStrippedOffset; }
-/**
- Retrieves the magic value from the PE/COFF header.
-
- @param Hdr The buffer in which to return the PE32, PE32+, or TE header.
-
- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
- )
-{
- //
- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value
- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
- // Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
- // then override the returned value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
- //
- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
- }
- //
- // Return the magic value from the PC/COFF Optional Header
- //
- return Hdr.Pe32->OptionalHeader.Magic; -}
-
-
/**
Retrieves the PE or TE Header from a PE/COFF or TE image.
@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
EFI_IMAGE_DOS_HEADER DosHdr;
UINTN Size;
UINTN ReadSize;
- UINT16 Magic;
UINT32 SectionHeaderOffset;
UINT32 Index;
UINT32 HeaderWithoutDataDir;
@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
ImageContext->IsTeImage = FALSE;
ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
//
@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
ImageContext->SectionAlignment = Hdr.Pe32->OptionalHeader.SectionAlignment;
ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
- } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+ } else if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
//
// 1. Check FileHeader.NumberOfRvaAndSizes filed.
//
@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
EFI_IMAGE_SECTION_HEADER SectionHeader;
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY DebugEntry;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
UINT32 TeStrippedOffset;
if (ImageContext == NULL) {
@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
return Status;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
//
// Retrieve the base address of the image
//
if (!(ImageContext->IsTeImage)) {
TeStrippedOffset = 0;
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
}
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
CHAR8 *FixupData;
PHYSICAL_ADDRESS BaseAddress;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
UINT32 TeStrippedOffset;
ASSERT (ImageContext != NULL);
@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
if (!(ImageContext->IsTeImage)) {
Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + ImageContext->PeCoffHeaderOffset);
TeStrippedOffset = 0;
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1230,7 +1192,6 @@ PeCoffLoaderLoadImage (
UINTN Size;
UINT32 TempDebugEntryRva;
UINT32 NumberOfRvaAndSizes;
- UINT16 Magic;
EFI_IMAGE_RESOURCE_DIRECTORY *ResourceDirectory;
EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;
EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;
@@ -1404,12 +1365,11 @@ PeCoffLoaderLoadImage (
//
// Get image's entry point
//
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
if (!(ImageContext->IsTeImage)) {
//
// Sizes of AddressOfEntryPoint are different so we need to do this safely
//
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1444,7 +1404,7 @@ PeCoffLoaderLoadImage (
// the optional header to verify a desired directory entry is there.
//
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1565,7 +1525,7 @@ PeCoffLoaderLoadImage (
//
ImageContext->HiiResourceData = 0;
if (!(ImageContext->IsTeImage)) {
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
@@ -1721,7 +1681,6 @@ PeCoffLoaderRelocateImageForRuntime (
CHAR8 *FixupData;
UINTN Adjust;
RETURN_STATUS Status;
- UINT16 Magic;
OldBase = (CHAR8 *)((UINTN)ImageBase);
NewBase = (CHAR8 *)((UINTN)VirtImageBase); @@ -1750,9 +1709,7 @@ PeCoffLoaderRelocateImageForRuntime (
return ;
}
- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+ if (Hdr.Pe32->OptionalHeader.Magic ==
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
//
// Use PE32 offset
//
--
2.18.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF
2018-09-07 5:15 ` Zeng, Star
@ 2018-09-07 5:36 ` Gao, Liming
0 siblings, 0 replies; 18+ messages in thread
From: Gao, Liming @ 2018-09-07 5:36 UTC (permalink / raw)
To: Zeng, Star, Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Wang, Jian J, Kinney, Michael D, Zhang, Chao B, Yao, Jiewen,
Laszlo Ersek, Leif Lindholm
Reviewed-by: Liming Gao <liming.gao@intel.com>
>-----Original Message-----
>From: Zeng, Star
>Sent: Friday, September 07, 2018 1:15 PM
>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
>Cc: Wang, Jian J <jian.j.wang@intel.com>; Kinney, Michael D
><michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Zhang,
>Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
>Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif.lindholm@linaro.org>;
>Zeng, Star <star.zeng@intel.com>
>Subject: RE: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header
>workaround for ELILO on IPF
>
>Reviewed-by: Star Zeng <star.zeng@intel.com>
>
>-----Original Message-----
>From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>Sent: Thursday, September 6, 2018 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Zeng, Star
><star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Kinney,
>Michael D <michael.d.kinney@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Yao,
>Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Leif
>Lindholm <leif.lindholm@linaro.org>
>Subject: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header
>workaround for ELILO on IPF
>
>Now that Itanium support has been dropped, we can remove the various
>occurrences of the ELILO on Itanium PE/COFF header workaround.
>
>Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>---
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-----------------
> 1 file changed, 9 insertions(+), 52 deletions(-)
>
>diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>index 32eca0ad2ef4..c57816a80887 100644
>--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
> SectionHeader->PointerToRawData -= TeStrippedOffset; }
>
>-/**
>- Retrieves the magic value from the PE/COFF header.
>-
>- @param Hdr The buffer in which to return the PE32, PE32+, or TE
>header.
>-
>- @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
>- @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
>-
>-**/
>-UINT16
>-PeCoffLoaderGetPeHeaderMagicValue (
>- IN EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION Hdr
>- )
>-{
>- //
>- // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic
>value
>- // in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
>- // Magic value in the OptionalHeader is
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>- // then override the returned value to
>EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>- //
>- if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
>Hdr.Pe32->OptionalHeader.Magic ==
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>- return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>- }
>- //
>- // Return the magic value from the PC/COFF Optional Header
>- //
>- return Hdr.Pe32->OptionalHeader.Magic; -}
>-
>-
> /**
> Retrieves the PE or TE Header from a PE/COFF or TE image.
>
>@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
> EFI_IMAGE_DOS_HEADER DosHdr;
> UINTN Size;
> UINTN ReadSize;
>- UINT16 Magic;
> UINT32 SectionHeaderOffset;
> UINT32 Index;
> UINT32 HeaderWithoutDataDir;
>@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
> ImageContext->IsTeImage = FALSE;
> ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
>
>- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>-
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
> //
>@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
> ImageContext->SectionAlignment = Hdr.Pe32-
>>OptionalHeader.SectionAlignment;
> ImageContext->SizeOfHeaders = Hdr.Pe32-
>>OptionalHeader.SizeOfHeaders;
>
>- } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>+ } else if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> //
> // 1. Check FileHeader.NumberOfRvaAndSizes filed.
> //
>@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
> EFI_IMAGE_SECTION_HEADER SectionHeader;
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY DebugEntry;
> UINT32 NumberOfRvaAndSizes;
>- UINT16 Magic;
> UINT32 TeStrippedOffset;
>
> if (ImageContext == NULL) {
>@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
> return Status;
> }
>
>- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>-
> //
> // Retrieve the base address of the image
> //
> if (!(ImageContext->IsTeImage)) {
> TeStrippedOffset = 0;
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
> }
>
> if (!(ImageContext->IsTeImage)) {
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
> CHAR8 *FixupData;
> PHYSICAL_ADDRESS BaseAddress;
> UINT32 NumberOfRvaAndSizes;
>- UINT16 Magic;
> UINT32 TeStrippedOffset;
>
> ASSERT (ImageContext != NULL);
>@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
> if (!(ImageContext->IsTeImage)) {
> Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext-
>>ImageAddress + ImageContext->PeCoffHeaderOffset);
> TeStrippedOffset = 0;
>- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -1230,7 +1192,6 @@ PeCoffLoaderLoadImage (
> UINTN Size;
> UINT32 TempDebugEntryRva;
> UINT32 NumberOfRvaAndSizes;
>- UINT16 Magic;
> EFI_IMAGE_RESOURCE_DIRECTORY *ResourceDirectory;
> EFI_IMAGE_RESOURCE_DIRECTORY_ENTRY *ResourceDirectoryEntry;
> EFI_IMAGE_RESOURCE_DIRECTORY_STRING *ResourceDirectoryString;
>@@ -1404,12 +1365,11 @@ PeCoffLoaderLoadImage (
> //
> // Get image's entry point
> //
>- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> if (!(ImageContext->IsTeImage)) {
> //
> // Sizes of AddressOfEntryPoint are different so we need to do this safely
> //
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -1444,7 +1404,7 @@ PeCoffLoaderLoadImage (
> // the optional header to verify a desired directory entry is there.
> //
> if (!(ImageContext->IsTeImage)) {
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -1565,7 +1525,7 @@ PeCoffLoaderLoadImage (
> //
> ImageContext->HiiResourceData = 0;
> if (!(ImageContext->IsTeImage)) {
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>@@ -1721,7 +1681,6 @@ PeCoffLoaderRelocateImageForRuntime (
> CHAR8 *FixupData;
> UINTN Adjust;
> RETURN_STATUS Status;
>- UINT16 Magic;
>
> OldBase = (CHAR8 *)((UINTN)ImageBase);
> NewBase = (CHAR8 *)((UINTN)VirtImageBase); @@ -1750,9 +1709,7 @@
>PeCoffLoaderRelocateImageForRuntime (
> return ;
> }
>
>- Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>-
>- if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+ if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> //
> // Use PE32 offset
> //
>--
>2.18.0
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-09-07 5:36 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-06 13:45 [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Ard Biesheuvel
2018-09-06 13:45 ` [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF Ard Biesheuvel
2018-09-06 16:29 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
2018-09-06 13:45 ` [PATCH 2/4] MdePkg/BasePeCoffLib: " Ard Biesheuvel
2018-09-06 16:31 ` Laszlo Ersek
2018-09-07 5:15 ` Zeng, Star
2018-09-07 5:36 ` Gao, Liming
2018-09-06 13:45 ` [PATCH 3/4] SecurityPkg: " Ard Biesheuvel
2018-09-06 16:47 ` Laszlo Ersek
2018-09-06 17:25 ` Ard Biesheuvel
2018-09-06 18:22 ` Laszlo Ersek
2018-09-06 13:45 ` [PATCH 4/4] EdkCompatibilityPkg: " Ard Biesheuvel
2018-09-06 16:53 ` Laszlo Ersek
2018-09-06 17:27 ` Ard Biesheuvel
2018-09-06 18:01 ` Carsey, Jaben
2018-09-06 18:15 ` Laszlo Ersek
2018-09-06 14:05 ` [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack Yao, Jiewen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox