From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) by mx.groups.io with SMTP id smtpd.web11.135243.1671114459492955286 for ; Thu, 15 Dec 2022 06:27:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=dMK5oCXZ; spf=pass (domain: gmail.com, ip: 209.85.216.48, mailfrom: joeyli.kernel@gmail.com) Received: by mail-pj1-f48.google.com with SMTP id e7-20020a17090a77c700b00216928a3917so2887139pjs.4 for ; Thu, 15 Dec 2022 06:27:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=CiU7KIQe20Aw2OFAIPmcZMxDQwRLdVWv0fn5lWYHbs4=; b=dMK5oCXZ3DFGor3Ul3zZwVNLdnmlOzz6Xz4YWxH+sNOlvgZJ0cYzvvFCzuMaZ9XaIE t0UIsQZ5Hql7FJK7TX4xnQbI5qV7D3xOf/4HNKWYT1RY2DikGi2a9tZdKmZIfb+Fna9O PcmAXKD4SFqFJxxSt6NETeI/4/ApaeOQKzxwRlLJWzN4z6CSVdlpAiL5y2HdbA4vTWyx nMyL73NhJpZah3e1BRC/73tUG0hB1q2gSH4JeeBWYHZlWqXqKNy+yhbWQ8ORt4BGfhFE 21fq0nx2+rpB/4fKtw8GNOjmVcRnRENOvnvDeGOhhYQTwQYEa8fhmPrfTjApyxza1mMj 05BQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=CiU7KIQe20Aw2OFAIPmcZMxDQwRLdVWv0fn5lWYHbs4=; b=svNh+IZxcN+6Qh8jwW2ugi7oEaMwqGtxFxX6FbfSaxqWb6dKZg+NA3IprUTEGYrWZE dExEK+y/7b++VdhcLN8tZU7fRYUaiKjK1nackl+Rcr+yhvqV1cMXYbGZkSlY9HI1znL+ DAhDL7RHVIyZhF0fnFjIc6W3bhJeAQzdr0Oz+QL4qj3M/rZv3nCJt60Gfgk7/UsBDBp/ lzkJy/Y+MT7vhx5Sxh0y6ghnvNVp8y5YQburzYJqI/igoudQcKx8YjBfCkDmrwwRuxOO w8TvZ29gjm3936t1Em9Ly4h8Ao7oXDmX0OrqgAqrw458TMjFQLRGk4Yv8lhuEQtuDTOq ARng== X-Gm-Message-State: ANoB5plQcw28OvW3yc+dt1TJK5CNV7iYGI/K2tzSaBLjBI4i/6tGlW6o YDBdJ5BlwpfrDQxhHaj9lGAAL3LMZdw= X-Google-Smtp-Source: AA0mqf78mtxbdmiVfjDyOucRkU8/u5UWumwnV9zPZ/1LWMdYt7nuQzWgghjVp+hr8gr3WKUSj0cUGA== X-Received: by 2002:a17:902:a9c2:b0:189:8b5:2fe3 with SMTP id b2-20020a170902a9c200b0018908b52fe3mr25283865plr.54.1671114458692; Thu, 15 Dec 2022 06:27:38 -0800 (PST) Return-Path: Received: from linux-l9pv.suse ([124.11.22.254]) by smtp.gmail.com with ESMTPSA id f6-20020a170902684600b001887e30b9ddsm3852736pln.257.2022.12.15.06.27.36 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Dec 2022 06:27:38 -0800 (PST) From: "Lee, Chun-Yi" X-Google-Original-From: "Lee, Chun-Yi" To: devel@edk2.groups.io Cc: Min M Xu , Gerd Hoffmann , Jiewen Yao , Tom Lendacky , James Bottomley , Erdem Aktas , "Lee, Chun-Yi" Subject: [PATCH v2] OvmfPkg/PlatformInitLib: Fix integrity checking failed of NvVarStore in some cases Date: Thu, 15 Dec 2022 22:27:23 +0800 Message-Id: <20221215142723.9788-1-jlee@suse.com> X-Mailer: git-send-email 2.12.3 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