Re: [edk2-devel] [PATCH] MdePkg/BasePeCoffLib: Deal with broken debug directories

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 5 May 2023 at 03:56, Michael Kubacki
<mikuback@linux.microsoft.com> wrote:
>
> Acked-by: Michael Kubacki <michael.kubacki@microsoft.com>
>

Merged as #4340

Thanks all


> On 5/4/2023 10:47 AM, Ard Biesheuvel wrote:
> > Older versions of GenFw put the wrong value in the debug directory size
> > field in the PE/COFF header: instead of putting the combined size of all
> > the entries, it puts the size of the only entry it creates, but adds the
> > size of the NB10 payload that the entry points to. This confuses the
> > loader now that we started using additional debug directory entries to
> > describe DLL characteristics.
> >
> > GenFw was fixed in commit 60e85a39fe49071, but the binaries that were
> > generated with it still need to be supported.
> >
> > So let's detect this condition, and check whether the size of the debug
> > directory is consistent with the NB10 payload: if we should expect
> > additional directory entries where we observe the NB10 payload, the size
> > field is clearly wrong, and we can break from the loop.
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4425
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >   MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> >
> > diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> > index 4b71176a0c7c2ed0..27f8526370fa3859 100644
> > --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> > +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> > @@ -585,6 +585,7 @@ PeCoffLoaderGetImageInfo (
> >     UINTN                                Size;
> >
> >     UINTN                                ReadSize;
> >
> >     UINTN                                Index;
> >
> > +  UINTN                                NextIndex;
> >
> >     UINTN                                DebugDirectoryEntryRva;
> >
> >     UINTN                                DebugDirectoryEntryFileOffset;
> >
> >     UINTN                                SectionHeaderOffset;
> >
> > @@ -755,6 +756,19 @@ PeCoffLoaderGetImageInfo (
> >                 ImageContext->ImageSize += DebugEntry.SizeOfData;
> >
> >               }
> >
> >
> >
> > +            //
> >
> > +            // Implementations of GenFw before commit 60e85a39fe49071 will
> >
> > +            // concatenate the debug directory entry and the codeview entry,
> >
> > +            // and erroneously put the combined size into the debug directory
> >
> > +            // entry's size field. If this is the case, no other relevant
> >
> > +            // directory entries can exist, and we can terminate here.
> >
> > +            //
> >
> > +            NextIndex = Index + sizeof (EFI_IMAGE_DEBUG_DIRECTORY_ENTRY);
> >
> > +            if ((NextIndex < DebugDirectoryEntry->Size) &&
> >
> > +                (DebugEntry.FileOffset == (DebugDirectoryEntryFileOffset + NextIndex))) {
> >
> > +              break;
> >
> > +            }
> >
> > +
> >
> >               continue;
> >
> >             }
> >
> >
> >
>
>
> 
>
>