public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Andrew Fish <afish@apple.com>
To: "Gao, Liming" <liming.gao@intel.com>
Cc: "Zhu, Yonghong" <yonghong.zhu@intel.com>,
	Mike Kinney <michael.d.kinney@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5
Date: Thu, 10 Aug 2017 09:59:10 -0700	[thread overview]
Message-ID: <2AC68977-AB40-45DC-B97D-27FFCE19C155@apple.com> (raw)
In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14D76DAD8@shsmsx102.ccr.corp.intel.com>


> On Aug 10, 2017, at 3:38 AM, Gao, Liming <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 <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 <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>>
> Cc: Michael D Kinney <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>>
> ---
> 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
> https://lists.01.org/mailman/listinfo/edk2-devel



  reply	other threads:[~2017-08-10 16:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-07  4:00 [Patch] BaseTools: Fix Segmentation fault: 11 when build AppPkg with XCODE5 Yonghong Zhu
2017-08-07 16:26 ` Andrew Fish
2017-08-08  1:14   ` Zhu, Yonghong
2017-08-10 10:38   ` Gao, Liming
2017-08-10 16:59     ` Andrew Fish [this message]
2017-08-11  4:48       ` Gao, Liming
2017-08-11  5:07         ` Andrew Fish
2017-08-11  5:49           ` Gao, Liming
2017-08-11 16:50             ` Andrew Fish
2017-08-14  2:13               ` Gao, Liming
2017-08-14  7:10 ` Gao, Liming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2AC68977-AB40-45DC-B97D-27FFCE19C155@apple.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox