From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by mx.groups.io with SMTP id smtpd.web09.1851.1645059637183613592 for ; Wed, 16 Feb 2022 17:00:38 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=dWwpH09v; spf=pass (domain: intel.com, ip: 134.134.136.24, mailfrom: nathaniel.l.desimone@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1645059637; x=1676595637; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=Oqx43x+Vz+XY+24s0FyghFTdMafta9cMH5MKWhK+02E=; b=dWwpH09va0GWopHgGQE8gQqNL4Q4e2t69hI8VVjJWWHpzf4WYMYDHtcF t+eDOWQq+/vHYGLKsI96oGbCQ2Iy0eHhTJFUojIBS/I8bi5p7XWFKAbKu IaRvwhsLo8LYciLRD+wR1UCZJSWM/ex55J9vAhr70WjIOkmLzBbGxWH3F 4556epxXoUOetog0j7jp4WkDVPh47FDSDKjnn1CLFwfBPvrza2SHD5P95 iHzR/kdTs5Iw0HuGs9/cOh6Y6sXK9UZnz6gGrULiMHXuhhpQygYYPUVJS bVb6HPkFZgLQV9R74JZR4NXoG++pXRSKktGhFLPmFPdskJF0benjT+Vsd Q==; X-IronPort-AV: E=McAfee;i="6200,9189,10260"; a="250494305" X-IronPort-AV: E=Sophos;i="5.88,374,1635231600"; d="scan'208";a="250494305" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Feb 2022 17:00:36 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.88,374,1635231600"; d="scan'208";a="571547278" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by orsmga001.jf.intel.com with ESMTP; 16 Feb 2022 17:00:36 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20; Wed, 16 Feb 2022 17:00:36 -0800 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20; Wed, 16 Feb 2022 17:00:35 -0800 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20 via Frontend Transport; Wed, 16 Feb 2022 17:00:35 -0800 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (104.47.66.46) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Wed, 16 Feb 2022 17:00:33 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZVjrwu0j/K3kvdCjnSqsV494DkO4ixeln2BrMpQhtHDu69Yl4uBc/fqU5gaInXiuBcf1xF6e244sCsGHYnfDkizVxiVTqib9uPOzoats6StNWRVvIBX0TnHvsli5aHau7Wm3jyncwZUPjU02ZD/tefOxWBpF/A+wewU4OIlJRUnMIVf+zU1JtxP52xffQ3PS9Cx+cmvaMnCqsBwRZtnxtjqOuUBj4D5v9UtucXqPmhT4gdYQDebT56IpcQgCgpxM2wX39iWoe0rwydFQSh40ojqy++RK+sSb6hUekPCqWTpF2J09oYApjejvwAWb34UpT796VbX//6t71zg23dqbnQ== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=RfycdS9TvVqJWbnaUDb3rsLRWP6MMI21AdOGEQdxcR4=; b=j79ukTXvdDPpFZgCECnY6mTpfLU+4YzbeHoJMH0SXB5btA3lEMr3kP9uaQnK2jndK5wSFZhjgBUZeHrdlP1yGSXnr+YNbO0wfVmwiIc4lag+PeyAiWfEtRJcltwThy6L0o6v68q3aiT7H9W5H/S9UMI5v8kY/bdYUsivbwtILoyBasWkaeAafgngw43kPLuhupTmz+br0nhDWL2Y4hTX4w1GWrMoWyUdWMEkxU99s3Ojon8N1SeFaKZO6xj/KXeGx5NwDJ1HbjR0k6ob5vvLhIV/eOXiXMcS5hM8zZ0ioyU9BvRTA+CM2mUjAp8PNWt7R4BOU2kk1yNixCJkLoZBGQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from MW4PR11MB5821.namprd11.prod.outlook.com (2603:10b6:303:184::5) by BYAPR11MB3575.namprd11.prod.outlook.com (2603:10b6:a03:b3::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4975.15; Thu, 17 Feb 2022 01:00:31 +0000 Received: from MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::d98c:fa5f:f283:f1f6]) by MW4PR11MB5821.namprd11.prod.outlook.com ([fe80::d98c:fa5f:f283:f1f6%6]) with mapi id 15.20.4995.016; Thu, 17 Feb 2022 01:00:31 +0000 From: "Nate DeSimone" To: "Chiu, Chasel" , "devel@edk2.groups.io" CC: "Gao, Liming" , "Dong, Eric" Subject: Re: [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked. Thread-Topic: [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: Variable may not be locked. Thread-Index: AQHYHyYfPZutvwaBm06Haq/V0otA7qyW8hMA Date: Thu, 17 Feb 2022 01:00:31 +0000 Message-ID: References: <20220211090204.987-1-chasel.chiu@intel.com> In-Reply-To: <20220211090204.987-1-chasel.chiu@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-reaction: no-action dlp-version: 11.6.200.16 dlp-product: dlpe-windows authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b741bca5-cd37-420f-4299-08d9f1b0e598 x-ms-traffictypediagnostic: BYAPR11MB3575:EE_ x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:142; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: jAbhk++n2fikDPzoq+6qE64rYFGX790icHKwp5LtiYkTL2TixxxAbGQNPVV/a11O7klLyqLk/WZQ3iCPcD/Er6Lp2sCEG4N2hX2P7Qc38v/84qcTvXNl/ENtRS7kGbW8MZ0C6AEGT8guUlGPInm9rF/S2y7OxvvtqRJMAi0fNMrcuYLc9MBIV9+VlJVFHtq5q2/zsRZmivTOnsjfV5zWS7+bBywSOKLn1/H6BbxNQPtQxX86LZAn11gYwq59wuH0gwhREsDE46hI29pGKrptJHiH8fNucnfqrmN7YUWAEa3uG5zcTYAl1XfylbLp4nXPoK63sM3tmE0wXTpzPLcZ3v888IIOzkmHhImM+dOlq0bgjfP6ZGzoUIl9F7w1nmxN8WYzABxEugG1uYLBg52VEXAAsUNrGrF5rg9O7L/R+5GU1Zwz289AU/AsjpaotAbI2Kq25VRfnSOAtJVU2Z224mhvFZ1oeziufHbSqhYMQ9ttwItIG6M8LIeP9hYVe2Fz3ZM6/+i9WsE21FBQck5WxvHx6j6yfmeAfRfiEtAENwSI8wn3yUrl2iGgrWKZuZ8p64gp4BWfYlkhG1/C3AjxxqbTQcby+AnU7x6e1/IuQid+7/UExq/5wwZ5ZOnYFGT7NO4+t2T1lc19jHKJTbBR50RlpKUUbQyJ3DUFU5q8Zub1Cekz/CumOpTK6d3DcFRH/iAsTubRpygK5ObIuXYctrFTddVccqAnXYXFxsJJ5EcHQd56w829NQkPYv6xuhy6z2rhF5i46bM2RW5oNPOOqSkEdr076Zp9jxBui5ygBI0= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5821.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230001)(366004)(71200400001)(53546011)(2906002)(9686003)(30864003)(52536014)(86362001)(8676002)(8936002)(7696005)(38070700005)(83380400001)(6506007)(5660300002)(110136005)(54906003)(508600001)(55016003)(66556008)(66446008)(64756008)(122000001)(26005)(82960400001)(66476007)(4326008)(966005)(66946007)(316002)(38100700002)(33656002)(76116006)(186003)(107886003)(579004);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?gPTRJrREeOgZfXm4nR/cHT0FekGiW71Y2olmGYtRAJuOKvGX4iQwhWcH9vzS?= =?us-ascii?Q?oJC5RkfH7P04ZG8rQueC9DqjiWy/zwxMEzJ28MyChGauSRkyXGxbfVnIKwwL?= =?us-ascii?Q?glosw413VW7AiLUyEfFmMOYL2X1lLBkGBunO320nuRsCI8/KBKk+nyRKOb7/?= =?us-ascii?Q?tPdRuWSIMn6qbw18QTKlFf59PvE7qSGQm9HCToD2Ui1PQ5otSA2EciRYkPSv?= =?us-ascii?Q?hfY5jVqJ15bDB/yYzx+zGOHbDP8oXcVxFXHZROMcGB0X8GPoDLsCruJKRPzi?= =?us-ascii?Q?B+/XdbF0Q2CK70xpT1BdkhhlMyU8ucdDSb1MA87yymd/ZSxBF1V1Np69L6P0?= =?us-ascii?Q?2gbQ1zXG+NCvZF6JPhJhPfT7uvnfkCmd/MgZZKi7Aqt6n45Rd7sLUD5MLAPQ?= =?us-ascii?Q?yTdK0RzX/YzDnAJQ1qCffeNLxDJe9pAjpD1XzkhgL7ILGyPwBSxcyLCZTPaE?= =?us-ascii?Q?NMAXIa5jf4PYG0WqBN6+dcEbVYAH7NyX6k24k0f14ogP/JiqyEkwUt5h8Fso?= =?us-ascii?Q?Qncz7vUBghKxIGNNpkQ0UlLgHo9kuKjTtqBlEUEZUFtswQhx4lvb/svNdV4h?= =?us-ascii?Q?xDs1javhVLWmRZK0nf9syZClqUG+VKHxsL3kaX8tgSVKaBNiiyC2mKAETnNj?= =?us-ascii?Q?Q5Os6W2t/wQ3n5oNF7OjhocTcq7j6b/OQea78Q4jdmWTmv8QlA9yuhabCsCt?= =?us-ascii?Q?RxWTLDTjZmCiocr7qEnfpZoJWxIIFQBpq03v7f2wA6BT1wDrZCLenqexkIls?= =?us-ascii?Q?9ITQFaL/rQtdinLdP8x5qzjlB7gPClm+1A1bzrxPhV60rLlskodCA5JM4anM?= =?us-ascii?Q?iZghht+8OkUxEanXjP0dWN4OI5VeMcgG+ba4C+zOPl4Q2IhrgTA5zD9zdVwV?= =?us-ascii?Q?aGkVVKxLvEgM9piHJ2rjnFso+zewu3kslNw05/qX8bOP4vNQHcG8v3eQDDqn?= =?us-ascii?Q?hMe+Mgz6RxYUhO1GpZ7MDJ9kIsGdDKNX/pkRvAZWeD7VLASooyess/H84g6W?= =?us-ascii?Q?ARidEtAyI9MywYJWLinUKkIcBQ1VSFbA6OnLTQ80yjR/stYjVFOGQoaRYV9f?= =?us-ascii?Q?ycY+QqfTLRC7C4Po9glrIwnW97DltGRunG2F8TeTVOSua83h64KZ9h16bvy/?= =?us-ascii?Q?BAD8NQHs9IHTtpRnz1gfVxdUFH+bOdSJFAdwcRlKAXdqUTQBjRvpIXDfkHCt?= =?us-ascii?Q?r/u8MkgYs4XgDHhGoEJ6i7/I//LPk0hRj6nKO5kfqV8TSMGTJEyHF2wZ4ENg?= =?us-ascii?Q?9Fsda7/FTyzFbJtlUKbG1+QIZbxaw4pIx9izlj7FG/lolfkG6AE/uUPfZFFC?= =?us-ascii?Q?KFPwqiqJi11S+qgYMqoecqZmw183Birmz7VJvo9475K3rFi7x/aL0jC2+qZg?= =?us-ascii?Q?ZrIvOnJZE72yDeBnViTir8ZeQ1nMUuwsWzjMQUyHEf5Z5rFG41LotnlLyknX?= =?us-ascii?Q?G/PLdedxvWYfzkOrgnKt9VXN8EAnhCIeUHw+vjkC1hvsSwJKplMV2X+1a8/S?= =?us-ascii?Q?6RrbPAkGNJ0bITzL4XGE0Ag7osToCn/b0cQ1Ovld2UQuXTiOzFG5Kh7s9OuA?= =?us-ascii?Q?BtyeTZtdDxb3bHYSFezDr2AVwYrVR0Gec3Deaflje50m6HwISO/SLKyFODW2?= =?us-ascii?Q?16uu1iGVFv7V+KrEpSyCPmjBQmNmZE7qOz/S2y6tS3ERcJIsD9M+PHT+uIs6?= =?us-ascii?Q?MeSR5A=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5821.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: b741bca5-cd37-420f-4299-08d9f1b0e598 X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Feb 2022 01:00:31.6885 (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: wjguAJmdQ6n3veWmzLPB0gCGF79hn+YaHG/k29haS4V1OX6zBQCcQUo5HYThSZLIkgMGlQr4KgZ2IvPnxV/g2BFLeFk2HoRLb38dgsn0Ddc= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3575 Return-Path: nathaniel.l.desimone@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Chasel, Please see feedback inline. Thanks, Nate > -----Original Message----- > From: Chiu, Chasel > Sent: Friday, February 11, 2022 1:02 AM > To: devel@edk2.groups.io > Cc: Chiu, Chasel ; Desimone, Nathaniel L > ; Gao, Liming > ; Dong, Eric > Subject: [edk2-platforms: PATCH v3] MinPlatformPkg/SaveMemoryConfig: > Variable may not be locked. >=20 > From: "Chiu, Chasel" >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D3829 >=20 > Fixed the bug that existing variable will not be locked when it is > identical with hob data by creating LockLargeVariable function, also > switched to VariablePolicyProtocol for locking variables. >=20 > Failing to lock variable could be security vulnerability, so the > function will return EFI_ABORTED when it failed and SaveMemoryConfig > driver will halt the system for developers to resolve this issue. >=20 > This patch also modified SaveMemoryConfig driver to be unloaded after > execution because it does not produce any service protocol. To achieve > this goal the DxeRuntimeVariableWriteLib should close registered > ExitBootService events in its DESTRUCTOR. >=20 > Cc: Nate DeSimone > Cc: Liming Gao > Cc: Eric Dong > Signed-off-by: Chasel Chiu > --- >=20 >=20 >=20 > V3: >=20 > Updated LargeVariableWriteLib to return EFI_ABORTED when locking variable= s failed. >=20 > Also SaveMemoryConfig driver will halt the system in this case for develo= pers to fix >=20 > such security vulnerability issue. >=20 >=20 > Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConf= ig.c | 27 ++++++++++++++++++++++++--- > Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariable= WriteLib.c | 115 +++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----- > Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunt= imeVariableWriteLib.c | 61 +++++++++++++++++++++++++++++++++++++++++++++= ---------------- > Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemoryConf= ig.inf | 3 ++- > Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h = | 25 +++++++++++++++++++++++-- > Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRunt= imeVariableWriteLib.inf | 8 +++++--- > 6 files changed, 209 insertions(+), 30 deletions(-) >=20 > diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/Sa= veMemoryConfig.c b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfi= g/SaveMemoryConfig.c > index 820585f676..54e11e20bd 100644 > --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor= yConfig.c > +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor= yConfig.c > @@ -2,13 +2,14 @@ > This is the driver that locates the MemoryConfigurationData HOB, if it > exists, and saves the data to nvRAM. > =20 > -Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
> +Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > =20 > **/ > =20 > #include > #include > +#include > #include > #include > #include > @@ -18,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > #include > #include > +#include > #include > =20 > /** > @@ -86,6 +88,18 @@ SaveMemoryConfigEntryPoint ( > Status =3D GetLargeVariable (L"FspNvsBuffer", &gFspNvsBuffer= VariableGuid, &BufferSize, VariableData); > if (!EFI_ERROR (Status) && (BufferSize =3D=3D DataSize) && (= 0 =3D=3D CompareMem (HobData, VariableData, DataSize))) { > DataIsIdentical =3D TRUE; > + // > + // No need to update Variable, only lock it. > + // > + Status =3D LockLargeVariable (L"FspNvsBuffer", &gFspNvsBu= fferVariableGuid); > + if (EFI_ERROR(Status)) { > + // > + // Fail to lock variable is security vulnerability and s= hould not happen. > + // > + DEBUG ((DEBUG_ERROR, "LockVariable is requested but fail= ed unexpectedly!\n")); > + ASSERT_EFI_ERROR (Status); > + CpuDeadLoop(); A CpuDeadLoop() on release builds will cause failures on production systems= . I understand the concern of a security vulnerability, but we should mitig= ate that by deleting the FspNvsBuffer variable if we are unable to lock it.= Then we will re-run the full MRC during the next boot without security iss= ue. > + } > } > FreePool (VariableData); > } > @@ -96,6 +110,13 @@ SaveMemoryConfigEntryPoint ( > if (!DataIsIdentical) { > Status =3D SetLargeVariable (L"FspNvsBuffer", &gFspNvsBufferVari= ableGuid, TRUE, DataSize, HobData); > ASSERT_EFI_ERROR (Status); > + if (Status =3D=3D EFI_ABORTED) { > + // > + // Fail to lock variable is security vulnerability and should = not happen. > + // > + DEBUG ((DEBUG_ERROR, "LockVariable is requested but failed une= xpectedly!\n")); It would be a good idea to put an ASSERT_EFI_ERROR() here since the dead lo= op should be gone. > + CpuDeadLoop(); A CpuDeadLoop() on release builds will cause failures on production systems= . I understand the concern of a security vulnerability, but we should mitig= ate that by deleting the FspNvsBuffer variable if we are unable to lock it.= Then we will re-run the full MRC during the next boot without security iss= ue. > + } > DEBUG ((DEBUG_INFO, "Saved size of FSP / MRC Training Data: 0x%x= \n", DataSize)); > } else { > DEBUG ((DEBUG_INFO, "FSP / MRC Training Data is identical to dat= a from last boot, no need to save.\n")); > @@ -106,7 +127,7 @@ SaveMemoryConfigEntryPoint ( > } > =20 > // > - // This driver cannot be unloaded because DxeRuntimeVariableWriteLib c= onstructor will register ExitBootServices callback. > + // This driver does not produce any protocol services, so always unloa= d it. > // > - return EFI_SUCCESS; > + return EFI_REQUEST_UNLOAD_IMAGE; > } > diff --git a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/L= argeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVar= iableLib/LargeVariableWriteLib.c > index e4b97ef1df..154f6f448f 100644 > --- a/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVar= iableWriteLib.c > +++ b/Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVar= iableWriteLib.c > @@ -10,7 +10,7 @@ > integer number will be added to the end of the variable name. This num= ber > will be incremented for each variable as needed to store the entire da= ta set. > =20 > - Copyright (c) 2021, Intel Corporation. All rights reserved.
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > =20 > **/ > @@ -245,7 +245,7 @@ Done: > @retval EFI_DEVICE_ERROR The variable could not be retrieved due= to a hardware error. > @retval EFI_WRITE_PROTECTED The variable in question is read-only. > @retval EFI_WRITE_PROTECTED The variable in question cannot be dele= ted. > - > + @retval EFI_ABORTED LockVariable was requested but failed. > @retval EFI_NOT_FOUND The variable trying to be updated or de= leted was not found. > =20 > **/ > @@ -412,7 +412,7 @@ SetLargeVariable ( > // all data is saved. > // > if (LockVariable) { > - for (Index =3D 0; Index < VariablesSaved; Index++) { > + for (Index =3D 0; Index <=3D VariablesSaved; Index++) { > ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); > UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d"= , VariableName, Index); > =20 > @@ -420,7 +420,7 @@ SetLargeVariable ( > Status =3D VarLibVariableRequestToLock (TempVariableName, Vendor= Guid); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error locking variable= : Status =3D %r\n", Status)); > - VariablesSaved =3D 0; > + Status =3D EFI_ABORTED; > goto Done; > } > } > @@ -442,9 +442,114 @@ Done: > 0, > NULL > ); > - DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable: Sta= tus =3D %r\n", Status2)); > + if (EFI_ERROR (Status2)) { > + DEBUG ((DEBUG_ERROR, "SetLargeVariable: Error deleting variable:= Status =3D %r\n", Status2)); > + } > } > } > DEBUG ((DEBUG_ERROR, "SetLargeVariable: Status =3D %r\n", Status)); > return Status; > } > + > +/** > + Locks the existing large variable. > + > + @param[in] VariableName A Null-terminated string that is the na= me of the vendor's variable. > + Each VariableName is unique for each Ve= ndorGuid. VariableName must > + contain 1 or more characters. If Variab= leName is an empty string, > + then EFI_INVALID_PARAMETER is returned. > + @param[in] VendorGuid A unique identifier for the vendor. > + @retval EFI_SUCCESS The firmware has successfully locked th= e variable. > + @retval EFI_INVALID_PARAMETER An invalid combination of variable name= and GUID was supplied > + @retval EFI_UNSUPPORTED The service for locking variable is not= ready. > + @retval EFI_NOT_FOUND The targeting variable for locking is n= ot present. > + @retval EFI_ABORTED Fail to lock variable. > +**/ > +EFI_STATUS > +EFIAPI > +LockLargeVariable ( > + IN CHAR16 *VariableName, > + IN EFI_GUID *VendorGuid > + ) > +{ > + CHAR16 TempVariableName[MAX_VARIABLE_NAME_SIZE]; > + UINT64 VariableSize; > + EFI_STATUS Status; > + UINTN Index; > + > + // > + // Check input parameters. > + // > + if (VariableName =3D=3D NULL || VariableName[0] =3D=3D 0 || VendorGuid= =3D=3D NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (!VarLibIsVariableRequestToLockSupported ()) { > + return EFI_UNSUPPORTED; > + } > + You are missing a buffer overflow check here. Something like this: if (StrLen (VariableName) >=3D (MAX_VARIABLE_NAME_SIZE - MAX_VARIABLE_SPL= IT_DIGITS)) { DEBUG ((DEBUG_ERROR, "LockLargeVariable: Variable name too long\n")); Status =3D EFI_OUT_OF_RESOURCES; goto Done; } > + VariableSize =3D 0; > + Index =3D 0; > + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); > + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", Vari= ableName, Index); > + Status =3D VarLibGetVariable (TempVariableName, VendorGuid, NULL, &Var= iableSize, NULL); For performance reasons I think it would be best to check the single variab= le case first. Because the single variable case should be the most common o= ne. > + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { > + // > + // Lock multiple variables. > + // > + > + // > + // Lock first variable and continue to rest of the variables. > + // > + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", TempVariableName, V= endorGuid)); > + Status =3D VarLibVariableRequestToLock (TempVariableName, VendorGuid= ); > + if (EFI_ERROR(Status)) { > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus =3D %r\n", = Status)); > + return EFI_ABORTED; > + } > + for (Index =3D 1; Index < MAX_VARIABLE_SPLIT; Index++) { > + ZeroMem (TempVariableName, MAX_VARIABLE_NAME_SIZE); > + UnicodeSPrint (TempVariableName, MAX_VARIABLE_NAME_SIZE, L"%s%d", = VariableName, Index); > + > + VariableSize =3D 0; > + Status =3D VarLibGetVariable (TempVariableName, VendorGuid, NULL, = &VariableSize, NULL); > + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { > + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", TempVariableNam= e, VendorGuid)); > + Status =3D VarLibVariableRequestToLock (TempVariableName, Vendor= Guid); > + if (EFI_ERROR(Status)) { > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus =3D %r\= n", Status)); > + return EFI_ABORTED; > + } > + } else if (Status =3D=3D EFI_NOT_FOUND) { > + // > + // No more variables need to lock. > + // > + return EFI_SUCCESS; > + } > + } // End of for loop > + } else if (Status =3D=3D EFI_NOT_FOUND) { > + // > + // Check if it is single variable scenario. > + // > + VariableSize =3D 0; > + Status =3D VarLibGetVariable (VariableName, VendorGuid, NULL, &Varia= bleSize, NULL); > + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { > + // > + // Lock single variable. > + // > + DEBUG ((DEBUG_INFO, "Locking %s, Guid =3D %g\n", VariableName, Ven= dorGuid)); > + Status =3D VarLibVariableRequestToLock (VariableName, VendorGuid); > + if (EFI_ERROR(Status)) { > + DEBUG ((DEBUG_ERROR, "LockLargeVariable: Failed! Satus =3D %r\n"= , Status)); > + return EFI_ABORTED; > + } > + return EFI_SUCCESS; > + } > + } > + > + // > + // Here probably means variable not present. > + // > + return Status; > + > +} > diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWrit= eLib/DxeRuntimeVariableWriteLib.c b/Platform/Intel/MinPlatformPkg/Library/D= xeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c > index 9ed59f8827..28730f858b 100644 > --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dx= eRuntimeVariableWriteLib.c > +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dx= eRuntimeVariableWriteLib.c > @@ -10,7 +10,7 @@ > Using this library allows code to be written in a generic manner that = can be > used in DXE or SMM without modification. > =20 > - Copyright (c) 2021, Intel Corporation. All rights reserved.
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > =20 > **/ > @@ -18,14 +18,16 @@ > #include > =20 > #include > -#include > +#include > =20 > #include > #include > #include > #include > =20 > -STATIC EDKII_VARIABLE_LOCK_PROTOCOL *mVariableWriteLibVariableLock =3D = NULL; > +STATIC EDKII_VARIABLE_POLICY_PROTOCOL *mVariableWriteLibVariablePolicy = =3D NULL; > +EFI_EVENT mExitBootServiceEvent; > +EFI_EVENT mLegacyBootEvent; > =20 > /** > Sets the value of a variable. > @@ -144,7 +146,7 @@ VarLibIsVariableRequestToLockSupported ( > VOID > ) > { > - if (mVariableWriteLibVariableLock !=3D NULL) { > + if (mVariableWriteLibVariablePolicy !=3D NULL) { > return TRUE; > } else { > return FALSE; > @@ -178,16 +180,45 @@ VarLibVariableRequestToLock ( > { > EFI_STATUS Status =3D EFI_UNSUPPORTED; > =20 > - if (mVariableWriteLibVariableLock !=3D NULL) { > - Status =3D mVariableWriteLibVariableLock->RequestToLock ( > - mVariableWriteLibVariableL= ock, > - VariableName, > - VendorGuid > - ); > + if (mVariableWriteLibVariablePolicy !=3D NULL) { > + Status =3D RegisterBasicVariablePolicy ( > + mVariableWriteLibVariablePolicy, > + (CONST EFI_GUID*) VendorGuid, > + (CONST CHAR16 *) VariableName, > + VARIABLE_POLICY_NO_MIN_SIZE, > + VARIABLE_POLICY_NO_MAX_SIZE, > + VARIABLE_POLICY_NO_MUST_ATTR, > + VARIABLE_POLICY_NO_CANT_ATTR, > + VARIABLE_POLICY_TYPE_LOCK_NOW > + ); > } > return Status; > } > =20 > +/** > + Close events when driver unloaded. > + > + @param[in] ImageHandle A handle for the image that is initializing th= is driver > + @param[in] SystemTable A pointer to the EFI system table > + > + @retval EFI_SUCCESS The initialization finished successfully. > +**/ > +EFI_STATUS > +EFIAPI > +DxeRuntimeVariableWriteLibDestructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + if (mExitBootServiceEvent !=3D 0) { > + gBS->CloseEvent (mExitBootServiceEvent); > + } > + if (mLegacyBootEvent !=3D 0) { > + gBS->CloseEvent (mLegacyBootEvent); > + } > + return EFI_SUCCESS; > +} > + > /** > Exit Boot Services Event notification handler. > =20 > @@ -202,7 +233,7 @@ DxeRuntimeVariableWriteLibOnExitBootServices ( > IN VOID *Context > ) > { > - mVariableWriteLibVariableLock =3D NULL; > + mVariableWriteLibVariablePolicy =3D NULL; > } > =20 > /** > @@ -227,13 +258,11 @@ DxeRuntimeVariableWriteLibConstructor ( > ) > { > EFI_STATUS Status; > - EFI_EVENT ExitBootServiceEvent; > - EFI_EVENT LegacyBootEvent; > =20 > // > // Locate VariableLockProtocol. > // > - Status =3D gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, NULL,= (VOID **)&mVariableWriteLibVariableLock); > + Status =3D gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, NUL= L, (VOID **)&mVariableWriteLibVariablePolicy); > ASSERT_EFI_ERROR (Status); > =20 > // > @@ -245,7 +274,7 @@ DxeRuntimeVariableWriteLibConstructor ( > DxeRuntimeVariableWriteLibOnExitBootServices, > NULL, > &gEfiEventExitBootServicesGuid, > - &ExitBootServiceEvent > + &mExitBootServiceEvent > ); > ASSERT_EFI_ERROR (Status); > =20 > @@ -257,7 +286,7 @@ DxeRuntimeVariableWriteLibConstructor ( > TPL_NOTIFY, > DxeRuntimeVariableWriteLibOnExitBootServices, > NULL, > - &LegacyBootEvent > + &mLegacyBootEvent > ); > ASSERT_EFI_ERROR (Status); > =20 > diff --git a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/Sa= veMemoryConfig.inf b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryCon= fig/SaveMemoryConfig.inf > index e2dbd2fb49..61e85a6586 100644 > --- a/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor= yConfig.inf > +++ b/Platform/Intel/MinPlatformPkg/FspWrapper/SaveMemoryConfig/SaveMemor= yConfig.inf > @@ -1,7 +1,7 @@ > ### @file > # Component information file for SaveMemoryConfig module > # > -# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -25,6 +25,7 @@ > BaseMemoryLib > LargeVariableReadLib > LargeVariableWriteLib > + BaseLib > =20 > [Packages] > MdePkg/MdePkg.dec > diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableW= riteLib.h b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWrit= eLib.h > index c847d7f152..64b0090c2c 100644 > --- a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib= .h > +++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib= .h > @@ -16,7 +16,7 @@ > is possible, adjusting the value of PcdMaxVariableSize may provide a s= impler > solution to this problem. > =20 > - Copyright (c) 2021, Intel Corporation. All rights reserved.
> + Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
> SPDX-License-Identifier: BSD-2-Clause-Patent > =20 > **/ > @@ -52,7 +52,7 @@ > @retval EFI_DEVICE_ERROR The variable could not be retrieved due= to a hardware error. > @retval EFI_WRITE_PROTECTED The variable in question is read-only. > @retval EFI_WRITE_PROTECTED The variable in question cannot be dele= ted. > - > + @retval EFI_ABORTED LockVariable was requested but failed. > @retval EFI_NOT_FOUND The variable trying to be updated or de= leted was not found. > =20 > **/ > @@ -66,4 +66,25 @@ SetLargeVariable ( > IN VOID *Data > ); > =20 > +/** > + Locks the existing large variable. > + > + @param[in] VariableName A Null-terminated string that is the na= me of the vendor's variable. > + Each VariableName is unique for each Ve= ndorGuid. VariableName must > + contain 1 or more characters. If Variab= leName is an empty string, > + then EFI_INVALID_PARAMETER is returned. > + @param[in] VendorGuid A unique identifier for the vendor. > + @retval EFI_SUCCESS The firmware has successfully locked th= e variable. > + @retval EFI_INVALID_PARAMETER An invalid combination of variable name= and GUID was supplied > + @retval EFI_UNSUPPORTED The service for locking variable is not= ready. > + @retval EFI_NOT_FOUND The targeting variable for locking is n= ot present. > + @retval EFI_ABORTED Fail to lock variable. > +**/ > +EFI_STATUS > +EFIAPI > +LockLargeVariable ( > + IN CHAR16 *VariableName, > + IN EFI_GUID *VendorGuid > + ); > + > #endif // _LARGE_VARIABLE_WRITE_LIB_H_ > diff --git a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWrit= eLib/DxeRuntimeVariableWriteLib.inf b/Platform/Intel/MinPlatformPkg/Library= /DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf > index 704a8ac7cc..f83090c847 100644 > --- a/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dx= eRuntimeVariableWriteLib.inf > +++ b/Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/Dx= eRuntimeVariableWriteLib.inf > @@ -10,7 +10,7 @@ > # Using this library allows code to be written in a generic manner that = can be > # used in DXE or SMM without modification. > # > -# Copyright (c) 2021, Intel Corporation. All rights reserved.
> +# Copyright (c) 2021 - 2022, Intel Corporation. All rights reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -24,6 +24,7 @@ > MODULE_TYPE =3D DXE_RUNTIME_DRIVER > LIBRARY_CLASS =3D VariableWriteLib|DXE_CORE DXE_DRIVE= R DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER > CONSTRUCTOR =3D DxeRuntimeVariableWriteLibConstruct= or > + DESTRUCTOR =3D DxeRuntimeVariableWriteLibDestructo= r > =20 > [Packages] > MdePkg/MdePkg.dec > @@ -37,13 +38,14 @@ > UefiLib > UefiBootServicesTableLib > UefiRuntimeServicesTableLib > + VariablePolicyHelperLib > =20 > [Guids] > gEfiEventExitBootServicesGuid ## CONSUMES ## Event > =20 > [Protocols] > gEfiVariableWriteArchProtocolGuid ## CONSUMES > - gEdkiiVariableLockProtocolGuid ## CONSUMES > + gEdkiiVariablePolicyProtocolGuid ## CONSUMES > =20 > [Depex] > - gEfiVariableWriteArchProtocolGuid AND gEdkiiVariableLockProtocolGuid > + gEfiVariableWriteArchProtocolGuid AND gEdkiiVariablePolicyProtocolGuid > --=20 > 2.28.0.windows.1