From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mout.perfora.net (mout.perfora.net [74.208.4.196]) by mx.groups.io with SMTP id smtpd.web10.47513.1679501085939393145 for ; Wed, 22 Mar 2023 09:04:46 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: smith-denny.com, ip: 74.208.4.196, mailfrom: osd@smith-denny.com) Received: from [10.137.194.171] ([131.107.8.107]) by mrelay.perfora.net (mreueus002 [74.208.5.2]) with ESMTPSA (Nemesis) id 0MYh3k-1q0Vqp2fFy-00VLmg; Wed, 22 Mar 2023 17:04:37 +0100 Message-ID: Date: Wed, 22 Mar 2023 09:04:36 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: Re: [edk2-devel] [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Umang Patel , Jiewen Yao , Jian J Wang References: <20230322020626.441-1-michael.d.kinney@intel.com> <20230322020626.441-3-michael.d.kinney@intel.com> From: "Oliver Smith-Denny" In-Reply-To: <20230322020626.441-3-michael.d.kinney@intel.com> X-Provags-ID: V03:K1:SBf+4hQXGN1Q7Q6LYsWplGySRIfFabk8nRFseGlwkL9BFX3Y3od CB5PeUKVI5MH4P7igqe22n15INcMrELQraW1lcXegt6ssX7KhJDp4+JqGDHFFlfgUq6bP6n cFQocEH3VmSc8NxbT1+gkQpG2UTFCeRiPaafKhZWTMbWxbANQHAkF0v6IjIxJz5cTQregSf 2oTxum7flCd5/sVPB13ig== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:cugqNiESyQ4=;YiVi+yuKgRXXH9m8Tzpcv3A6svm nvzL+zgxEgGgVtcLLxbeveUki3e3l9wpq0IO9+jqCX2FfnLV52vuj/Cgjzg1chu7H723L2fVR xE5h/LHvtI+wyGFhYiSBGTdtai3kMNmIFno5xwJhDBkhnh54DCLxMUiumAlAo5qI+kgasVx0o v81G0V6fJEOlPwpRdXiVcQN3hZ8FtxutcGJQq7HLELIn9s52sT5L6/JaFhiRu6tV+eazJb7xs 3wOvW9SIgPB06gifmWUyUILDRGI93UEl5d9iQvx9+g0ysKJbYKSW7arr9xI3o37MXs8Gd15bC weB+Way4sxjh8M9v9ePsF4Z/TbbE1f2xL8BgD3A6zy6sOCmIBaYtYosdHz8zpzqWDyZH6BIN+ yPD9liOYu3g0k3QbwDMfi6fh1S2Zk4ooajipkKIk6pWGc2eZQjUNubGmRPqhLYj9qN3/7M/Ab tdhXMwEWlg8XPu0XCWNsfsPNNiMKmD56iK/BjC8f7rq1TQ82woqrOTaCKByXsFnzKWa+p9hnd yUd6TeWrBLbCJ1v4uNu8PW3/noGC70xWKpkdHv+Mii9yntHQ/2/CfIVQ7wf9EUnHHHDkG61cu fQypgWYMdEsqBecTxYZq7Al+Er26LIK1nnn9Abgl+zzKqIow0/pgsm9NKGNY/hEkSVl/g6hRo 8bjbro5B2su2STMIG+jDmUZ8Is0PCz4gGCFmygyEFA== Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit One comment below, thanks! On 3/21/2023 7:06 PM, Michael D Kinney wrote: > From: Umang Patel > > If FirmwareVolumeShadow PPI is available, then use it to > shadow FVs to memory. Otherwise fallback to CopyMem(). > > Cc: Jiewen Yao > Cc: Jian J Wang > Signed-off-by: Patel Umang > --- > SecurityPkg/FvReportPei/FvReportPei.c | 37 ++++++++++++++++++++----- > SecurityPkg/FvReportPei/FvReportPei.h | 1 + > SecurityPkg/FvReportPei/FvReportPei.inf | 1 + > 3 files changed, 32 insertions(+), 7 deletions(-) > > diff --git a/SecurityPkg/FvReportPei/FvReportPei.c b/SecurityPkg/FvReportPei/FvReportPei.c > index 846605cda1e4..6288dde16b2a 100644 > --- a/SecurityPkg/FvReportPei/FvReportPei.c > +++ b/SecurityPkg/FvReportPei/FvReportPei.c > @@ -114,12 +114,13 @@ VerifyHashedFv ( > IN EFI_BOOT_MODE BootMode > ) > { > - UINTN FvIndex; > - CONST HASH_ALG_INFO *AlgInfo; > - UINT8 *HashValue; > - UINT8 *FvHashValue; > - VOID *FvBuffer; > - EFI_STATUS Status; > + UINTN FvIndex; > + CONST HASH_ALG_INFO *AlgInfo; > + UINT8 *HashValue; > + UINT8 *FvHashValue; > + VOID *FvBuffer; > + EDKII_PEI_FIRMWARE_VOLUME_SHADOW_PPI *FvShadowPpi; > + EFI_STATUS Status; > > if ((HashInfo == NULL) || > (HashInfo->HashSize == 0) || > @@ -191,8 +192,30 @@ VerifyHashedFv ( > // Copy FV to permanent memory to avoid potential TOC/TOU. > // > FvBuffer = AllocatePages (EFI_SIZE_TO_PAGES ((UINTN)FvInfo[FvIndex].Length)); > + > ASSERT (FvBuffer != NULL); While we are here, should we make this more robust (and amenable to static analysis) and add error handling if FvBuffer is NULL, not just assert? > - CopyMem (FvBuffer, (CONST VOID *)(UINTN)FvInfo[FvIndex].Base, (UINTN)FvInfo[FvIndex].Length); > + Status = PeiServicesLocatePpi ( > + &gEdkiiPeiFirmwareVolumeShadowPpiGuid, > + 0, > + NULL, > + (VOID **)&FvShadowPpi > + ); > + > + if (!EFI_ERROR (Status)) { > + Status = FvShadowPpi->FirmwareVolumeShadow ( > + (EFI_PHYSICAL_ADDRESS)FvInfo[FvIndex].Base, > + FvBuffer, > + (UINTN)FvInfo[FvIndex].Length > + ); > + } > + > + if (EFI_ERROR (Status)) { > + CopyMem ( > + FvBuffer, > + (CONST VOID *)(UINTN)FvInfo[FvIndex].Base, > + (UINTN)FvInfo[FvIndex].Length > + ); > + } > > if (!AlgInfo->HashAll (FvBuffer, (UINTN)FvInfo[FvIndex].Length, FvHashValue)) { > Status = EFI_ABORTED; > diff --git a/SecurityPkg/FvReportPei/FvReportPei.h b/SecurityPkg/FvReportPei/FvReportPei.h > index 92504a3c51e1..07ffb2f5768c 100644 > --- a/SecurityPkg/FvReportPei/FvReportPei.h > +++ b/SecurityPkg/FvReportPei/FvReportPei.h > @@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > > #include > +#include > > #include > #include > diff --git a/SecurityPkg/FvReportPei/FvReportPei.inf b/SecurityPkg/FvReportPei/FvReportPei.inf > index 408406889765..4246fb75ebaa 100644 > --- a/SecurityPkg/FvReportPei/FvReportPei.inf > +++ b/SecurityPkg/FvReportPei/FvReportPei.inf > @@ -46,6 +46,7 @@ [LibraryClasses] > [Ppis] > gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid ## PRODUCES > gEdkiiPeiFirmwareVolumeInfoStoredHashFvPpiGuid ## CONSUMES > + gEdkiiPeiFirmwareVolumeShadowPpiGuid ## CONSUMES > > [Pcd] > gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass