From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR02-VI1-obe.outbound.protection.outlook.com (EUR02-VI1-obe.outbound.protection.outlook.com [40.107.241.76]) by mx.groups.io with SMTP id smtpd.web11.66549.1671774867499512598 for ; Thu, 22 Dec 2022 21:54:28 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@suse.com header.s=selector1 header.b=S3sfXDIY; spf=pass (domain: suse.com, ip: 40.107.241.76, mailfrom: jlee@suse.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OEVcUWMgKFfFQnM8MWuhGqmTEpSFBWXVrfqanPPpxT2gOM89qC3vhoHniaLf2A07KNTmY9Qzly/txzMLAKSyC4hJpTSTN6bRPAnnGuHDOwoCE+Y4Q3H4IhwV04O0tbL/bxRF5an+mVibCCSGON93SPnYqPiDYXKAiSpB0FzEMZtcGZJiy3sdpW/RJgT9Tjc1NDmRiGW52bPLdHDzcjP91tPEF88igxomj5yrUXbA4dEBuAnvWF8yB7Ig9dzHACBzc2+0CE/z/9CrB8CU6iIqpr/cEpbK50PgIG5e74TTcClI5xu6FSWzNvwKCPRBDYJ7LrczUxNxZyzcy3GS0yshHA== 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=Qisl0DkoQuy4jQFb9bsUI9dNisd0ZBqZDW5Fc9IUeJU=; b=oQsoeQoRe+1mcKMBiiQxWHfHxBI5sxQiAuMn5bPJAFMzhjvZLf55ro4tdIOwcysUN0bSEySSO2q3Rh7FLDioEmMcvfE1qryaJfdRWzEu+AMuMVte7tTODdB0WqF18U1wS5yBh1XaQXEV6gDs3KVibRcADVv6q0IRaRfS3SNYOhx3rGcdturoDn5foZC4KHl+7+/TQIDFxBvGGEC8Bl6Pee46loWyxn/bSW2UoKA4mI0Z1Xj6JA9uEZF5++wmAxz3SSdRZii9yVu/2rzz3RQST9ktBKbdL5g5qHtR6bGPpW7jsp1kZmBZCe+44wLvRnyS4PTSVsSF0hh/PA1zN06VIQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Qisl0DkoQuy4jQFb9bsUI9dNisd0ZBqZDW5Fc9IUeJU=; b=S3sfXDIYMEDVlTB4FzI3nGkDom9mPpK6SkDRdTkQpehcBEsmDjhIy8N1zj/QT6qXWNXPuD09GGD1CwiAbPdlJkLa7Us2llNR+HxH2AoYsE2Fmt3QGcKu3OU0BU7nC644WeTrmDvuUGC+6n47JyxBMmLs7NGmFRpuu2uVRRctbzS7gOLfe4BIvD0BRTdSnvbLMqRusWt687nQGQ69ZrcFGiYL0M2KPPq5bCNCB/DSyWez5oP9ctwn1QzZYZhOZ9QHJV7muF/hLGyccaJMTfC9Xu3NckMhC2WGAxbu/yr7vVSAAT/rwh6kS/O60uCoNJMfqhSnQRP8V+qQu4muOu/f4g== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from VI1PR04MB7165.eurprd04.prod.outlook.com (2603:10a6:800:125::13) by PA4PR04MB7759.eurprd04.prod.outlook.com (2603:10a6:102:c6::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5924.16; Fri, 23 Dec 2022 05:54:24 +0000 Received: from VI1PR04MB7165.eurprd04.prod.outlook.com ([fe80::94f4:6be1:e54:4811]) by VI1PR04MB7165.eurprd04.prod.outlook.com ([fe80::94f4:6be1:e54:4811%5]) with mapi id 15.20.5924.016; Fri, 23 Dec 2022 05:54:23 +0000 Date: Fri, 23 Dec 2022 13:54:17 +0800 From: "joeyli" To: "Yao, Jiewen" Cc: "devel@edk2.groups.io" , "Lee, Chun-Yi" , "Xu, Min M" , Gerd Hoffmann , Tom Lendacky , James Bottomley , "Aktas, Erdem" Subject: Re: [edk2-devel] [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Message-ID: <20221223055417.GS11807@linux-l9pv.suse> References: <20221215142723.9788-1-jlee@suse.com> <20221220070958.GJ11807@linux-l9pv.suse> In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) X-ClientProxiedBy: FR0P281CA0133.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:97::11) To VI1PR04MB7165.eurprd04.prod.outlook.com (2603:10a6:800:125::13) Return-Path: JLee@suse.com MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: VI1PR04MB7165:EE_|PA4PR04MB7759:EE_ X-MS-Office365-Filtering-Correlation-Id: 23142d77-0ca5-437d-cc0b-08dae4aa24a0 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 4rYiP6BJEJFIqBIXkuK+JPhqepDyWhfVGgFg3oBbY5sNboF9U6DazyWTMuoErTRfMsp6hM5hhLNSiksE2CGpOlARCP2WNSNStibWQXtSG4B6d6t0k42M/bJbpcRV5aDjECgewhyKzR83zD99zIHuEzsZ5qwLHOfGRZQbFDeZB0vg8Ge8wLdjfi18GKFZURAcobxmSx2Aj0W+lbZSmQoy7PDyy6pAzzeO/m2PuCYtbc+r5NPrBKzLE2SNGaW3l8h6nhRPJ4pAZkIGnhh8Vwax1xgeDk04BNdWP0dJwwl4EhB285yf2n8Boug22eP9tDQmFz4lGKXnsVTZSKB+9ac9jCLTxmjM4ikL3FpcxhBgv2eFGVwVr4ZrFpirQ6eyWqpvrkwoCiuOmlo4vWZLsnC/bNBayKe+b9Ipd8NT0DkIPnWEBkNaC/RsiCLg8c4ouODYs8dMk/vbaEMb6wqH0MvCKDviiinrKkS+hd8fRdlpbtXCgTZmzimqSwZqBWkWtuDNMF7DePk+ZUhGURdkofIEZ1vYe13dgBZTIq7Nuyya8WvDoGqntINRY3dU5GNmsTNAHnZvHIWx0ugGcuVg2wqd/c0xYi1j5Ul74svreieUFgwer91IDnh16ViVQ5l5SWCKd7xbskla2hvhppMXpK2mpyFQoRnxvyHeA8oRgXIk4A+22m3bbp/tpxvSm+2meN4NvwJD/jYNMRNPEAbTemWqNA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR04MB7165.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(6029001)(376002)(39860400002)(396003)(366004)(346002)(136003)(451199015)(36756003)(86362001)(6486002)(966005)(186003)(478600001)(26005)(33656002)(6666004)(41300700001)(4326008)(6506007)(8676002)(66946007)(6512007)(5660300002)(316002)(66476007)(2906002)(54906003)(8936002)(66556008)(9686003)(6916009)(38100700002)(1076003)(53546011)(83380400001)(66899015)(43062005);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?T1KJ9iLqee0UEXYfzIjhzL+cOzdB2AcRcEL7pOXQ5e/CqyPyU/MbcrGhSYo0?= =?us-ascii?Q?rkT4RnliLbRXetqeTjJWbapOnjRQTT0omQvXW5kt+0N1tJH3ASSdaVjXrJLT?= =?us-ascii?Q?wlOoVNku09CdfPQflFH63UF4tIiP0kc7dqstmMj8euaYQ3D48QRfhLFgWmxH?= =?us-ascii?Q?KGVy5PiEiZb+z2hjGIwidOigd/gDBUmE+zwN+1AdEpfHqOJxneHKeCs1nY62?= =?us-ascii?Q?+wqTuT1mkltpIxdHgxAIFcr/aOoZ6Ei1EPZR2Tw4ONhHtuG+FKPZlyE7AF/v?= =?us-ascii?Q?VZTbvm222EgbRdGaXLjr9H/itltfu3Kycj803Dh+0bnuybCmYUWq0ehB2zHt?= =?us-ascii?Q?R03ejrn34a3XIfsU0quAtOiGojdnA5Bce/oVqH33l/M97KVThedqR+TnMa5R?= =?us-ascii?Q?8fhl+u+dOI2k4nVtUQtpikXJEbQeTEFvYS1gslY0wMt5h/bMXbIcbH2ebZbA?= =?us-ascii?Q?bYxlZGZwz8h0YNBy/4CW4Od3E7q7GqUX+2NMZP2IypSYu75m8J5Kw2JhTDHg?= =?us-ascii?Q?XeimPf73n2/TXz/nVRC/zBeE4HxFardfwkLjsZfTjGv7+dVZac1xpWLhnj+Q?= =?us-ascii?Q?iEOzFID1k0R/wmcBO3cASefsO2okRfLNNNYcMNS+uV04yI4KVGDncS0iSxC9?= =?us-ascii?Q?h3ieixglZhQcYynqOPYRYcsPX3HLFyO6zbkYZJEk+j45lFeyoZEKhnf7/8Wc?= =?us-ascii?Q?gZMOvzw1N2pVNYoOM4YWEVWLvxz+3Rb7o6c2Ngt0SlyDhZTsUFjJisE++BVY?= =?us-ascii?Q?JH7udnHD6Mc4U8Qh/ZVaWFvDC6ff9r0LDiRtvMRCKGRmOCczKs1YcsZbIx9X?= =?us-ascii?Q?HAU6umK2aCKp0H07248CmqO79qlPB72ZX/9oOEaLOwHCI86Q7xTq++c0TBh5?= =?us-ascii?Q?EmE2QkrcJuXYfPj9c/BMmaNoCRxpRG27DFgHnxGxpK8eILIsOZ0ywU/i9jbP?= =?us-ascii?Q?ooyzCwWomAdirXO2TD7Qg+DADnqu2kKYBwhaFGiqpZw9U5fqwaV+e6MOnSbp?= =?us-ascii?Q?n8OKbZzqsP5xy0j4Fk66IKcUgZeJui8Vf1yAqkq/YiaoN+NvkSir0CI5jFIk?= =?us-ascii?Q?LuatAnReLkFUNP3p7yK6TZJmlWeFv8maCi/reMdy7q0Lgyn1vW+MWgRR0/Ij?= =?us-ascii?Q?HMiH4X81tAyEmpMQKY0YvSSBy/frqtfdTvcrTDEhdRHunfdz8P9mis0f+wtG?= =?us-ascii?Q?Kl/CKm5/pgxyMAXJp3SHa6iajb+ny//GA2xPMxIaduX/AJeT+awqI07FPRlI?= =?us-ascii?Q?Dzbxrn9EEvtr49zHigSg2KgBzEZw1+8aFfMzGmbkVNPhaXIQqTmfqr+Pd6wF?= =?us-ascii?Q?q5nu3RT14n3P/7tM8nXrzoqZZ/1ZMin91tOX3h16pSVFtDBQjDBH62omsnUw?= =?us-ascii?Q?NJnWYXvfOJwXXFYIbfIv+2RCo9jooMpyrsWd/ZdsY0ZCGg82vpuOzeXNXBmN?= =?us-ascii?Q?YUUrEqN//kwyEZf5hq4aUMMwMNe2auUo5tdJcyxqZ6EgoyvRIDOBK+zdE6Xb?= =?us-ascii?Q?XdHe+ANSsaol6wmI7eDhcgFbJmcj3u4KQN/rq4umYfnGpd65LLe05Aqr5Oum?= =?us-ascii?Q?ifHT3nZVC/cb1QY38cw=3D?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 23142d77-0ca5-437d-cc0b-08dae4aa24a0 X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB7165.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Dec 2022 05:54:23.8468 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: f7a17af6-1c5c-4a36-aa8b-f5be247aa4ba X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: yAEWYIhvcgwaYnb8A1002LRpsHNRQ83yiJw5UKxc/TVH+iDThI/VOs9VgG38CL+b X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB7759 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Jiewen, Thanks for your information. I will follow the process. Joey Lee On Wed, Dec 21, 2022 at 08:46:04AM +0000, Yao, Jiewen wrote: > Hi Joey > You are welcome. Thanks to catch the issue and provide the patch. > > One thing to clarify: The patch submitter is expected to pass CI, *before* submit the patch. > > The benefit is that: If the patch passes the review, the maintainer may merge it directly. > > There is no need to trigger another round CI-fix, V2 patch submission and review. > > Thank you > Yao, Jiewen > > > -----Original Message----- > > From: joeyli > > Sent: Tuesday, December 20, 2022 3:10 PM > > To: devel@edk2.groups.io; Yao, Jiewen > > Cc: Lee, Chun-Yi ; Xu, Min M > > ; Gerd Hoffmann ; Tom > > Lendacky ; James Bottomley > > ; Aktas, Erdem > > Subject: Re: [edk2-devel] [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity > > checking failed of NvVarStore in some cases > > > > Hi Jiewen, > > > > Sorry for I didn't create tiano CI on github in time. And thanks for your > > help to merge my patch. > > > > I will create tiano CI in next time after getting our Review-by tag. > > > > Thanks! > > Joey Lee > > > > On Sat, Dec 17, 2022 at 03:17:42AM +0000, Yao, Jiewen via groups.io wrote: > > > 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 failed > > of > > > > NvVarStore in some cases > > > > > > > > In the commit 4f173db8b4 "OvmfPkg/PlatformInitLib: Add functions for > > > > EmuVariableNvStore", it introduced a PlatformValidateNvVarStore() > > function > > > > for checking the integrity of NvVarStore. > > > > > > > > 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 = (VAR_IN_DELETED_TRANSITION & VAR_ADDED) & > > VAR_DELETED > > > > or > > > > 0x3D = VAR_ADDED & VAR_DELETED > > > > > > > > When the variable store has those variables, system booting/rebooting > > will > > > > hangs in a ASSERT: > > > > > > > > NvVarStore Variable header State was invalid. > > > > ASSERT > > > > /mnt/working/source_code- > > > > git/edk2/OvmfPkg/Library/PlatformInitLib/Platform.c(819): > > > > ((BOOLEAN)(0==1)) > > > > > > > > Adding more log to UpdateVariable() and PlatformValidateNvVarStore(), > > we > > > > saw some variables which have 0x3C or 0x3D state in store. > > > > e.g. > > > > > > > > UpdateVariable(), VariableName=BootOrder > > > > L1871, State=0000003F <-- VAR_ADDED > > > > State &= VAR_DELETED=0000003D > > > > FlushHobVariableToFlash(), VariableName=BootOrder > > > > ... > > > > UpdateVariable(), VariableName=InitialAttemptOrder > > > > L1977, State=0000003F > > > > State &= VAR_IN_DELETED_TRANSITION=0000003E > > > > L2376, State=0000003E > > > > State &= VAR_DELETED=0000003C > > > > FlushHobVariableToFlash(), VariableName=InitialAttemptOrder > > > > ... > > > > UpdateVariable(), VariableName=ConIn > > > > L1977, State=0000003F > > > > State &= VAR_IN_DELETED_TRANSITION=0000003E > > > > L2376, State=0000003E > > > > State &= VAR_DELETED=0000003C > > > > FlushHobVariableToFlash(), VariableName=ConIn > > > > ... > > > > > > > > So, only allowing the four primary states is not enough. This patch > > changes > > > > the falid states list (Follow Jiewen Yao's suggestion): > > > > > > > > 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. > > > > > > > > And removed (VAR_IN_DELETED_TRANSITION) and (VAR_DELETED) > > because > > > > they are > > > > invalid states. > > > > > > > > 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 > > > > > > > > Signed-off-by: "Lee, Chun-Yi" > > > > --- > > > > OvmfPkg/Library/PlatformInitLib/Platform.c | 9 +++++---- > > > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > > > > > 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 ( > > > > > > > > VariableOffset = NvVarStoreHeader->Size - sizeof > > > > (VARIABLE_STORE_HEADER); > > > > } else { > > > > - if (!((VariableHeader->State == VAR_IN_DELETED_TRANSITION) || > > > > - (VariableHeader->State == VAR_DELETED) || > > > > - (VariableHeader->State == VAR_HEADER_VALID_ONLY) || > > > > - (VariableHeader->State == VAR_ADDED))) > > > > + if (!((VariableHeader->State == VAR_HEADER_VALID_ONLY) || > > > > + (VariableHeader->State == VAR_ADDED) || > > > > + (VariableHeader->State == (VAR_ADDED & VAR_DELETED)) || > > > > + (VariableHeader->State == (VAR_ADDED & > > > > VAR_IN_DELETED_TRANSITION)) || > > > > + (VariableHeader->State == (VAR_ADDED & > > > > VAR_IN_DELETED_TRANSITION & VAR_DELETED)))) > > > > { > > > > DEBUG ((DEBUG_ERROR, "NvVarStore Variable header State was > > > > invalid.\n")); > > > > return FALSE; > > > > -- > > > > 2.35.3 > > > > > > > > > > > > > > >