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.66]) by mx.groups.io with SMTP id smtpd.web10.44959.1671520208446989030 for ; Mon, 19 Dec 2022 23:10:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@suse.com header.s=selector1 header.b=ymqaGpPP; spf=pass (domain: suse.com, ip: 40.107.241.66, mailfrom: jlee@suse.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=P1/Vwlv5p9jTRuHAwNRb4rK4ZZF3TO+VE8z7uHp3Tb+ciRlrsuhjA/7DNODueHzUq9HxpzqRb3h5XpYv63kcfK766SkPNVZZHDC7yhjdvHWg/xsn2xSX83I8phQv+oc5eKhyXl24MxatfFzDlQ4VK7sXAtPpGro3lN3D/b/Ao+PNFLIn8K4wbu2psNHcOPTcurjNestcsUWbybyQjEmjdAHh+da3FPOEtqkIp887SbUHnRvh81MUB0UrNOCN9RAzSW3U4KopGBQhpnieDJilPxNUCCgW8aunLBQGBJ14pJTEjsPAfU6yURqrtxTXArfFox+RZgHmMjmhpBWs6cwNRw== 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=T77pKR8QsrWmAF7bafiEkv7tDTHmWiBF1K8L1zRGLeY=; b=m6/EFYSxpnWc0IU6AIdCUmkHXEnxlgSHKXpZh9U4mWeq+bL01FJ4QanTZ3IDeEw0tp+sbfVZP2cR8y9bIq5+oTog6542P2v4zB6I8i2ciedJQCFHHzL0oX3EqPYbtujGXh036hIEtV+HarftAqG2QvwbBjEMjF3XDxXQzE0ImJaMTc0tX5SNiMeu3VAWl1UjQe6V1d9z5i9qlm0Ws3cpSVDqcSRYh6VF42DjH3IcXvJttgegcMlzn1YuNLfseCgchiBeSxr2l2BqrZB5QIz1zE4kJ4Cg4yrKRlMdEqt6tT1BYm5zkhfBtgLYUVFuco/Hy/Xvz+1I+lBcAQuQm0FVeA== 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=T77pKR8QsrWmAF7bafiEkv7tDTHmWiBF1K8L1zRGLeY=; b=ymqaGpPPXnuHW/ef47doyQ5Ws2FfasEiQ6D/WrZuFVD74tGR298Pqtb1xt3VCVhEiHLILFoHMPMAKdc7F+H35yJrnRESXQdWz8wHWAI44cIeXYPgytTXWPk8b+xBWfYsQMvZqbGDgg2PgPTv79sYovSv5S4fRiSmzXHfmL2qH4lHP3kyZKIHfPtE28eoH3y9yGMVRSqjI5Gsoq5c70QuuJYOr+LPKtCAHP/LyjhglrlcHmfhl4SvM6i+IlmXhu/52l2IzKUjjU7l1w6i9pnHwD53SCgnL5TfHp+74KrqOrwy8ju2ivc0vJTQIyvhbsMsqqS+nM6mG3sr4nj5Iu18VQ== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com; Received: from DB8PR04MB7164.eurprd04.prod.outlook.com (2603:10a6:10:129::23) by DB9PR04MB8394.eurprd04.prod.outlook.com (2603:10a6:10:244::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5924.16; Tue, 20 Dec 2022 07:10:06 +0000 Received: from DB8PR04MB7164.eurprd04.prod.outlook.com ([fe80::7af1:18f8:9805:193d]) by DB8PR04MB7164.eurprd04.prod.outlook.com ([fe80::7af1:18f8:9805:193d%8]) with mapi id 15.20.5924.016; Tue, 20 Dec 2022 07:10:05 +0000 Date: Tue, 20 Dec 2022 15:09:58 +0800 From: "joeyli" To: devel@edk2.groups.io, jiewen.yao@intel.com 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 Message-ID: <20221220070958.GJ11807@linux-l9pv.suse> References: <20221215142723.9788-1-jlee@suse.com> In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) X-ClientProxiedBy: FR0P281CA0109.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a8::9) To DB8PR04MB7164.eurprd04.prod.outlook.com (2603:10a6:10:129::23) Return-Path: JLee@suse.com MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DB8PR04MB7164:EE_|DB9PR04MB8394:EE_ X-MS-Office365-Filtering-Correlation-Id: 49705a83-8c23-4eaa-b285-08dae2593896 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: VwyfV5oRY2olmlpxDmO5tVUOpmTvrNaOv5eKwkeaKOJoGj/jwli6iczGmg4EAqoBpYq1pn0nyZiGg2Rhj0ou137z3CP9E3YhBzRkyyrLjtEhqi1JmyRvZ+RrHggHgOQjvjoyeyrPrwTvfwEfIJ+s7CIyoIfivkJg0dFN8MEaNfHkExV94hqgfyGS6+vCoCuOBmKJag25PSa4ZQXwF6OfLtIL9fszZSdqyBCUDSc3Rw0dAr8HNjwnESnDImNE0q4T42RAU4oCkXs3nW3nx2lth2Z8rN6E5lv7nPwvIQ43QMdpBvkqy6Qj/hEgXLQtQlVtd69Stsr1I2vmwcltaXHf7wWZxQzOo6l8KM1GBLnk5A6cea8yQBeqZnrMF98yacA1WukQO0L9k9O58TEbmW6jOuZb2EjEGeJDmeFPmwap6kmxlP/C8561lnNLmwa1uGrQNevwOMm465kCncq5oBchbKaCboFd0iVL0+u5W4hDbynqBWENPm1kWabRtsFgfJ86dlyOQiDJ0PphJLPDAfregpDbxZUKMc4Wzz+chgDcyU97f28R47nh9kIbVVdY1KyCe7cj9p1KpmOdHCHww53CD6zGYfzoxhg0F0R7gXLw8lgbXuBgaamVsvmdho0YS8WmpqNgDYkpBqFGTPXzt2Mt0Y+eK+YabxKNjICd0E8nPWAwdID9I3Qcd8KzYCN+SGW8G0BBVGcmDzHTKRcjmw7xkA== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB8PR04MB7164.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230022)(6029001)(376002)(136003)(346002)(39860400002)(396003)(366004)(451199015)(38100700002)(41300700001)(8936002)(4326008)(66556008)(5660300002)(66946007)(8676002)(66476007)(83380400001)(1076003)(478600001)(53546011)(9686003)(186003)(26005)(86362001)(6512007)(66899015)(6486002)(2906002)(33656002)(6506007)(54906003)(6666004)(36756003)(316002)(966005)(43062005);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?d6PKh/px1FRnQMtwPvpDP6hkwjWQgLnk0J2zx1hIKRlZczyp58ixmvDsY3ww?= =?us-ascii?Q?NuRNva3KV6l+NBNELK53ARas5qs7wN3/btM0qYOfrWmanHuwI0t3GS9ZB/YD?= =?us-ascii?Q?LZimz/PJr6R9nZBt1KsdEMBXcIu9qWPYkDcRm4ZjN57v4qiXk3Maj2H6csm1?= =?us-ascii?Q?Kd9sWam5jh5k4QtRSRhZVwOupfvMU4q6SI+mb0XypVAAxDhfobISzRc/4eg4?= =?us-ascii?Q?w4vphw9UXQvJ/qOPMqdnYhYxICFmu+g6K22gbBowLRd4GcfI5xvuVFJzy3yj?= =?us-ascii?Q?v6lI4j/bfd/ojjepyxtVngOEEOK28YMdCc6HOkwW3SFsoWx9v8f0heTuP0dC?= =?us-ascii?Q?z0urohwVTqBfbocRTNj1JE5OpCjwH1OVxXP5wwgKcDX4FqiDwtlYjijx3TPA?= =?us-ascii?Q?YV2jAvbF+OFv8rAGkN8hDYUV76RBuSk0Q7mS6/3NADCpO90Nw1pIxK8M+EN/?= =?us-ascii?Q?R6KLWQSBXrOEuYjH41LF7ExhRFX13yTzpjONK7xHT7mt5VnHcB4omNbvcG8O?= =?us-ascii?Q?FeAcOe1Z3Nkg0erWDF//lyhsqb3zpThAsiSMZE7ClPt69bwnIe97jEi0Irs5?= =?us-ascii?Q?5jHsQDpisz7ue4yFSzeTYTAJDtDY5M+A8RvlLSOT5hLR1dngSsU6DYt6EBYO?= =?us-ascii?Q?7ySt6u/DGRwcM2mXU1BKxyJlUW2BMzzwvR8Pl+Nqxr9GFqAvwR3OlQJMKGKx?= =?us-ascii?Q?cdQ5OjLwwYqavvqPrRW+DNUD457At6p2R3WV7NJu8BApP7KK3Zj+VdMr/9kA?= =?us-ascii?Q?l9URJplHqQZeTgYb3IcQ+Ei3f2NNfrPKho7BnNQIufP2oYr76aTRI9CTlF1B?= =?us-ascii?Q?5xhvNCvspjudVXeFh3CGTQlp9Yulfe6kvSmGeAMb0g00yuqr5c4wUiHI8F+R?= =?us-ascii?Q?3Dxn5imK3hWj+N65FuXfqoafqju5dYh8IYu/dgTEM041E8nVc68FTi7uzhUO?= =?us-ascii?Q?aGzQ+Wgjd6Cq5Fu7wxKjr2rqoxVigbif2Jj1htqJM4DTwn0+sYO78Mj49BrD?= =?us-ascii?Q?NpCXcheJmMNnUf9wYv52ib1bcClRgDHOFN1jAhXnYv/RMVNgtQiZpvBuU71A?= =?us-ascii?Q?sJuSOEBH+ifStORlJzNxBV9yBqjI5VKFb5FoIKGj/mPxzYc/nz26R9obJCto?= =?us-ascii?Q?Jiqi7ps7J83keElGSxXHV20dapvmTzaTwiiDrF6N0/QFpPj7SbT6u2uo/dyr?= =?us-ascii?Q?3m0LdM6qATtnFBXj/WNRJUPXmrWEaspJC30bwgZFEhNzIjAW4KH2DEDkvzwi?= =?us-ascii?Q?fYXE5ghCR8A5SFfQ4x4csRrARVk15HlFHuN9jAiSUQMNcBg6U+7PRMzoQiX5?= =?us-ascii?Q?dqA+APsQsUYI9dvLDhtrJaDdEDzV4+OEH+oMmv/HrQ7f+BYJta5M/4Z6kNhS?= =?us-ascii?Q?RKiLNWCv9PwXqgYTt0FFq9dh+nyUwRGuv7sCvfZg2qEepiOdf113Qo5D6gD+?= =?us-ascii?Q?A1LCV21TCDASd4JCK3G6WW7v8EniZZvuU8Ur0hgjtoIQadwPCV7uGoEr2Aby?= =?us-ascii?Q?rvimcqe+ewn9isjK+6mPKYsvjNyDklCpXCPIqZ0WZWRxN9xzs6uTuONuVFmu?= =?us-ascii?Q?wWafSw3jDeBjh1ty2sI=3D?= X-OriginatorOrg: suse.com X-MS-Exchange-CrossTenant-Network-Message-Id: 49705a83-8c23-4eaa-b285-08dae2593896 X-MS-Exchange-CrossTenant-AuthSource: DB8PR04MB7164.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2022 07:10:05.7744 (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: /11TfZ/EOIH+UQ9xZvuwKS4vbNbaeQBlealdIzxHYDDaUyRc/wcVQG2S4ADavgty X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8394 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > > > > >