From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
Gerd Hoffmann <kraxel@redhat.com>,
Leif Lindholm <leif.lindholm@linaro.org>,
Liming Gao <liming.gao@intel.com>,
Yonghong Zhu <yonghong.zhu@intel.com>
Subject: Re: [PATCH] BaseTools/GenFw: work around GNU Binutils bug wrt. DebugDirectoryEntrySize
Date: Wed, 5 Jul 2017 17:45:57 +0100 [thread overview]
Message-ID: <CAKv+Gu_B0PMdJwXJ3awKyHo5K07QM2Eftx9aJo5zu5hGgOqc6w@mail.gmail.com> (raw)
In-Reply-To: <20170705164218.25814-1-lersek@redhat.com>
On 5 July 2017 at 17:42, Laszlo Ersek <lersek@redhat.com> wrote:
> GNU Binutils produce a PE debug directory with one
This sentence already confuses me. This crash is reproducible on ARM,
but the ARM toolchains are strictly ELF based, and all PE/COFF data
structures are created by GenFw itself, never by binutils. So I don't
see how this could be a binutils bug.
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY:
> - the Type field of the entry is EFI_IMAGE_DEBUG_TYPE_CODEVIEW,
> - the FileOffset field of the entry points right past the entry itself,
> - the data structure placed at FileOffset is a CV_INFO_PDB20 structure,
> with an "NB10" signature.
>
> This is all correct, except GNU Binutils include the pointed-to
> CV_INFO_PDB20 structure in the size of the debug directory (that is,
> Optional64Hdr->DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_DEBUG].Size).
> That's a bug.
>
> The malformed debug directory size causes the loop in GenFw's
> ZeroDebugData() function to process the CV_INFO_PDB20 structure as a set
> of EFI_IMAGE_DEBUG_DIRECTORY_ENTRY elements, which crashes GenFw.
>
> This problem was exposed by commit e4129b0e5897 ("BaseTools: Update GenFw
> to clear unused debug entry generated by VS tool chain", 2017-06-19).
>
> Work around the Binutils issue by noticing when an
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset points back into the debug
> directory. (This can never happen with a well-formed PE file.) In this
> case, truncate DebugDirectoryEntrySize such that the debug directory will
> end right before the debug structure pointed-to by
> EFI_IMAGE_DEBUG_DIRECTORY_ENTRY.FileOffset.
>
> Tested with OVMF:
> - gcc-4.8.5-14.el7.x86_64
> - binutils-2.25.1-27.base.el7.x86_64
>
> and with ArmVirtPkg:
> - gcc-aarch64-linux-gnu-6.1.1-2.el7.x86_64
> - binutils-aarch64-linux-gnu-2.27-3.el7.x86_64
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> Reported-by: Gerd Hoffmann <kraxel@redhat.com>
> Reported-by: Leif Lindholm <leif.lindholm@linaro.org>
> Ref: http://mid.mail-archive.com/a1de67a8-57c2-908e-dd4d-9726d60fb388@redhat.com
> Ref: http://mid.mail-archive.com/20170705134136.GB26676@bivouac.eciton.net
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> Repo: https://github.com/lersek/edk2.git
> Branch: binutils_debugdirsize_workaround
>
> BaseTools/Source/C/GenFw/GenFw.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/BaseTools/Source/C/GenFw/GenFw.c b/BaseTools/Source/C/GenFw/GenFw.c
> index 6569460f34f7..a79f485ee681 100644
> --- a/BaseTools/Source/C/GenFw/GenFw.c
> +++ b/BaseTools/Source/C/GenFw/GenFw.c
> @@ -2771,6 +2771,7 @@ Returns:
> UINT32 Index;
> UINT32 DebugDirectoryEntryRva;
> UINT32 DebugDirectoryEntrySize;
> + UINT32 TruncatedDebugDirectorySize;
> UINT32 DebugDirectoryEntryFileOffset;
> UINT32 ExportDirectoryEntryRva;
> UINT32 ExportDirectoryEntryFileOffset;
> @@ -2893,6 +2894,25 @@ Returns:
> DebugEntry = (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY *) (FileBuffer + DebugDirectoryEntryFileOffset);
> Index = 0;
> for (Index=0; Index < DebugDirectoryEntrySize / sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY); Index ++, DebugEntry ++) {
> + //
> + // Work around GNU Binutils bug: if the debug information pointed-to by
> + // DebugEntry was incorrectly included in DebugDirectoryEntrySize, then
> + // the debug directory doesn't actually extend past the pointed-to debug
> + // information. Truncate DebugDirectoryEntrySize accordingly.
> + //
> + if (DebugEntry->FileOffset >= DebugDirectoryEntryFileOffset &&
> + DebugEntry->FileOffset < (DebugDirectoryEntryFileOffset +
> + DebugDirectoryEntrySize)) {
> + TruncatedDebugDirectorySize = (DebugEntry->FileOffset -
> + DebugDirectoryEntryFileOffset);
> + VerboseMsg (
> + "truncating debug directory size from %u to %u",
> + DebugDirectoryEntrySize,
> + TruncatedDebugDirectorySize
> + );
> + DebugDirectoryEntrySize = TruncatedDebugDirectorySize;
> + }
> +
> DebugEntry->TimeDateStamp = 0;
> if (ZeroDebugFlag || DebugEntry->Type != EFI_IMAGE_DEBUG_TYPE_CODEVIEW) {
> memset (FileBuffer + DebugEntry->FileOffset, 0, DebugEntry->SizeOfData);
> --
> 2.13.1.3.g8be5a757fa67
>
next prev parent reply other threads:[~2017-07-05 16:44 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-05 16:42 [PATCH] BaseTools/GenFw: work around GNU Binutils bug wrt. DebugDirectoryEntrySize Laszlo Ersek
2017-07-05 16:45 ` Ard Biesheuvel [this message]
2017-07-05 17:28 ` Laszlo Ersek
2017-07-05 17:33 ` Ard Biesheuvel
2017-07-05 17:49 ` Laszlo Ersek
2017-07-05 17:33 ` Laszlo Ersek
2017-07-05 17:37 ` Ard Biesheuvel
2017-07-05 17:52 ` Laszlo Ersek
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=CAKv+Gu_B0PMdJwXJ3awKyHo5K07QM2Eftx9aJo5zu5hGgOqc6w@mail.gmail.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