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

["Michael Kubacki" <mikuback@linux.microsoft.com>]

Acked-by: Michael Kubacki <michael.kubacki@microsoft.com>

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;
> 
>             }
> 
>   
>