Guomin, Some typo or syntax issues below.
> -----Original Message----- > From: Jiang, Guomin <guomin.ji...@intel.com> > Sent: Thursday, July 09, 2020 9:57 AM > To: devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; > Bi, Dandan <dandan...@intel.com>; Gao, Liming <liming....@intel.com>; De, > Debkumar <debkumar...@intel.com>; Han, Harry <harry....@intel.com>; > West, Catharine <catharine.w...@intel.com>; Laszlo Ersek <ler...@redhat.com> > Subject: [PATCH v5 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for > calculating hash (CVE-2019-11098) > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614 > > When we allocate pool to save the rebased PEIMs, the address will change > randomly, therefore the hash will change and result PCR0 change as well. > To avoid this, we save the raw PEIMs and use it to calculate hash. > > The MigratedFvInfo HOB will never produce when > PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD control > the total feature. > > Cc: Jian J Wang <jian.j.w...@intel.com> > Cc: Hao A Wu <hao.a...@intel.com> > Cc: Dandan Bi <dandan...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Cc: Debkumar De <debkumar...@intel.com> > Cc: Harry Han <harry....@intel.com> > Cc: Catharine West <catharine.w...@intel.com> > Signed-off-by: Guomin Jiang <guomin.ji...@intel.com> > Acked-by: Laszlo Ersek <ler...@redhat.com> > --- > MdeModulePkg/MdeModulePkg.dec | 3 ++ > MdeModulePkg/Core/Pei/PeiMain.inf | 1 + > MdeModulePkg/Core/Pei/PeiMain.h | 1 + > MdeModulePkg/Include/Guid/MigratedFvInfo.h | 22 +++++++++++++++ > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 28 +++++++++++++++++++ > 5 files changed, 55 insertions(+) > create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h > > diff --git a/MdeModulePkg/MdeModulePkg.dec > b/MdeModulePkg/MdeModulePkg.dec > index 16db17d0a873..40197dba862c 100644 > --- a/MdeModulePkg/MdeModulePkg.dec > +++ b/MdeModulePkg/MdeModulePkg.dec > @@ -389,6 +389,9 @@ [Guids] > ## GUID indicates the capsule is to store Capsule On Disk file names. > gEdkiiCapsuleOnDiskNameGuid = { 0x98c80a4f, 0xe16b, 0x4d11, { 0x93, 0x9a, > 0xab, 0xe5, 0x61, 0x26, 0x3, 0x30 } } > > + ## Include/Guid/MigratedFvInfo.h > + gEdkiiMigratedFvInfoGuid = { 0xc1ab12f7, 0x74aa, 0x408d, { 0xa2, 0xf4, > 0xc6, > 0xce, 0xfd, 0x17, 0x98, 0x71 } } > + > [Ppis] > ## Include/Ppi/AtaController.h > gPeiAtaControllerPpiGuid = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, > 0x7a, > 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }} > diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf > b/MdeModulePkg/Core/Pei/PeiMain.inf > index 5b36d516b3fa..0cf357371a16 100644 > --- a/MdeModulePkg/Core/Pei/PeiMain.inf > +++ b/MdeModulePkg/Core/Pei/PeiMain.inf > @@ -77,6 +77,7 @@ [Guids] > ## CONSUMES ## GUID # Used to compare with FV's file system GUID and > get the FV's file system format > gEfiFirmwareFileSystem3Guid > gStatusCodeCallbackGuid > + gEdkiiMigratedFvInfoGuid ## SOMETIMES_PRODUCES ## > HOB > > [Ppis] > gEfiPeiStatusCodePpiGuid ## SOMETIMES_CONSUMES # > PeiReportStatusService is not ready if this PPI doesn't exist > diff --git a/MdeModulePkg/Core/Pei/PeiMain.h > b/MdeModulePkg/Core/Pei/PeiMain.h > index b0101dba5e30..cbf74d5b9d9a 100644 > --- a/MdeModulePkg/Core/Pei/PeiMain.h > +++ b/MdeModulePkg/Core/Pei/PeiMain.h > @@ -44,6 +44,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include <Guid/FirmwareFileSystem2.h> > #include <Guid/FirmwareFileSystem3.h> > #include <Guid/AprioriFileName.h> > +#include <Guid/MigratedFvInfo.h> > > /// > /// It is an FFS type extension used for PeiFindFileEx. It indicates current > diff --git a/MdeModulePkg/Include/Guid/MigratedFvInfo.h > b/MdeModulePkg/Include/Guid/MigratedFvInfo.h > new file mode 100644 > index 000000000000..061c17ed0e48 > --- /dev/null > +++ b/MdeModulePkg/Include/Guid/MigratedFvInfo.h > @@ -0,0 +1,22 @@ > +/** @file > + Migrated FV information > + > +Copyright (c) 2020, Intel Corporation. All rights reserved.<BR> > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__ > +#define __EDKII_MIGRATED_FV_INFO_GUID_H__ > + > +typedef struct { > + UINT32 FvOrgBase; // original FV address > + UINT32 FvNewBase; // new FV address > + UINT32 FvDataBase; // original FV data > + UINT32 FvLength; // Fv Length > +} EDKII_MIGRATED_FV_INFO; > + > +extern EFI_GUID gEdkiiMigratedFvInfoGuid; > + > +#endif // #ifndef __EDKII_MIGRATED_FV_INFO_GUID_H__ > + > diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > index ef88b3423376..f654cea15c59 100644 > --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > @@ -1223,10 +1223,12 @@ EvacuateTempRam ( > EFI_FIRMWARE_VOLUME_HEADER *FvHeader; > EFI_FIRMWARE_VOLUME_HEADER *ChildFvHeader; > EFI_FIRMWARE_VOLUME_HEADER *MigratedFvHeader; > + EFI_FIRMWARE_VOLUME_HEADER *RawDataFvHeader; > EFI_FIRMWARE_VOLUME_HEADER *MigratedChildFvHeader; > > PEI_CORE_FV_HANDLE PeiCoreFvHandle; > EFI_PEI_CORE_FV_LOCATION_PPI *PeiCoreFvLocationPpi; > + EDKII_MIGRATED_FV_INFO MigratedFvInfo; > > ASSERT (Private->PeiMemoryInstalled); > > @@ -1263,6 +1265,9 @@ EvacuateTempRam ( > (((EFI_PHYSICAL_ADDRESS)(UINTN) FvHeader + (FvHeader->FvLength - 1)) > < > Private->FreePhysicalMemoryTop) > ) > ) { > + // > + // Allocate page to save the rebased PEIMs, the PEIMs will get control > later Do you mean 'control' -> 'dispatched'? > + // > Status = PeiServicesAllocatePages ( > EfiBootServicesCode, > EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength), > @@ -1270,6 +1275,17 @@ EvacuateTempRam ( > ); > ASSERT_EFI_ERROR (Status); > > + // > + // Allocate pool to save the raw PEIMs, it used to keep consistent > context > across > + // multiple boot and PCR0 will keep same no matter if allocate random > page > address. Consider to revise the statement: 'no matter if ...' -> 'no matter if the address of allocated page is changed' > + // > + Status = PeiServicesAllocatePages ( > + EfiBootServicesCode, > + EFI_SIZE_TO_PAGES ((UINTN) FvHeader->FvLength), > + (EFI_PHYSICAL_ADDRESS *) &RawDataFvHeader > + ); > + ASSERT_EFI_ERROR (Status); > + > DEBUG (( > DEBUG_VERBOSE, > " Migrating FV[%d] from 0x%08X to 0x%08X\n", > @@ -1278,7 +1294,19 @@ EvacuateTempRam ( > (UINTN) MigratedFvHeader > )); > > + // > + // Copy the context to the rebased pages and raw pages, and create hob > to > save the > + // information. the MigratedFvInfo HOB will never produce when 'the' -> 'The' 'produce' -> 'be produced' Regards, Jian > + // PcdMigrateTemporaryRamFirmwareVolumes is FALSE, because the PCD > control the > + // feature. > + // > CopyMem (MigratedFvHeader, FvHeader, (UINTN) FvHeader->FvLength); > + CopyMem (RawDataFvHeader, MigratedFvHeader, (UINTN) FvHeader- > >FvLength); > + MigratedFvInfo.FvOrgBase = (UINT32) (UINTN) FvHeader; > + MigratedFvInfo.FvNewBase = (UINT32) (UINTN) MigratedFvHeader; > + MigratedFvInfo.FvDataBase = (UINT32) (UINTN) RawDataFvHeader; > + MigratedFvInfo.FvLength = (UINT32) (UINTN) FvHeader->FvLength; > + BuildGuidDataHob (&gEdkiiMigratedFvInfoGuid, &MigratedFvInfo, sizeof > (MigratedFvInfo)); > > // > // Migrate any children for this FV now > -- > 2.25.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62396): https://edk2.groups.io/g/devel/message/62396 Mute This Topic: https://groups.io/mt/75390179/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
