From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by mx.groups.io with SMTP id smtpd.web10.523.1671247067661784414 for ; Fri, 16 Dec 2022 19:17:47 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=dr/3A0FL; spf=pass (domain: intel.com, ip: 192.55.52.151, mailfrom: jiewen.yao@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1671247067; x=1702783067; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=J0Mj/0t8jbTFzFVEaKJVXck1c/cDXuZSkeUcIr1wmiI=; b=dr/3A0FLws1rVTnfogAuCEwROy4JGM9PMgNuGzPb6QgdGud59gkWpwC9 7FptR/XNHTF7RxKUq4tHCzPedGb5vCdzKttij1qETDAEqMTv3aNOa1+eF K8ovkaQhJCx8B06DYyAWG+L8Vlxqxv5ZRD71I0C8rcBFQF7deO8mBmvkN LliksGPGqSmn5XyFVxlbFuTwiCqfP8edB0qD32bjEaHK08o7jx0eWelI/ 6CQCD+q/1V4jO8vXkxFYb4hvKVvJZ9I7D+gOreUD0jL5t2e50kk1oKzQs RXF1du3Yh4pVmMKs3+Ru2SFaEZ/wv2wGqvOeOfbSwShIKn6iSyU9R6OvS w==; X-IronPort-AV: E=McAfee;i="6500,9779,10563"; a="299431707" X-IronPort-AV: E=Sophos;i="5.96,252,1665471600"; d="scan'208";a="299431707" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Dec 2022 19:17:46 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10563"; a="627758151" X-IronPort-AV: E=Sophos;i="5.96,252,1665471600"; d="scan'208";a="627758151" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by orsmga006.jf.intel.com with ESMTP; 16 Dec 2022 19:17:46 -0800 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 16 Dec 2022 19:17:45 -0800 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Fri, 16 Dec 2022 19:17:45 -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.2507.16; Fri, 16 Dec 2022 19:17:45 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=GjeKqao0JeupTGc9o3MaaRgGGuVNuBiMQ7EGHrH8M46RzG3Ba53GfjpvXZIRsX0Rgy4dS46R+JZmUfO9WnwlZSI+5WjiMEg7LeHMX6+gw+OZYsoj4rxZ2TRSHC6MzoyLqwh8ttbV/u3cvfL1hf2SNe5RuNrC7csDe0OWvFElo9axUGcQDL5YvF8JY7ScB5dyuCFIzZiLCjG3RjMJ8eeHieTpzlpzCmZOrX/buBwgDnDEO2TLBeMLMcRzjW4SrMJ+DKsVT6KVjEbD7txOFgC24lRwdCckxQrNPpI2wHs/di22X2V3yh0R0tMyq7Oc1ODbTmGIr9b+MBf3iJb+55ky2A== 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=Vffj5oCmvwei8+/WmQ+d+ZY5HDB1hae6MnyIeS50F7M=; b=HMgWCQelXtQCb+UFRUbnW263sE0fWTnLSfiMo6GKPkZUbOVojUF20CB5CDvXgI31koqXRhjMADUWFayuCgrU1tSwR/2jAOffimxBEgTliGWxbzV3RGIGqJUZHnaAB+e3EKR+CwfRRbrcWd6Xm0yxI8eOgUR08QcUSVmLpvVJ+LN4LS8NPXT3PYrh+U0i9V2tsThPVSyJZlpqWblTdTBcF5CPIDm9Iwiv2x6Uf/WviS+54ppLgzXq4qaan9qTb/wm2aUqqd9Kxm9li1Q6/MbOmmzdGWfDbABGkUyVikfeyaxOGIrv6J/ieD+iQeQPGLT1BfYz9alnRZufVAqtOAj5dA== 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 Received: from MW4PR11MB5872.namprd11.prod.outlook.com (2603:10b6:303:169::14) by CY5PR11MB6284.namprd11.prod.outlook.com (2603:10b6:930:20::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.19; Sat, 17 Dec 2022 03:17:43 +0000 Received: from MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041]) by MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041%7]) with mapi id 15.20.5924.012; Sat, 17 Dec 2022 03:17:43 +0000 From: "Yao, Jiewen" To: "Lee, Chun-Yi" , "devel@edk2.groups.io" CC: "Xu, Min M" , Gerd Hoffmann , "Tom Lendacky" , James Bottomley , "Aktas, Erdem" , "Lee, Chun-Yi" Subject: Re: [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Thread-Topic: [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Thread-Index: AQHZEJbIsLy//RV+80m2n9Ozk9GWPq5xarbQ Date: Sat, 17 Dec 2022 03:17:42 +0000 Message-ID: References: <20221215142723.9788-1-jlee@suse.com> In-Reply-To: <20221215142723.9788-1-jlee@suse.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: MW4PR11MB5872:EE_|CY5PR11MB6284:EE_ x-ms-office365-filtering-correlation-id: 466ef2a5-bd8f-4915-b0af-08dadfdd42ec x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: vMBbB93C8Hf3MTdiCq7RJs2xMNFLzP5vb67FTmAPrDV1fhDqcAHX+GYzEd0ILlM47boV3KiS/JbiC+dNSz/+VynH4pexQgDiPUOpVMrcBEVOTccdXi8jDHSTWdErzJtKo4usg+mpneXAz3leieZ/jqq6Yb1y0tZm9zAtKH8VCAQ23PzEgFLdAPTMrUtQBUq4+cGgU6ETNooknZcSuxlH1W/EQW4cYrXByFPv98SbcUSqhlWbJ8vOqL1AOG0e9pE0ge8tGbTZVd0maA3LuTt13B0uSCYqiEYOSUz5DECE5awmE14a7hFg7V1jM0V0W7OfpCFpFVou3ySnknRHC+zNQTJLRkBfq7gYIpi5nTv6qEVOVofJBW37fODz4d2FJCsoSUczgCkFlb+4MatZ+W0y6hK8eWgB/DlpHtbK7KAxNYJcKEjCg5VFSPCMmAgy5d+l2ipEWQNiMMH7cC/R3cjDPmLbSsaH0ZbxTPp9bVO50BT61MmF1xPofC9dr5mG+Tig9q6HKtJD2AoK8s8JAmYt7QSRUVJhPTkC44wPHlx8hZt5t9vet90tV96dmKPkCk9gek4N7rc6BsGk4IUK8h2/wTGS7e8oFjcWmAm8F18SmFqw7tvJnK4IC0oZXgxoygkGupExblB7DO1X1xaiQXlyE9Vs3hoNaMiIEYb28ibhhmU+z/3JZ3kAhCYSFbvOr4BylBPAppKK6qVZJhBFhyJ52Q== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR11MB5872.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(6029001)(39860400002)(376002)(396003)(346002)(136003)(366004)(451199015)(38070700005)(55016003)(122000001)(38100700002)(186003)(82960400001)(2906002)(8676002)(5660300002)(66556008)(71200400001)(66446008)(4326008)(26005)(66476007)(86362001)(33656002)(64756008)(9686003)(52536014)(8936002)(41300700001)(110136005)(83380400001)(54906003)(7696005)(478600001)(76116006)(6506007)(66946007)(316002)(53546011);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?3N4VK+DXoo1d0A7FlQLUw5xuzsidMnR2tLqsdCGqnSf5TyukYWMhfuFDRCVI?= =?us-ascii?Q?vcoNfceKZnnbCvqS1VPNvb494v6XgRYxqTsaRZDSwfi9/UlYS0DsHWGSFhLQ?= =?us-ascii?Q?Uel6d1ROvVkCuuEaA8sdnCI6fwfCZLtBZy/8cj1bUlHRk4MnnLNnQuQL/mxY?= =?us-ascii?Q?A7RFjzlcGAinDhBHjjR03/Ui/5/OU8LDIMmJ8/YLTD5B5AXr425BRaFiEc87?= =?us-ascii?Q?0snb9yDcClWREebU0C47XFb+0+HMYirtAn/poQajbE+RjsbsKraeRdXX5g/s?= =?us-ascii?Q?hitUg3DLvI6RMHKyMy3Yxn21waaQrkZ8yt4DgCzjr2HMSz84R3UDlq6av9Ve?= =?us-ascii?Q?d1MZfg50e9RiE2yuCmU6KgaIdUWvRD+sHWvubcIi1G16tsWavdlXk+vMDcll?= =?us-ascii?Q?eCHgEEuh1fl00evNWdFubUGvseZFN4WRsuee69JMA/Y2lJHDCS+STbmvbzIt?= =?us-ascii?Q?OH3dJkDgr61m3tLbQ+JY84CkYQEvHvvUkt9fWFwtqEl4I4HskgrXrRveKVGl?= =?us-ascii?Q?vGBmZCSWqxX8JGYbohAKUVDsgHVUY2Z7T2UPR7n5Ysg9aAUnuV6uZgOc/x/6?= =?us-ascii?Q?7azFC7AhrUkEFN/q7hPx9RPKGYb5KvWPC4N7C3WQJVAGqz+zGIHFOtb+IUPM?= =?us-ascii?Q?QbHg+xXw2t7iKCcleubOL4TdcqosgjsKWqR2zJvTYJdoKEPh/JqKcieuC7ag?= =?us-ascii?Q?f1hUa6bxqrwxCftjTAppICrKACtXHhfmkDn+0i3RJRfHqd2rMmqBerBYzllh?= =?us-ascii?Q?S5N6P4X47k75l3E0r2f4aGORvEK104sfZYZaSo3zYTbziPUm3rucLkj37PL1?= =?us-ascii?Q?hoggjld9ripzDV8p7qBbPnm1Hv5MirKZ7xu1ZVaUFK6aOJ6wsDoQTJmxo6WG?= =?us-ascii?Q?KScpbW9cV+XhAisE3qaUzUKSOfXRB+v6nKJtyUTy+Rr3DIArlbta9a3DCa+d?= =?us-ascii?Q?nscN4QABsG5mGyfic0fgrsz8il31Mb/J0B/4HEuxolb9hG17O8xq9mP3F7Xx?= =?us-ascii?Q?RuISmmQvH2CyDxc47banGiOcG4ZOMT101THjE5t7S056kuZxwKE4vGoxy7Z2?= =?us-ascii?Q?nEkIf/nDXwAZVj1DRdH4u8+Iq9U73ZGEX2Mak6sDiRA01aI73NXpjrFUg2xI?= =?us-ascii?Q?gPELYlKX1bjpMkruDAQJ2lZQFNbT8C3ugxOiEot7FdVLMFd1Hf8nYNPr96Gl?= =?us-ascii?Q?8gahwvS/75m1HaW+Nw/aJt0xPbodr3DqG8shmVYnqtNHnn9QvWTCzIb7/Uv8?= =?us-ascii?Q?zLSh4/HErUkeptj/TLGN2F1nqy3CwREElrIGDI+dPP4bYFu3IUPLglAWfUP2?= =?us-ascii?Q?kWBZ68TOzsnTZP3PeI4d5gW3FojGBNQQgXKY8yF92rJ3aahHx0KcHK85Mg8B?= =?us-ascii?Q?0AWxLlJUIjMbeVk44qBUZ/TH9qIr32WWW7k1nlyqtlSljKyOSV+dA1x3YhzU?= =?us-ascii?Q?X59AePZlQkDHXZZKWhz7xbiwv4aks/HeS5FQ4+z27cUBGsNAgSdXGcPV7afi?= =?us-ascii?Q?Taan7stor6gYA7vF+r0XYCJUHDZelHcmT87JCgGpIZxL8zCFKx1EUAiJ2LV6?= =?us-ascii?Q?HNuXimswE1dB7k2wyKHp3sQgRJECi7TDlyl/yt5V?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR11MB5872.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 466ef2a5-bd8f-4915-b0af-08dadfdd42ec X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Dec 2022 03:17:42.9452 (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: jyn/6e86VtdBmazIpoK7972aWY+Hjj3EcCLKZH5jHGQ5TW6KGPBysAdQFigt3LGLf/TG+jNT+L7U6LdivWFt6A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6284 Return-Path: jiewen.yao@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Thanks for the fix. Reviewed-by: Jiewen Yao Question: Have you run tiano CI by yourself, before submit the patch? > -----Original Message----- > From: Lee, Chun-Yi > Sent: Thursday, December 15, 2022 10:27 PM > To: devel@edk2.groups.io > Cc: Xu, Min M ; Gerd Hoffmann > ; Yao, Jiewen ; Tom Lendacky > ; James Bottomley ; > Aktas, Erdem ; Lee, Chun-Yi > Subject: [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity checking faile= d of > NvVarStore in some cases >=20 > In the commit 4f173db8b4 "OvmfPkg/PlatformInitLib: Add functions for > EmuVariableNvStore", it introduced a PlatformValidateNvVarStore() functio= n > for checking the integrity of NvVarStore. >=20 > In some cases when the VariableHeader->StartId is VARIABLE_DATA, the > VariableHeader->State is not just one of the four primary states: > VAR_IN_DELETED_TRANSITION, VAR_DELETED, VAR_HEADER_VALID_ONLY, > VAR_ADDED. > The state may combined two or three states, e.g. >=20 > 0x3C =3D (VAR_IN_DELETED_TRANSITION & VAR_ADDED) & VAR_DELETED > or > 0x3D =3D VAR_ADDED & VAR_DELETED >=20 > When the variable store has those variables, system booting/rebooting wil= l > hangs in a ASSERT: >=20 > NvVarStore Variable header State was invalid. > ASSERT > /mnt/working/source_code- > git/edk2/OvmfPkg/Library/PlatformInitLib/Platform.c(819): > ((BOOLEAN)(0=3D=3D1)) >=20 > Adding more log to UpdateVariable() and PlatformValidateNvVarStore(), we > saw some variables which have 0x3C or 0x3D state in store. > e.g. >=20 > UpdateVariable(), VariableName=3DBootOrder > L1871, State=3D0000003F <-- VAR_ADDED > State &=3D VAR_DELETED=3D0000003D > FlushHobVariableToFlash(), VariableName=3DBootOrder > ... > UpdateVariable(), VariableName=3DInitialAttemptOrder > L1977, State=3D0000003F > State &=3D VAR_IN_DELETED_TRANSITION=3D0000003E > L2376, State=3D0000003E > State &=3D VAR_DELETED=3D0000003C > FlushHobVariableToFlash(), VariableName=3DInitialAttemptOrder > ... > UpdateVariable(), VariableName=3DConIn > L1977, State=3D0000003F > State &=3D VAR_IN_DELETED_TRANSITION=3D0000003E > L2376, State=3D0000003E > State &=3D VAR_DELETED=3D0000003C > FlushHobVariableToFlash(), VariableName=3DConIn > ... >=20 > So, only allowing the four primary states is not enough. This patch chang= es > the falid states list (Follow Jiewen Yao's suggestion): >=20 > 1. VAR_HEADER_VALID_ONLY (0x7F) > - Header added (*) > 2. VAR_ADDED (0x3F) > - Header + data added > 3. VAR_ADDED & VAR_IN_DELETED_TRANSITION (0x3E) > - marked as deleted, but still valid, before new data is added. (*) > 4. VAR_ADDED & VAR_IN_DELETED_TRANSITION & VAR_DELETED (0x3C) > - deleted, after new data is added. > 5. VAR_ADDED & VAR_DELETED (0x3D) > - deleted directly, without new data. > (*) means to support surprise shutdown. >=20 > And removed (VAR_IN_DELETED_TRANSITION) and (VAR_DELETED) because > they are > invalid states. >=20 > v2: > Follow Jiewen Yao's suggestion to add the following valid states: > VAR_ADDED & VAR_DELETED (0x3D) > VAR_ADDED & VAR_IN_DELETED_TRANSITION (0x3E) > VAR_ADDED & VAR_IN_DELETED_TRANSITION & VAR_DELETED > (0x3C) > and removed the following invalid states: > VAR_IN_DELETED_TRANSITION > VAR_DELETED >=20 > Signed-off-by: "Lee, Chun-Yi" > --- > OvmfPkg/Library/PlatformInitLib/Platform.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) >=20 > diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c > b/OvmfPkg/Library/PlatformInitLib/Platform.c > index 77f22de046..6963c47e0b 100644 > --- a/OvmfPkg/Library/PlatformInitLib/Platform.c > +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c > @@ -702,10 +702,11 @@ PlatformValidateNvVarStore ( >=20 > VariableOffset =3D NvVarStoreHeader->Size - sizeof > (VARIABLE_STORE_HEADER); > } else { > - if (!((VariableHeader->State =3D=3D VAR_IN_DELETED_TRANSITION) || > - (VariableHeader->State =3D=3D VAR_DELETED) || > - (VariableHeader->State =3D=3D VAR_HEADER_VALID_ONLY) || > - (VariableHeader->State =3D=3D VAR_ADDED))) > + if (!((VariableHeader->State =3D=3D VAR_HEADER_VALID_ONLY) || > + (VariableHeader->State =3D=3D VAR_ADDED) || > + (VariableHeader->State =3D=3D (VAR_ADDED & VAR_DELETED)) || > + (VariableHeader->State =3D=3D (VAR_ADDED & > VAR_IN_DELETED_TRANSITION)) || > + (VariableHeader->State =3D=3D (VAR_ADDED & > VAR_IN_DELETED_TRANSITION & VAR_DELETED)))) > { > DEBUG ((DEBUG_ERROR, "NvVarStore Variable header State was > invalid.\n")); > return FALSE; > -- > 2.35.3