On 5 July 2017 at 17:42, Laszlo Ersek <ler...@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.biesheu...@linaro.org> > Cc: Gerd Hoffmann <kra...@redhat.com> > Cc: Leif Lindholm <leif.lindh...@linaro.org> > Cc: Liming Gao <liming....@intel.com> > Cc: Yonghong Zhu <yonghong....@intel.com> > Reported-by: Gerd Hoffmann <kra...@redhat.com> > Reported-by: Leif Lindholm <leif.lindh...@linaro.org> > Ref: > a1de67a8-57c2-908e-dd4d-9726d60fb388@redhat.com">http://mid.mail-archive.com/a1de67a8-57c2-908e-dd4d-9726d60fb388@redhat.com > Ref: 20170705134136.GB26676@bivouac.eciton.net">http://mid.mail-archive.com/20170705134136.GB26676@bivouac.eciton.net > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <ler...@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 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel