From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by mx.groups.io with SMTP id smtpd.web10.22216.1683274346840849942 for ; Fri, 05 May 2023 01:12:27 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=AYXgv301; spf=pass (domain: kernel.org, ip: 139.178.84.217, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1E83261778 for ; Fri, 5 May 2023 08:12:24 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8336DC4339C for ; Fri, 5 May 2023 08:12:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1683274343; bh=P4uyETh1OHOqJYIXui6+vmUy28tgGPTinqzWAJjyVDk=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=AYXgv301hakWmFeAUSVXuqS3yrmtybE518LvvIKDb2whOYu/Cc30m0vpOqftk9nPk F8i5xYdhicwKgxs0MrKKoBrFTmetumalLf9DH5+CAG0CtOaFKdpK0LoycJtO50zA9t Ct4dfShwXfzz1mbFzbleZ4Trh53VcHJ8fsQWA/qdvb++Upp+BAued4s2u27xIOuSLX ErC0SVzGZUNBRGwVqIOCg+9temYZq+g3IU5hWoNq1KjdKpZ2a6/vxWyPz08U82bwS3 9zuRKmda0RlJPNtYLqESxZe1TrjInP/TKdXsyUtqLkTMAJz2rFNx6AqgTRPj4qRa1f 81WXPvDwN2gGg== Received: by mail-lj1-f180.google.com with SMTP id 38308e7fff4ca-2ac811c6691so15361131fa.3 for ; Fri, 05 May 2023 01:12:23 -0700 (PDT) X-Gm-Message-State: AC+VfDyIPgiqzyvuFEB50Jh5YiYUaAm7IcHNwm2G+bkNRp5J/wOzEtTZ apHvTr/wvl5uoizHiH6/NWdpQMx7GeAo2B6D9C8= X-Google-Smtp-Source: ACHHUZ46PbDTn2oneQe3Q+hS7DXw3giJ902DbIt5OHhA/PXYrKK39L3q3hHRuLFeZBm7vZm76SqXW14Nv+Qgmgpfb8A= X-Received: by 2002:a2e:4942:0:b0:2ac:7904:e38f with SMTP id b2-20020a2e4942000000b002ac7904e38fmr163152ljd.12.1683274341465; Fri, 05 May 2023 01:12:21 -0700 (PDT) MIME-Version: 1.0 References: <20230504144739.3912103-1-ardb@kernel.org> <7122d2bc-2e7e-d189-1cee-adb2465621d7@linux.microsoft.com> In-Reply-To: <7122d2bc-2e7e-d189-1cee-adb2465621d7@linux.microsoft.com> From: "Ard Biesheuvel" Date: Fri, 5 May 2023 10:12:10 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [PATCH] MdePkg/BasePeCoffLib: Deal with broken debug directories To: devel@edk2.groups.io, mikuback@linux.microsoft.com Cc: gaoliming@byosoft.com.cn, michael.kubacki@microsoft.com Content-Type: text/plain; charset="UTF-8" On Fri, 5 May 2023 at 03:56, Michael Kubacki wrote: > > Acked-by: Michael Kubacki > 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 > > --- > > 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; > > > > } > > > > > > > > > > >