From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.17760.1683251791339246985 for ; Thu, 04 May 2023 18:56:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=QgGyTUf6; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 6494720EA252; Thu, 4 May 2023 18:56:30 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 6494720EA252 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1683251790; bh=Rbol8noEGX9KKV9uG2QEo6BnKlfMACCxy6xfgLR0Y2k=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=QgGyTUf6n1HA0Q2LMDOIDQ5wqvvbAk0fnHlosUpMCrLs/pCzGLNdC8K8YC4PHLS2F BOIymqmMEYYR16Zd5M8ONhBjry/GgP5Kuo0e+HIBRDmz7HB0wnmonlfmCuujtKvp5e RrypZs7ahKq899puB1p2rfu5nyddahz2xJko5njA= Message-ID: <7122d2bc-2e7e-d189-1cee-adb2465621d7@linux.microsoft.com> Date: Thu, 4 May 2023 21:56:29 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [edk2-devel] [PATCH] MdePkg/BasePeCoffLib: Deal with broken debug directories To: devel@edk2.groups.io, ardb@kernel.org Cc: gaoliming@byosoft.com.cn, michael.kubacki@microsoft.com References: <20230504144739.3912103-1-ardb@kernel.org> From: "Michael Kubacki" In-Reply-To: <20230504144739.3912103-1-ardb@kernel.org> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Acked-by: Michael Kubacki 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 > --- > 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; > > } > > >