Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

["Gao, Liming" <liming.gao@intel.com>]

Andrew:
  Edk2 Build system calls GenFw to generate EFI image in build phase. Even if this image is not built into BIOS image, its EFI image will be generated by GenFw. So, only if this EFI image is built from EDK2 project, it can be updated by GenFw tool. You can see this step in build_rule.txt to convert .dll to .efi image.

Thanks
Liming
From: afish@apple.com [mailto:afish@apple.com]
Sent: Friday, August 11, 2017 12:59 AM
To: Gao, Liming <liming.gao@intel.com>
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5


On Aug 10, 2017, at 3:38 AM, Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>> wrote:

Andrew:
 If this is a mtoc bug, I suggest to update GenFw to always correct it in the generated EFI image. If so, the EFI image is always correct. There is no change requirement in PeCoff library in MdePkg.


Liming,

EFI supports loading PE/COFF images that are not built at the same time as the platform firmware (UEFI Shell, OS loader), and that is why I added the fix to the PE/COFF library code.

Thanks,

Andrew Fish


Thanks
Liming
From: afish@apple.com<mailto:afish@apple.com> [mailto:afish@apple.com]
Sent: Tuesday, August 8, 2017 12:26 AM
To: Zhu, Yonghong <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5

Should that be:
Contributed-under: TianoCore Contribution Agreement 1.1

I also noticed the PeCoff lib is going to loop and reload the .debug suction due to this mtoc bug, so it would be good to harden that code too.

git diff MdePkg/Library/BasePeCoffLib/BasePeCoff.c
diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 8d1daba..1e4c67e 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -771,6 +771,8 @@ PeCoffLoaderGetImageInfo (
            }

            return RETURN_SUCCESS;
+          } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+            return RETURN_SUCCESS;
          }
        }
      }
@@ -862,6 +864,8 @@ PeCoffLoaderGetImageInfo (
        if (DebugEntry.Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
          ImageContext->DebugDirectoryEntryRva = (UINT32) (DebugDirectoryEntryRva + Index);
          return RETURN_SUCCESS;
+        } else if (DebugEntry.Type == CODEVIEW_SIGNATURE_MTOC) {
+          return RETURN_SUCCESS;
        }
      }
    }



https://bugzilla.tianocore.org/show_bug.cgi?id=663
Contributed-under: TianoCore Contribution Agreement 1.1

Thanks,

Andrew Fish


On Aug 6, 2017, at 9:00 PM, Yonghong Zhu <yonghong.zhu@intel.com<mailto:yonghong.zhu@intel.com><mailto:yonghong.zhu@intel.com>> wrote:

it is a bug in mtoc setting the size of the debug directory entry to
the size of the .debug section, not the size of the
EFI_IMAGE_DEBUG_DIRECTORY_ENTRY. It was causing a loop to iterate and
get bogus EFI_IMAGE_DEBUG_DIRECTORY_ENTRY data and pass that to
memset() and boom.

Cc: Liming Gao <liming.gao@intel.com<mailto:liming.gao@intel.com><mailto:liming.gao@intel.com>>
Cc: Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com><mailto:michael.d.kinney@intel.com>>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrew Fish <afish@apple.com<mailto:afish@apple.com><mailto:afish@apple.com>>
---
BaseTools/Source/C/GenFw/GenFw.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
index 246deb0..af60c92 100644
--- a/BaseTools/Source/C/GenFw/GenFw.c
+++ b/BaseTools/Source/C/GenFw/GenFw.c
@@ -2813,10 +2813,11 @@ Returns:
 //
 // Get Debug, Export and Resource EntryTable RVA address.
 // Resource Directory entry need to review.
 //
 Optional32Hdr = (EFI_IMAGE_OPTIONAL_HEADER32 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
+  Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
 if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional32Hdr +  FileHdr->SizeOfOptionalHeader);
   if (Optional32Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
       Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
     ExportDirectoryEntryRva = Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
@@ -2833,11 +2834,10 @@ Returns:
       Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = 0;
       Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].VirtualAddress = 0;
     }
   }
 } else {
-    Optional64Hdr = (EFI_IMAGE_OPTIONAL_HEADER64 *) ((UINT8*) FileHdr + sizeof (EFI_IMAGE_FILE_HEADER));
   SectionHeader = (EFI_IMAGE_SECTION_HEADER *) ((UINT8 *) Optional64Hdr +  FileHdr->SizeOfOptionalHeader);
   if (Optional64Hdr->NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_EXPORT && \
       Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].Size != 0) {
     ExportDirectoryEntryRva = Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_EXPORT].VirtualAddress;
   }
@@ -2907,10 +2907,20 @@ Returns:
         RsdsEntry->Unknown  = 0;
         RsdsEntry->Unknown2 = 0;
         RsdsEntry->Unknown3 = 0;
         RsdsEntry->Unknown4 = 0;
         RsdsEntry->Unknown5 = 0;
+        } else if (RsdsEntry->Signature == CODEVIEW_SIGNATURE_MTOC) {
+          // MTOC sets DebugDirectoryEntrySize to size of the .debug section, so fix it.
+          if (!ZeroDebugFlag) {
+            if (Optional32Hdr->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+              Optional32Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+            } else {
+              Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size = sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
+            }
+          }
+          break;
       }
     }
   }
 }

--
2.6.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel