From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mx.groups.io with SMTP id smtpd.web11.13344.1595401135013516251 for ; Tue, 21 Jul 2020 23:58:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=SRXzoWTB; spf=pass (domain: intel.com, ip: 192.55.52.115, mailfrom: jian.j.wang@intel.com) IronPort-SDR: 13UvQnGzpJK8UrERUs5uu6HJaKVhd4nm69wpKZVZpB+7jDhcaZDEYBEgRnl16qAPHOXz5P3Kmd nTY3jIkbxmkw== X-IronPort-AV: E=McAfee;i="6000,8403,9689"; a="149447048" X-IronPort-AV: E=Sophos;i="5.75,381,1589266800"; d="scan'208";a="149447048" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jul 2020 23:58:54 -0700 IronPort-SDR: 3qQ2EAThh3WTgxrKkfvnihOKxrmbdKCeJt3xJ4cd/RFBYJ4TrXlqcF44x5ZWfkLDcodXXCxMKF YIm4gNVd/0vg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,381,1589266800"; d="scan'208";a="488360595" Received: from orsmsx604.amr.corp.intel.com ([10.22.229.17]) by fmsmga005.fm.intel.com with ESMTP; 21 Jul 2020 23:58:54 -0700 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 21 Jul 2020 23:58:54 -0700 Received: from ORSEDG001.ED.cps.intel.com (10.7.248.4) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 21 Jul 2020 23:58:54 -0700 Received: from NAM04-CO1-obe.outbound.protection.outlook.com (104.47.45.59) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 21 Jul 2020 23:58:51 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=T+moPk+ds6CpEC4anxpocHgRgOf+HcjRiZ4emB9EL+qwHKhzPHoDl7sQojTTEY7kYTGTCsJoG4qX7hA4apFyw13UnMCbow/ta+dNvo3dSM+nJaZE0kPQ1kxS6Z1LPknI0s0oZXuCTGBsG9cT0mo+yskNDd3KY7j7a/OyrVDsLi2uYMb+BgMLk3lkUZrPUDH+1964vchsiubz94AAsuPuVSOeRc4r6vLjZPYY+FAYD3w6tBM8gdRn92WnC+14UvpH8GcbxXTDVZzj24ld/LYv4b2JmP+RYD4F++y4Us3rLWD+qXKi3t5PuBSez8BC+2cSydFxRobGUV0ZO1bO0MfIEg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=U7OtiD/UNmPDP1z2xlaANwjwKoSnPygGpzV4dcJcgk8=; b=h2xzi62tlNZgOidoxjuqqbKrzyYAh71ww76l0TlVY1azFIc6wjW46zQfOglCulH6h5M1Rxce99OvvXkfoyQ01wpXdFC+unZyVqXoJ8DnU00yVsFzo41ZyIn5Cjo1yfTnM6WB0VQ4P/jlzfGnNPdz+JObGj83M8Db0j+LrPIOcuuGrbtmu076+oLwSD2GGjkCimnqOLYztzWQRfVhAwLZY/+9I/pSTQi7VLh0HaF37bJSDHxy2Tq7ZcJZ46JyOkR5U5aedXbMyORWG8tUGzmKtEg5u/aF4MFpt2pPak3P+qQ3kvQS6QwTkly+olbOAxYvTMMPLlsHWLKrXWN6tk8fYg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=U7OtiD/UNmPDP1z2xlaANwjwKoSnPygGpzV4dcJcgk8=; b=SRXzoWTBd5fsb3aPY8OLohXvpPqT9Sdc34SOsUmo0mAS/DdvR7lirMzxu/Wbda265kAOy796RPJloIgpZw3SHYLVYbGQg5/QZdACJVkwm/K2JGd/9QYLuhJMomgihn5WmZjPI83RyjQ02/ZNSHecGEW3uPrW2lca0BqrAHccKoI= Received: from BYAPR11MB3303.namprd11.prod.outlook.com (2603:10b6:a03:18::15) by BYAPR11MB2598.namprd11.prod.outlook.com (2603:10b6:a02:cb::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3216.22; Wed, 22 Jul 2020 06:58:50 +0000 Received: from BYAPR11MB3303.namprd11.prod.outlook.com ([fe80::fc2a:d66e:8c79:6ecd]) by BYAPR11MB3303.namprd11.prod.outlook.com ([fe80::fc2a:d66e:8c79:6ecd%7]) with mapi id 15.20.3195.025; Wed, 22 Jul 2020 06:58:50 +0000 From: "Wang, Jian J" To: "devel@edk2.groups.io" , "Jiang, Guomin" CC: "Wu, Hao A" , "Bi, Dandan" , "Gao, Liming" , "De, Debkumar" , "Han, Harry" , "West, Catharine" Subject: Re: [edk2-devel] [PATCH v6 10/10] MdeModulePkg/Core: Avoid redundant shadow when enable the Migrated PCD (CVE-2019-11098) Thread-Topic: [edk2-devel] [PATCH v6 10/10] MdeModulePkg/Core: Avoid redundant shadow when enable the Migrated PCD (CVE-2019-11098) Thread-Index: AQHWXolGq6ZGgrd9/U+H8MbviW9bhakS9Fsw Date: Wed, 22 Jul 2020 06:58:50 +0000 Message-ID: References: <20200720113022.675-1-guomin.jiang@intel.com> <20200720113022.675-11-guomin.jiang@intel.com> In-Reply-To: <20200720113022.675-11-guomin.jiang@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMGUyZjQwNTQtOGU1Yy00Y2M5LTk3OGItZWFiMDdmODg5NGZmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiWjN0XC9WaEhQaElrbFNxTmNvVHNCWjdWdUdUejlkTFNPXC9Odnc3T3RLeURYK3NvT0tDNnFkZW01VHFNaHRFYWVTIn0= dlp-reaction: no-action dlp-version: 11.2.0.6 dlp-product: dlpe-windows x-ctpclassification: CTP_NT authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.194] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 89bc0386-e3d3-4445-5c69-08d82e0cb05d x-ms-traffictypediagnostic: BYAPR11MB2598: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: lD4iUSFCTejuQDgPGDcjUrAGUNXLlB0Eyp26JOjE5myCBoEaE5O/KSdEf8/7o81ZKeGNQsXOIM+gjMFkwzXeWaYr+oF5k2Y03LQLaodoIIdHNqY4vSa/icIzOH3kYafFC9CbftT+F5GpnTv5olmUfvYHwrk9hL0nF3TemHdnBoVjMNHQeMyKYKecGZD1BdJSEZxN60cFHwbPr5iI6scbanVPWehVRgLAqHhpiP1qKzwGbtLEEBVkvRpdxI3PVLYvxvnLx4moUxg8uB5DLcan9dt/gEWA02cp9/BBtSlK8lQn0S1MtdAb/ZSx351xxt+7m/ae1nWPrxxFBKfl/ImffkO8nkj/ZpOCz3FJ47+YyDloB3hG5+89MYGT8qbLjLTjbpm7XEnFIG9R5TzC25EINw== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:BYAPR11MB3303.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(396003)(39860400002)(136003)(376002)(346002)(366004)(52536014)(4326008)(6636002)(478600001)(966005)(9686003)(55016002)(5660300002)(83380400001)(33656002)(19627235002)(8676002)(8936002)(76116006)(66476007)(64756008)(66446008)(66556008)(66946007)(26005)(54906003)(86362001)(2906002)(6506007)(53546011)(316002)(110136005)(107886003)(7696005)(186003)(71200400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: wKfgoOhqud0TsTRY9YBoIfpV93g0y31bZFCS9bTw0pE9qHRErK3k9574UKyCF6p5KQuZyDXIU56amHIy3VluedkbK4GnzLQVKdaVDt6JlvzZiDZqKX2UZeighvDDjHN4ePtISddJHOxhdA1gOZ0zqQwfYMoBpbU6G/e/w7N4mvr6UHgOXO2L2R+s7GQXfkJRfIFhOCwaY5nWtCi6+e0nxB7waIq21b4pQXl4Qp0+HoRQ/3sj+IqUMYaTfqyenmmnnTykqHcKDUDR9EOyxsIAkyr6PSqJeYZIIqoJUIPNSLm2OhSa57PxypCLMePlUmwuNf2qc0hwXCQhorJagzxRBI5r2q6Y9E1JhQq0XCKN7lWaYwaqxNFO6DMKs630j1go5to4vdqd9LLCUc54+qccdtZ++NphAHVkQDV2T/Ns4C8WXqxoyxtmjhG47502xpvZGw9el7rhc6bXVCeUyPJL/KjohHPYCDHlvE8vovZsQyX9eFRvPDJNKrr2w2mew9Qi MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3303.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 89bc0386-e3d3-4445-5c69-08d82e0cb05d X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jul 2020 06:58:50.4939 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: A8/Yjn8AxJC8HGp2wCHOW2p7IqPmc0/2wHE232bMXz27pID4DiUoDpFHVl5SxYxoQvER6Ekg6HpAwHtCPiKywg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2598 Return-Path: jian.j.wang@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Guomin, See my inline comments below. > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Guomin > Jiang > Sent: Monday, July 20, 2020 7:30 PM > To: devel@edk2.groups.io > Cc: Wang, Jian J ; Wu, Hao A = ; > Bi, Dandan ; Gao, Liming ; De= , > Debkumar ; Han, Harry ; > West, Catharine > Subject: [edk2-devel] [PATCH v6 10/10] MdeModulePkg/Core: Avoid redundan= t > shadow when enable the Migrated PCD (CVE-2019-11098) >=20 > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D1614 >=20 > When PcdMigrateTemporaryRamFirmwareVolumes is TRUE, it will shadow the > PEIMs, when it is disabled, PEIMs marked REGISTER_FOR_SHADOW will be > shadowed as well and it is controled by PcdShadowPeimOnBoot and > PcdShadowPeimOnS3Boot. >=20 > To cover the shadow behavior, the change will always shadow PEIMs when > enable PcdMigrateTemporaryRamFirmwareVolumes. >=20 > When PcdMigrateTemporaryRamFirmwareVolumes is true, if enable > PcdShadowPeiOnBoot, it will shadow some PEIMs twice and occupy more > memory and waste more boot time. it is unnecessary, so the only valid > choice is to enable PcdMigrateTemporaryRamFirmwareVolumes and disable > PcdShadowPeimOnBoot. You could add similar clarification in dec where the new PCD is defined. >=20 > Signed-off-by: Guomin Jiang > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Dandan Bi > Cc: Liming Gao > Cc: Debkumar De > Cc: Harry Han > Cc: Catharine West > --- > MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 11 +++++++++-- > MdeModulePkg/Core/Pei/Image/Image.c | 14 ++++++++++---- > MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 18 +++++++++++++----- > 3 files changed, 32 insertions(+), 11 deletions(-) >=20 > diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > index 210b5b22f727..74cd5387c158 100644 > --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c > @@ -1408,7 +1408,11 @@ PeiDispatcher ( > PeimFileHandle =3D NULL; > EntryPoint =3D 0; >=20 > - if ((Private->PeiMemoryInstalled) && (Private- > >HobList.HandoffInformationTable->BootMode !=3D BOOT_ON_S3_RESUME || > PcdGetBool (PcdShadowPeimOnS3Boot))) { > + if ((Private->PeiMemoryInstalled) && > + (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes) > + || (Private->HobList.HandoffInformationTable->BootMode !=3D > BOOT_ON_S3_RESUME) > + || PcdGetBool (PcdShadowPeimOnS3Boot)) > + ) { > // > // Once real memory is available, shadow the RegisterForShadow modu= les. > And meanwhile > // update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW t= o > PEIM_STATE_DONE. > @@ -1607,7 +1611,10 @@ PeiDispatcher ( > PeiCheckAndSwitchStack (SecCoreData, Private); >=20 > if ((Private->PeiMemoryInstalled) && (Private- > >Fv[FvCount].PeimState[PeimCount] =3D=3D PEIM_STATE_REGISTER_FOR_SHADOW) > && \ > - (Private->HobList.HandoffInformationTable->BootMode != =3D > BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) { > + (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes) > + || (Private->HobList.HandoffInformationTable->BootMode= !=3D > BOOT_ON_S3_RESUME) > + || PcdGetBool (PcdShadowPeimOnS3Boot)) > + ) { > // > // If memory is available we shadow images by default for= performance > reasons. > // We call the entry point a 2nd time so the module knows= it's shadowed. > diff --git a/MdeModulePkg/Core/Pei/Image/Image.c > b/MdeModulePkg/Core/Pei/Image/Image.c > index 0caeb63e26b4..85d1a84e4b67 100644 > --- a/MdeModulePkg/Core/Pei/Image/Image.c > +++ b/MdeModulePkg/Core/Pei/Image/Image.c > @@ -328,8 +328,11 @@ LoadAndRelocatePeCoffImage ( > // > // When Image has no reloc section, it can't be relocated into memory= . > // > - if (ImageContext.RelocationsStripped && (Private->PeiMemoryInstalled)= && > ((!IsPeiModule) || > - (!IsS3Boot && (PcdGetBool (PcdShadowPeimOnBoot) || > IsRegisterForShadow)) || (IsS3Boot && PcdGetBool > (PcdShadowPeimOnS3Boot)))) { > + if (ImageContext.RelocationsStripped && (Private->PeiMemoryInstalled) > + && ((!IsPeiModule) || PcdGetBool > (PcdMigrateTemporaryRamFirmwareVolumes) > + || (!IsS3Boot && (PcdGetBool (PcdShadowPeimOnBoot) || > IsRegisterForShadow)) > + || (IsS3Boot && PcdGetBool (PcdShadowPeimOnS3Boot))) > + ) { > DEBUG ((EFI_D_INFO|EFI_D_LOAD, "The image at 0x%08x without reloc > section can't be loaded into memory\n", (UINTN) Pe32Data)); > } >=20 > @@ -343,8 +346,11 @@ LoadAndRelocatePeCoffImage ( > // On normal boot, PcdShadowPeimOnBoot decides whether load PEIM or > PeiCore into memory. > // On S3 boot, PcdShadowPeimOnS3Boot decides whether load PEIM or > PeiCore into memory. > // > - if ((!ImageContext.RelocationsStripped) && (Private->PeiMemoryInstall= ed) && > ((!IsPeiModule) || > - (!IsS3Boot && (PcdGetBool (PcdShadowPeimOnBoot) || > IsRegisterForShadow)) || (IsS3Boot && PcdGetBool > (PcdShadowPeimOnS3Boot)))) { > + if ((!ImageContext.RelocationsStripped) && (Private->PeiMemoryInstall= ed) > + && ((!IsPeiModule) || PcdGetBool > (PcdMigrateTemporaryRamFirmwareVolumes) > + || (!IsS3Boot && (PcdGetBool (PcdShadowPeimOnBoot) || > IsRegisterForShadow)) > + || (IsS3Boot && PcdGetBool (PcdShadowPeimOnS3Boot))) > + ) { > // > // Allocate more buffer to avoid buffer overflow. > // > diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c > b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c > index 48605eeada86..ce8649f954a8 100644 > --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c > +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c > @@ -322,7 +322,8 @@ PeiCore ( > // PEI Core and PEIMs to get high performance. > // > OldCoreData->ShadowedPeiCore =3D (PEICORE_FUNCTION_POINTER) (UINT= N) > PeiCore; > - if ((HandoffInformationTable->BootMode =3D=3D BOOT_ON_S3_RESUME &= & > PcdGetBool (PcdShadowPeimOnS3Boot)) > + if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes) > + || (HandoffInformationTable->BootMode =3D=3D BOOT_ON_S3_RESUM= E && > PcdGetBool (PcdShadowPeimOnS3Boot)) > || (HandoffInformationTable->BootMode !=3D BOOT_ON_S3_RESUME = && > PcdGetBool (PcdShadowPeimOnBoot))) { > OldCoreData->ShadowedPeiCore =3D ShadowPeiCore (OldCoreData); > } > @@ -422,10 +423,17 @@ PeiCore ( > } > } else { > if (PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes)) { > - if (PrivateData.HobList.HandoffInformationTable->BootMode =3D=3D > BOOT_ON_S3_RESUME) { > - TempRamEvacuation =3D PcdGetBool (PcdShadowPeimOnS3Boot); > - } else { > - TempRamEvacuation =3D PcdGetBool (PcdShadowPeimOnBoot); > + TempRamEvacuation =3D TRUE; > + > + // > + // When PcdMigrateTemporaryRamFirmwareVolumes is TRUE, it make > sense only make -> makes > + // when PcdShadowPeimOnBoot is FALSE. in this situation, all PEIM= s will be > copied in -> In > + // and shadowed, and doesn't need shadow PEIMs again, it will occ= upy > more > + // memory and waste more time if you enable it. > + // > + if (PcdGetBool (PcdShadowPeimOnBoot)) { > + DEBUG ((DEBUG_ERROR, "!!!IMPORTANT NOTICE!!!\nWhen you see the > message, it mean that you enable the > PcdMigrateTemporaryRamFirmwareVolumes and PcdShadowPeimOnBoot at the > same time\nIt make no sense because it will occupy more memory and waste > more time.\nYou must disable PcdShadowPeimOnBoot when you enable > PcdMigrateTemporaryRamFirmwareVolumes for performance reason.\n\n")); Too long string literal. You can split it into more string literals separa= ted by space. For example, DEBUG (( DEBUG_INFO, "String part 1" "String part 2" "String part 3" )); > + ASSERT ((PcdGetBool (PcdMigrateTemporaryRamFirmwareVolumes) =3D= = =3D > TRUE) && (PcdGetBool (PcdShadowPeimOnBoot) =3D=3D FALSE)); > } > } >=20 > -- > 2.25.1.windows.1 >=20 >=20 >=20