From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web10.96648.1671000826438457539 for ; Tue, 13 Dec 2022 22:53:46 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=SLfqOdoz; spf=pass (domain: intel.com, ip: 192.55.52.136, 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=1671000826; x=1702536826; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=7kb6KsLa2YqT2GLEQyz/vTBTM6v41SN7ho2VSLc1fbU=; b=SLfqOdozr7yshiQOJ8H8zvIsge/nW2FH8OWbuADfE1B6PzamZtTXpjGl BSYlhXOntYYnno67JnqCIDzDMpuNltIT4LQl/UG7HUxwLKaxQQzl/g4NZ UVhtHqXqCGxXbQAkgveDGlVoYPyYfSi5yEx1jftTO47A7MmLILKkEJx9U drHwk7CTMKRc0lkUzYiHkG9AG5LU8P9nPPB6Jg5kjJKrNrVA+j58WvGCB 7n3YYFnxK0+24tW1KU/RD5jSWCq/kqZ+ulCpwLV/swdhvr1yxNT4kA5yU /F22fsRQwDBMvYuNfbLGamorp/1svRO/hBGP1zU69RCelTmkea4VeSAV9 A==; X-IronPort-AV: E=McAfee;i="6500,9779,10560"; a="298010352" X-IronPort-AV: E=Sophos;i="5.96,243,1665471600"; d="scan'208";a="298010352" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Dec 2022 22:53:45 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10560"; a="717503467" X-IronPort-AV: E=Sophos;i="5.96,243,1665471600"; d="scan'208";a="717503467" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga004.fm.intel.com with ESMTP; 13 Dec 2022 22:53:45 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Tue, 13 Dec 2022 22:53:44 -0800 Received: from orsmsx610.amr.corp.intel.com (10.22.229.23) by ORSMSX610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Tue, 13 Dec 2022 22:53:44 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx610.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Tue, 13 Dec 2022 22:53:44 -0800 Received: from NAM04-BN8-obe.outbound.protection.outlook.com (104.47.74.46) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Tue, 13 Dec 2022 22:53:44 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=JuEt3pcB1qbWSsktWplqMiJlPXEEOzYyQk/60HSEdggJCi/4whZMVuQ4QoPjDXmMiJTBHOgLNw7VOXpWMARPVeddEqM4hUsfnk2cA96QLBVD9nyJ/QNYnvmuF/6y1Yf0nGIZzZef4FCITgqUPdaKr9kDVV0uUO6pGQIsLAY9tUmqU+F+ddIOL/wEfWxCXwVUm1PFkGyatT91JN4J9AuK5G18iuEwLDQ0NH/YGgQbL+0fSA4y+sVRhQ801s3zqLQQBDkuWIkEhTMIathZI1PTWloAM//NmSKZkGPwxBCXQkXJV192Bp8ApnpStkr51RsuciIKaKcHfGDGBoKJxjc/NQ== 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=/JGer00ilxfl0Jl4KGn5b/JUlIx8n60yAvPgrO1SdQU=; b=oXnIxx6s1L6e9cNjdLJU+VZ5q7pEJoW3gYFfWmIDC4xIfTXiJIgT/FBd546r5odXjiWDcCFPX8k3nsmgqSqwgPDRmesReTPN9Md8ixYYCdUQUFvo8BhKHh2iCLNA1mNwNHojbLXhcjHbqxd3f/aFT3zfjLrvuCd+Aqqv71tYag5CZ2kVZ29qsKPmpTmK3QCFzZep3E5FhEsgWVg5UXZbgQnWDZTlE0fTJdVjdxZyjsrGtvX4Wi+OcWRssVL0wFb69gjPepfodHMYRq73YemCuj11fktDlx/zr74NaUo21gQdJLCFH9Swalj/T9GR0sREoeFvTxA18xq2lHNOwU4LzQ== 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 CO1PR11MB5091.namprd11.prod.outlook.com (2603:10b6:303:6c::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.19; Wed, 14 Dec 2022 06:53:42 +0000 Received: from MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041]) by MW4PR11MB5872.namprd11.prod.outlook.com ([fe80::5f56:1bdc:2eae:c041%9]) with mapi id 15.20.5880.019; Wed, 14 Dec 2022 06:53:42 +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] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Thread-Topic: [PATCH] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Thread-Index: AQHZDwtT/dtmjUi7C0+SJJb9BIm1yK5s7sMA Date: Wed, 14 Dec 2022 06:53:42 +0000 Message-ID: References: <20221213155502.29548-1-jlee@suse.com> In-Reply-To: <20221213155502.29548-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_|CO1PR11MB5091:EE_ x-ms-office365-filtering-correlation-id: 6f83a25e-9aaa-41a8-24a7-08dadd9ff006 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: ogYljK0aIW6zFnjQcKovum1HFO4YRmo1zm5DodDMVNB3ta/cr6AkedLPH66H+lJRo5vYc8Ka7R8PRlXbJTKjQyFLAOJYMR+zrGoKBUsS98cz4nzR8uajBv1B6PRGjxfdwLBzpO8XQQaUxsWU9WArSB3hl8ghfshZu451Bw0e4NrE2VaNkZ2kviCjmOzogS43zkl7UtpE+pYfk8sPG9mPVFNnKrQnIL2imb5lrQIOr3jnRvaxTW20Kra4ST2gC4SNTaLfh42D3wKO4IPjxVQCBr5fs4mcWWqERmYuWP5agwgkIR46LxgAaOFVv9s24rRv5+rUEZWE1xBPguTkQ3P0qaqSfENjMhkP+qnmWjBePZ+U90EflshIWFgBXk7S/sfygCN7gVJERbCsTjTAtUru8aow1JM5C2p2MtNmyNBdsWj/rz1iRNQH2qR+AasR6Ms7NhyH97mAg+TjKPYoEY2g+YRFa2lEOtCQcZh1PXDN+rpbsQ8cHg88w8E7IQM1o8PEJoWXypAcqw3kVHot2vO5/x26DrfMSXcfHXI0BwgX8Ee4Oj+r0o0ZFCLksQAieJKBnPIfYfgdW8t2642PoE5zqegHo4wwlI/dep3x/+ZSZCgUClJpFvjLo1etrw5shmAuqAY30dPSu3mannII25WwAvIAHY2RwWGPP/upMC33zpW8bbZCoePGTzkigthKpMEuJ6vpuTHVTaZCac8MDzk7zQ== 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)(396003)(366004)(136003)(39860400002)(376002)(346002)(451199015)(71200400001)(54906003)(478600001)(53546011)(6506007)(316002)(7696005)(4326008)(110136005)(66446008)(66946007)(76116006)(66476007)(64756008)(8676002)(66556008)(2906002)(41300700001)(83380400001)(5660300002)(8936002)(9686003)(52536014)(86362001)(122000001)(38070700005)(33656002)(26005)(186003)(55016003)(82960400001)(38100700002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Fx/o/WQQ7jH0qWIRfES7fmIB0Mr2d0yX+fjWJ+RMd1m+XVtnVYmbLiyVh/+2?= =?us-ascii?Q?ltxZ4Q8waht4KKjciLXZe5l8G409H2sQ81aJFsnE5Ek1L+ubl6JEcDSKnQxA?= =?us-ascii?Q?FchPhoB7ubn7GtsjwTy0mLjtIyZ8+DoS+HKj0mCie0G3vq6MQM2bSJWLCmie?= =?us-ascii?Q?zlc6FhV0zZfgfCIZnd+qlBztQPMlPBDA+Iq0TNs0fNMMNjUrOXx1UkhCYuJH?= =?us-ascii?Q?402SxqJNqukxO/YvC13AZ4TMypMyAifwUa8T6mQ1cqOzmcMLfUSUpRGND/FV?= =?us-ascii?Q?PSdG1waDoxpyuWhLWflba6IafRGT/AeZEsewoSuQicDNUJDGEinlgTp9fYgc?= =?us-ascii?Q?7jtZVDkmDWY59nhmvsHXWR8Rh/BHo1dtN41n/4K1CTKErvRqZ8xwjv2U9fhK?= =?us-ascii?Q?B84J4pdMAHyPg8mquUeR3v7XQHPPi78EM0RV7mgZjAM1qNvHiDLGnIaZC2mo?= =?us-ascii?Q?IggZqnvB/iC3N/t3+Jj5Xo+Q/wj/ONN2IN0I1c2bFlOBcWGOCOAgci4LzGas?= =?us-ascii?Q?o4uEWrSsDvmtFeHDcVIusaHWTjhe+n1ZaxDz8rYnSBtHkTQlStVJOvTcnlQO?= =?us-ascii?Q?4IZfxLzX6KFW+nvCrmoS5UPsasK1loGlXnEviLfgHtcVxMhgyPpA+oM072lO?= =?us-ascii?Q?YbnTSS2GQY3jDWpOMX6EGpBr0kbn88Vae+Gcw6ozg7VVCEDBQKZ61EPJ8WV2?= =?us-ascii?Q?jhwU0PWwgRVM2ycQ1SpwdtxqsEFrulMDljVYpUdlNFkaICCIjXCcqbNST/G9?= =?us-ascii?Q?EhiKzLG2Spk/GnUHaw107+yZm//pbMFljbL3a35gTpjv2RuZ0IoXgU6Ycjof?= =?us-ascii?Q?AUyLKw0oAwUFFhYFPrnvXaLbzgemgMCh1XjZSKyVpIJTHsiwnrUR1czk/pKb?= =?us-ascii?Q?2ckmMF2aLcv6S24aB+PVYp4rczDeMToLfQkdmkk8ClOroIm+3Zfufb5o89zY?= =?us-ascii?Q?xMHKnYcotHA3hSLc1aNc6pwzBxUXcCjOdNMl3O8vmRl40wwMlRDPgyvQEXCh?= =?us-ascii?Q?dMGfEkQu7Gg/1hKmHp+8zvVjaNd3AMhAf1Nro8Xepgd/Kh0CgzWBWa6vwkzh?= =?us-ascii?Q?dAIcJJVYN/K4EGuiLhxqeYJFl0YFaw3YTgHp332+GLiptMkO5jWvJHAve6Z7?= =?us-ascii?Q?OZh8S5AfbOyJ9dZxA9VwRLniPF6ltQ0rZWanJmMoHmsrPdIVgRb3Gt5DVyjz?= =?us-ascii?Q?duR6LT55aZR5g1P3buu8GcZWqUvjNM2ngZVm5g/7fr3ONjmBNnWkzhRYce+u?= =?us-ascii?Q?iydA4uE13NLRoFagEZd1cONY6UpQwtqk+w1Z+6tvTTvqtn8SVrVaSFvHSKdo?= =?us-ascii?Q?sWDgN30/GaqukBs1CmreRPdmPVmIRt/O5fQuXdT4Zg58WEAWetK1b4dKDIPC?= =?us-ascii?Q?5xPHcIgscPMAnnJiH+UZZISELkwZ4PQ6OQRR1lZ3gvGeX4olqe9F3c1BJ8cN?= =?us-ascii?Q?7z3CFhEPzsUm2PWVxYhPJR0HDFbC+xFwv6DSNxaoT1OQWMmhYrLkQ8480Qr7?= =?us-ascii?Q?f4r1nk3LyKSndtnuK01unS5kwPX1fDnsnCmN8/hZP74I7y4YSMlbl7qM0FWJ?= =?us-ascii?Q?NGtgxjPmSJsMDpNfBrIO8L+S9BWiQusklkqRP5Oj?= 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: 6f83a25e-9aaa-41a8-24a7-08dadd9ff006 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Dec 2022 06:53:42.2350 (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: JmJICuJXIU3etzqWqU63IF4aSL513mQ66DDDmOJjG4TCHhtZLkG1Zolmdh2Zcs+FiRhbZHD3/EzMEranMyCVSw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5091 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 Hey Good catch! I think we need handle below valid cases: 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. For the patch: > 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))) > + (VariableHeader->State =3D=3D VAR_ADDED) || > + (VariableHeader->State =3D=3D (VAR_ADDED & VAR_DELETED)) || > + (VariableHeader->State =3D=3D (VAR_ADDED & VAR_IN_DELETED_TR= ANSITION & VAR_DELETED)))) I think: A. If we allow (VAR_HEADER_VALID_ONLY), then we support surprise shutdown, = we need also allow (VAR_ADDED & VAR_IN_DELETED_TRANSITION). It should be ad= ded as well. B. The (VAR_IN_DELETED_TRANSITION) and (VAR_DELETED) are invalid state. The= y should be removed. =20 Would you please double check? > -----Original Message----- > From: Lee, Chun-Yi > Sent: Tuesday, December 13, 2022 11:55 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] OvmfPkg/PlatformInitLib: Fix integrity checking failed o= f > NvVarStore in some cases >=20 > In the commit 4f173db8b4 "OvmfPkg/PlatformInitLib: Add functions for > EmuVariableNvStore" > , it introduced a PlatformValidateNvVarStore() function 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. > 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, then system booting/rebootin= g > will > 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 > can see there have some variables 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 adds > two more combined states to the valid states list: >=20 > (VAR_IN_DELETED_TRANSITION & VAR_ADDED) & VAR_DELETED =3D 0x3c >=20 > VAR_ADDED & VAR_DELETED =3D 0x3d >=20 > Signed-off-by: "Lee, Chun-Yi" > --- > OvmfPkg/Library/PlatformInitLib/Platform.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) >=20 > diff --git a/OvmfPkg/Library/PlatformInitLib/Platform.c > b/OvmfPkg/Library/PlatformInitLib/Platform.c > index 77f22de046..2af4cefd10 100644 > --- a/OvmfPkg/Library/PlatformInitLib/Platform.c > +++ b/OvmfPkg/Library/PlatformInitLib/Platform.c > @@ -705,7 +705,9 @@ PlatformValidateNvVarStore ( > 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))) > + (VariableHeader->State =3D=3D VAR_ADDED) || > + (VariableHeader->State =3D=3D (VAR_ADDED & VAR_DELETED)) || > + (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