From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.10854.1593785027396854983 for ; Fri, 03 Jul 2020 07:03:47 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Y0Md7S52; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1593785026; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=z9qsjmJpAjR1lx61pCN2DOZWg7blyw4b86nqJiNPBwA=; b=Y0Md7S52g54lMZFlKDo95hbvKFYAsq3LzuLriHa8Kbdc6d/MRapQRTSqK2xaOlcwqXkvv5 06Wbt1esD66vU3vv29dNjYktkZtuHenO6SOto8SepJZk+zUwSv4xqdrtR0FFEm8aWVu4y3 Obdk1xh9g+UcZmamPEMeGPTODbEaooI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-438-wDIFG5UGP1KQhfcTPQ650g-1; Fri, 03 Jul 2020 10:03:45 -0400 X-MC-Unique: wDIFG5UGP1KQhfcTPQ650g-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6EDC12DA1; Fri, 3 Jul 2020 14:03:43 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-238.ams2.redhat.com [10.36.114.238]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6693C6CA4A; Fri, 3 Jul 2020 14:03:41 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 5/9] MdeModulePkg/Core: Create Migrated FV Info Hob for calculating hash (CVE-2019-11098) To: devel@edk2.groups.io, guomin.jiang@intel.com Cc: Jian J Wang , Hao A Wu , Dandan Bi , Liming Gao , Debkumar De , Harry Han , Catharine West References: <20200702051525.1102-1-guomin.jiang@intel.com> <20200702051525.1102-6-guomin.jiang@intel.com> From: "Laszlo Ersek" Message-ID: <2c15eab2-f519-f429-84d9-e25da00694d5@redhat.com> Date: Fri, 3 Jul 2020 16:03:40 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200702051525.1102-6-guomin.jiang@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 07/02/20 07:15, Guomin Jiang wrote: > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1614 > > When we allocate pool to save rebased the 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. (1) Please extend the commit message. We should state that "gEdkiiMigratedFvInfoGuid" HOBs are *never* produced if "PcdMigrateTemporaryRamFirmwareVolumes" is FALSE. > > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Dandan Bi > Cc: Liming Gao > Cc: Debkumar De > Cc: Harry Han > Cc: Catharine West > Signed-off-by: Guomin Jiang > --- > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 15 +++++++++++++ > MdeModulePkg/Core/Pei/PeiMain.h | 1 + > MdeModulePkg/Core/Pei/PeiMain.inf | 1 + > MdeModulePkg/Include/Guid/MigratedFvInfo.h | 22 +++++++++++++++++++ > MdeModulePkg/MdeModulePkg.dec | 3 +++ > 5 files changed, 42 insertions(+) > create mode 100644 MdeModulePkg/Include/Guid/MigratedFvInfo.h > > diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > index ef88b3423376..7e1ac38f35c8 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); > > @@ -1270,6 +1272,13 @@ EvacuateTempRam ( > ); > ASSERT_EFI_ERROR (Status); > > + 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", > @@ -1279,6 +1288,12 @@ EvacuateTempRam ( > )); > > 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) Please repeat the same statement here (from the commit message). That statement is very important for understanding the control flow of this feature. Some of the other modules do not consult "PcdMigrateTemporaryRamFirmwareVolumes"; instead, they consume "gEdkiiMigratedFvInfoGuid" HOBs. We should explain (both in the commit message and in the code) that "PcdMigrateTemporaryRamFirmwareVolumes" controls the *latter* kind of modules too, via the production of "gEdkiiMigratedFvInfoGuid" HOBs. (In other words, if the PCD is FALSE, such HOBs are never produced.) Thanks Laszlo > 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 > #include > #include > +#include > > /// > /// It is an FFS type extension used for PeiFindFileEx. It indicates current > diff --git a/MdeModulePkg/Core/Pei/PeiMain.inf b/MdeModulePkg/Core/Pei/PeiMain.inf > index 5ff14100a65f..c80d16b4efa6 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/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.
> +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/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec > index 843e963ad34b..5e25cbe98ada 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 }} >