[stable/202011][Patch 0/2] MdeModulePkg/Variable/RuntimeDxe:Restore Variable Lock Protocol behavior

[stable/202011][Patch 0/2] MdeModulePkg/Variable/RuntimeDxe:Restore Variable Lock Protocol behavior

[Michael D Kinney]

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

Cherry-pick the following commits from edk2/master to stable/202011.

(cherry picked from commit dcaa93936591883aa7826eb45ef00416ad82ef08) 
(cherry picked from commit a18a9bde36d2ffc12df29cdced1efa1f8f9f2021)

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>

Bret Barkelew (1):
  MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol
    behavior

Michael D Kinney (1):
  MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit
    Tests

 MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
 .../VariableLockRequestToLockUnitTest.c       | 565 ++++++++++++++++++
 .../VariableLockRequestToLockUnitTest.inf     |  36 ++
 .../RuntimeDxe/VariableLockRequestToLock.c    |  95 +--
 4 files changed, 671 insertions(+), 36 deletions(-)
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf

-- 
2.29.2.windows.2

[stable/202011][Patch 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior

[Michael D Kinney]

From: Bret Barkelew <bret.barkelew@microsoft.com>

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

The VariableLock shim currently fails if called twice because the
underlying Variable Policy engine returns an error if a policy is set
on an existing variable.

This breaks existing code which expect it to silently pass if a variable
is locked multiple times (because it should "be locked").

Refactor the shim to confirm that the variable is indeed locked and then
change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so
the duplicate lock can be reported in a debug log and removed.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
(cherry picked from commit a18a9bde36d2ffc12df29cdced1efa1f8f9f2021)
---
 .../RuntimeDxe/VariableLockRequestToLock.c    | 95 ++++++++++++-------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
index 4aa854aaf260..7d87e50efdcd 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
@@ -1,67 +1,90 @@
-/** @file -- VariableLockRequestToLock.c
-Temporary location of the RequestToLock shim code while
-projects are moved to VariablePolicy. Should be removed when deprecated.
+/** @file
+  Temporary location of the RequestToLock shim code while projects
+  are moved to VariablePolicy. Should be removed when deprecated.
 
-Copyright (c) Microsoft Corporation.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include <Uefi.h>
-
 #include <Library/DebugLib.h>
 #include <Library/MemoryAllocationLib.h>
-
-#include <Protocol/VariableLock.h>
-
-#include <Protocol/VariablePolicy.h>
 #include <Library/VariablePolicyLib.h>
 #include <Library/VariablePolicyHelperLib.h>
-
+#include <Protocol/VariableLock.h>
 
 /**
   DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING.
-  Mark a variable that will become read-only after leaving the DXE phase of execution.
-  Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTOCOL is allowed.
+  Mark a variable that will become read-only after leaving the DXE phase of
+  execution. Write request coming from SMM environment through
+  EFI_SMM_VARIABLE_PROTOCOL is allowed.
 
   @param[in] This          The VARIABLE_LOCK_PROTOCOL instance.
-  @param[in] VariableName  A pointer to the variable name that will be made read-only subsequently.
-  @param[in] VendorGuid    A pointer to the vendor GUID that will be made read-only subsequently.
+  @param[in] VariableName  A pointer to the variable name that will be made
+                           read-only subsequently.
+  @param[in] VendorGuid    A pointer to the vendor GUID that will be made
+                           read-only subsequently.
 
-  @retval EFI_SUCCESS           The variable specified by the VariableName and the VendorGuid was marked
-                                as pending to be read-only.
+  @retval EFI_SUCCESS           The variable specified by the VariableName and
+                                the VendorGuid was marked as pending to be
+                                read-only.
   @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL.
                                 Or VariableName is an empty string.
-  @retval EFI_ACCESS_DENIED     EFI_END_OF_DXE_EVENT_GROUP_GUID or EFI_EVENT_GROUP_READY_TO_BOOT has
-                                already been signaled.
-  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock request.
+  @retval EFI_ACCESS_DENIED     EFI_END_OF_DXE_EVENT_GROUP_GUID or
+                                EFI_EVENT_GROUP_READY_TO_BOOT has already been
+                                signaled.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock
+                                request.
 **/
 EFI_STATUS
 EFIAPI
 VariableLockRequestToLock (
-  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This,
-  IN       CHAR16                       *VariableName,
-  IN       EFI_GUID                     *VendorGuid
+  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
+  IN CHAR16                              *VariableName,
+  IN EFI_GUID                            *VendorGuid
   )
 {
-  EFI_STATUS              Status;
-  VARIABLE_POLICY_ENTRY   *NewPolicy;
+  EFI_STATUS             Status;
+  VARIABLE_POLICY_ENTRY  *NewPolicy;
+
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! %a() will go away soon!\n", __FUNCTION__));
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Please move to use Variable Policy!\n"));
+  DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Variable: %g %s\n", VendorGuid, VariableName));
 
   NewPolicy = NULL;
-  Status = CreateBasicVariablePolicy( VendorGuid,
-                                      VariableName,
-                                      VARIABLE_POLICY_NO_MIN_SIZE,
-                                      VARIABLE_POLICY_NO_MAX_SIZE,
-                                      VARIABLE_POLICY_NO_MUST_ATTR,
-                                      VARIABLE_POLICY_NO_CANT_ATTR,
-                                      VARIABLE_POLICY_TYPE_LOCK_NOW,
-                                      &NewPolicy );
+  Status = CreateBasicVariablePolicy(
+             VendorGuid,
+             VariableName,
+             VARIABLE_POLICY_NO_MIN_SIZE,
+             VARIABLE_POLICY_NO_MAX_SIZE,
+             VARIABLE_POLICY_NO_MUST_ATTR,
+             VARIABLE_POLICY_NO_CANT_ATTR,
+             VARIABLE_POLICY_TYPE_LOCK_NOW,
+             &NewPolicy
+             );
   if (!EFI_ERROR( Status )) {
-    Status = RegisterVariablePolicy( NewPolicy );
+    Status = RegisterVariablePolicy (NewPolicy);
+
+    //
+    // If the error returned is EFI_ALREADY_STARTED, we need to check the
+    // current database for the variable and see whether it's locked. If it's
+    // locked, we're still fine, but also generate a DEBUG_ERROR message so the
+    // duplicate lock can be removed.
+    //
+    if (Status == EFI_ALREADY_STARTED) {
+      Status = ValidateSetVariable (VariableName, VendorGuid, 0, 0, NULL);
+      if (Status == EFI_WRITE_PROTECTED) {
+        DEBUG ((DEBUG_ERROR, "  Variable: %g %s is already locked!\n", VendorGuid, VariableName));
+        Status = EFI_SUCCESS;
+      } else {
+        DEBUG ((DEBUG_ERROR, "  Variable: %g %s can not be locked!\n", VendorGuid, VariableName));
+        Status = EFI_ACCESS_DENIED;
+      }
+    }
   }
-  if (EFI_ERROR( Status )) {
+  if (EFI_ERROR (Status)) {
     DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, VariableName, Status ));
-    ASSERT_EFI_ERROR( Status );
   }
   if (NewPolicy != NULL) {
     FreePool( NewPolicy );
-- 
2.29.2.windows.2

[stable/202011][Patch 2/2] MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit Tests

[Michael D Kinney]

https://bugzilla.tianocore.org/show_bug.cgi?id=3111

Add host based unit tests for the multiple lock case using Variable Lock
Protocol, Variable Policy Protocol, and mixes of Variable Lock Protocol
and Variable Policy Protocol.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
(cherry picked from commit dcaa93936591883aa7826eb45ef00416ad82ef08)
---
 MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
 .../VariableLockRequestToLockUnitTest.c       | 565 ++++++++++++++++++
 .../VariableLockRequestToLockUnitTest.inf     |  36 ++
 3 files changed, 612 insertions(+)
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
 create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf

diff --git a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
index 72a119db4568..4da4692c8451 100644
--- a/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
+++ b/MdeModulePkg/Test/MdeModulePkgHostTest.dsc
@@ -19,6 +19,9 @@ [Defines]
 
 !include UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc
 
+[LibraryClasses]
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+
 [Components]
   MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
 
@@ -30,3 +33,11 @@ [Components]
       ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
       UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
   }
+
+  MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf {
+    <LibraryClasses>
+      VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
+      VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
+    <PcdsFixedAtBuild>
+      gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDisable|TRUE
+  }
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
new file mode 100644
index 000000000000..44d70e639d77
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
@@ -0,0 +1,565 @@
+/** @file
+  This is a host-based unit test for the VariableLockRequestToLock shim.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <stdio.h>
+#include <string.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <Uefi.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UnitTestLib.h>
+#include <Library/VariablePolicyLib.h>
+#include <Library/VariablePolicyHelperLib.h>
+
+#include <Protocol/VariableLock.h>
+
+#define UNIT_TEST_NAME        "VarPol/VarLock Shim Unit Test"
+#define UNIT_TEST_VERSION     "1.0"
+
+///=== CODE UNDER TEST ===========================================================================
+
+EFI_STATUS
+EFIAPI
+VariableLockRequestToLock (
+  IN CONST EDKII_VARIABLE_LOCK_PROTOCOL  *This,
+  IN       CHAR16                        *VariableName,
+  IN       EFI_GUID                      *VendorGuid
+  );
+
+///=== TEST DATA ==================================================================================
+
+//
+// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592}
+//
+EFI_GUID  mTestGuid1 = {
+  0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x5, 0x92}
+};
+
+//
+// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A}
+//
+EFI_GUID  mTestGuid2 = {
+  0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xaf, 0x3a}
+};
+
+//
+// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9}
+//
+EFI_GUID  mTestGuid3 = {
+  0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x82, 0xa9}
+};
+
+#define TEST_VAR_1_NAME              L"TestVar1"
+#define TEST_VAR_2_NAME              L"TestVar2"
+#define TEST_VAR_3_NAME              L"TestVar3"
+
+#define TEST_POLICY_ATTRIBUTES_NULL  0
+#define TEST_POLICY_MIN_SIZE_NULL    0
+#define TEST_POLICY_MAX_SIZE_NULL    MAX_UINT32
+
+#define TEST_POLICY_MIN_SIZE_10      10
+#define TEST_POLICY_MAX_SIZE_200     200
+
+///=== HELPER FUNCTIONS ===========================================================================
+
+/**
+  Mocked version of GetVariable, for testing.
+
+  @param  VariableName
+  @param  VendorGuid
+  @param  Attributes
+  @param  DataSize
+  @param  Data
+**/
+EFI_STATUS
+EFIAPI
+StubGetVariableNull (
+  IN     CHAR16    *VariableName,
+  IN     EFI_GUID  *VendorGuid,
+  OUT    UINT32    *Attributes,  OPTIONAL
+  IN OUT UINTN     *DataSize,
+  OUT    VOID      *Data         OPTIONAL
+  )
+{
+  UINT32      MockedAttr;
+  UINTN       MockedDataSize;
+  VOID        *MockedData;
+  EFI_STATUS  MockedReturn;
+
+  check_expected_ptr (VariableName);
+  check_expected_ptr (VendorGuid);
+  check_expected_ptr (DataSize);
+
+  MockedAttr     = (UINT32)mock();
+  MockedDataSize = (UINTN)mock();
+  MockedData     = (VOID*)(UINTN)mock();
+  MockedReturn   = (EFI_STATUS)mock();
+
+  if (Attributes != NULL) {
+    *Attributes = MockedAttr;
+  }
+  if (Data != NULL && !EFI_ERROR (MockedReturn)) {
+    CopyMem (Data, MockedData, MockedDataSize);
+  }
+
+  *DataSize = MockedDataSize;
+
+  return MockedReturn;
+}
+
+//
+// Anything you think might be helpful that isn't a test itself.
+//
+
+/**
+  This is a common setup function that will ensure the library is always
+  initialized with the stubbed GetVariable.
+
+  Not used by all test cases, but by most.
+
+  @param[in]  Context  Unit test case context
+**/
+STATIC
+UNIT_TEST_STATUS
+EFIAPI
+LibInitMocked (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  return EFI_ERROR (InitVariablePolicyLib (StubGetVariableNull)) ? UNIT_TEST_ERROR_PREREQUISITE_NOT_MET : UNIT_TEST_PASSED;
+}
+
+/**
+  Common cleanup function to make sure that the library is always de-initialized
+  prior to the next test case.
+
+  @param[in]  Context  Unit test case context
+**/
+STATIC
+VOID
+EFIAPI
+LibCleanup (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  DeinitVariablePolicyLib();
+}
+
+///=== TEST CASES =================================================================================
+
+///===== SHIM SUITE ===========================================================
+
+/**
+  Test Case that locks a single variable using the Variable Lock Protocol.
+  The call is expected to succeed.
+
+  @param[in]  Context  Unit test case context
+**/
+UNIT_TEST_STATUS
+EFIAPI
+LockingWithoutAnyPoliciesShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks the same variable twice using the Variable Lock Protocol.
+  Both calls are expected to succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingTwiceShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol then locks
+  the same variable using the Variable Lock Protocol.
+  Both calls are expected to succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableShouldSucceed (
+  IN UNIT_TEST_CONTEXT  Context
+  )
+{
+  EFI_STATUS             Status;
+  VARIABLE_POLICY_ENTRY  *NewEntry;
+
+  //
+  // Create a variable policy that locks the variable.
+  //
+  Status = CreateBasicVariablePolicy (
+             &mTestGuid1,
+             TEST_VAR_1_NAME,
+             TEST_POLICY_MIN_SIZE_NULL,
+             TEST_POLICY_MAX_SIZE_200,
+             TEST_POLICY_ATTRIBUTES_NULL,
+             TEST_POLICY_ATTRIBUTES_NULL,
+             VARIABLE_POLICY_TYPE_LOCK_NOW,
+             &NewEntry
+             );
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  //
+  // Register the new policy.
+  //
+  Status = RegisterVariablePolicy (NewEntry);
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW then attempts to lock the same variable using the
+  Variable Lock Protocol.  The call to Variable Policy is expected to succeed
+  and the call to Variable Lock is expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingAnUnlockedVariableShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, 0 );                              // Size
+  will_return( StubGetVariableNull, NULL );                           // DataPtr
+  will_return( StubGetVariableNull, EFI_NOT_FOUND);                   // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW, but is currently locked.  Then attempts to lock
+  the same variable using the Variable Lock Protocol.  The call to Variable
+  Policy is expected to succeed and the call to Variable Lock also expected to
+  succeed.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableWithMatchingDataShouldSucceed (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+  UINT8                     Data;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  Data = 1;
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, sizeof (Data) );                  // Size
+  will_return( StubGetVariableNull, &Data );                          // DataPtr
+  will_return( StubGetVariableNull, EFI_SUCCESS);                     // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (!EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using the Variable Policy Protocol with a
+  policy other than LOCK_NOW, but variable data does not match.  Then attempts
+  to lock the same variable using the Variable Lock Protocol.  The call to
+  Variable Policy is expected to succeed and the call to Variable Lock is
+  expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+LockingALockedVariableWithNonMatchingDataShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+  UINT8                     Data;
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+
+ // Configure the stub to not care about parameters. We're testing errors.
+  expect_any_always( StubGetVariableNull, VariableName );
+  expect_any_always( StubGetVariableNull, VendorGuid );
+  expect_any_always( StubGetVariableNull, DataSize );
+
+  // With a policy, make sure that writes still work, since the variable doesn't exist.
+  Data = 2;
+  will_return( StubGetVariableNull, TEST_POLICY_ATTRIBUTES_NULL );    // Attributes
+  will_return( StubGetVariableNull, sizeof (Data) );                  // Size
+  will_return( StubGetVariableNull, &Data );                          // DataPtr
+  will_return( StubGetVariableNull, EFI_SUCCESS);                     // Status
+
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Test Case that locks a variable using Variable Lock Protocol Policy Protocol
+  then and then attempts to lock the same variable using the Variable Policy
+  Protocol.  The call to Variable Lock is expected to succeed and the call to
+  Variable Policy is expected to fail.
+
+  @param[in]  Context  Unit test case context
+  **/
+UNIT_TEST_STATUS
+EFIAPI
+SettingPolicyForALockedVariableShouldFail (
+  IN UNIT_TEST_CONTEXT      Context
+  )
+{
+  EFI_STATUS                Status;
+  VARIABLE_POLICY_ENTRY     *NewEntry;
+
+  // Lock the variable.
+  Status = VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, &mTestGuid1);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Create a variable policy that locks the variable.
+  Status = CreateVarStateVariablePolicy (&mTestGuid1,
+                                         TEST_VAR_1_NAME,
+                                         TEST_POLICY_MIN_SIZE_NULL,
+                                         TEST_POLICY_MAX_SIZE_200,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         TEST_POLICY_ATTRIBUTES_NULL,
+                                         &mTestGuid2,
+                                         1,
+                                         TEST_VAR_2_NAME,
+                                         &NewEntry);
+  UT_ASSERT_NOT_EFI_ERROR (Status);
+
+  // Register the new policy.
+  Status = RegisterVariablePolicy (NewEntry);
+  UT_ASSERT_TRUE (EFI_ERROR (Status));
+
+  FreePool (NewEntry);
+
+  return UNIT_TEST_PASSED;
+}
+
+/**
+  Main entry point to this unit test application.
+
+  Sets up and runs the test suites.
+**/
+VOID
+EFIAPI
+UnitTestMain (
+  VOID
+  )
+{
+  EFI_STATUS                  Status;
+  UNIT_TEST_FRAMEWORK_HANDLE  Framework;
+  UNIT_TEST_SUITE_HANDLE      ShimTests;
+
+  Framework = NULL;
+
+  DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME, UNIT_TEST_VERSION));
+
+  //
+  // Start setting up the test framework for running the tests.
+  //
+  Status = InitUnitTestFramework (&Framework, UNIT_TEST_NAME, gEfiCallerBaseName, UNIT_TEST_VERSION);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status = %r\n", Status));
+    goto EXIT;
+  }
+
+  //
+  // Add all test suites and tests.
+  //
+  Status = CreateUnitTestSuite (
+             &ShimTests, Framework,
+             "Variable Lock Shim Tests", "VarPolicy.VarLockShim", NULL, NULL
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for ShimTests\n"));
+    Status = EFI_OUT_OF_RESOURCES;
+    goto EXIT;
+  }
+  AddTestCase (
+    ShimTests,
+    "Locking a variable with no matching policies should always work", "EmptyPolicies",
+    LockingWithoutAnyPoliciesShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable twice should always work", "DoubleLock",
+    LockingTwiceShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that's already locked by another policy should work", "LockAfterPolicy",
+    LockingALockedVariableShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an unlocked policy should fail", "LockAfterUnlockedPolicy",
+    LockingAnUnlockedVariableShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an locked policy should succeed", "LockAfterLockedPolicyMatchingData",
+    LockingALockedVariableWithMatchingDataShouldSucceed, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Locking a variable that already has an locked policy with matching data should succeed", "LockAfterLockedPolicyNonMatchingData",
+    LockingALockedVariableWithNonMatchingDataShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+  AddTestCase (
+    ShimTests,
+    "Adding a policy for a variable that has previously been locked should always fail", "SetPolicyAfterLock",
+    SettingPolicyForALockedVariableShouldFail, LibInitMocked, LibCleanup, NULL
+    );
+
+  //
+  // Execute the tests.
+  //
+  Status = RunAllTestSuites (Framework);
+
+EXIT:
+  if (Framework != NULL) {
+    FreeUnitTestFramework (Framework);
+  }
+
+  return;
+}
+
+///
+/// Avoid ECC error for function name that starts with lower case letter
+///
+#define Main main
+
+/**
+  Standard POSIX C entry point for host based unit test execution.
+
+  @param[in] Argc  Number of arguments
+  @param[in] Argv  Array of pointers to arguments
+
+  @retval 0      Success
+  @retval other  Error
+**/
+INT32
+Main (
+  IN INT32  Argc,
+  IN CHAR8  *Argv[]
+  )
+{
+  UnitTestMain ();
+  return 0;
+}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf
new file mode 100644
index 000000000000..2a659d7e1370
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.inf
@@ -0,0 +1,36 @@
+## @file
+# This is a host-based unit test for the VariableLockRequestToLock shim.
+#
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION         = 0x00010017
+  BASE_NAME           = VariableLockRequestToLockUnitTest
+  FILE_GUID           = A7388B6C-7274-4717-9649-BDC5DFD1FCBE
+  VERSION_STRING      = 1.0
+  MODULE_TYPE         = HOST_APPLICATION
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
+#
+
+[Sources]
+  VariableLockRequestToLockUnitTest.c
+  ../VariableLockRequestToLock.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
+
+[LibraryClasses]
+  UnitTestLib
+  DebugLib
+  VariablePolicyLib
+  VariablePolicyHelperLib
+  BaseMemoryLib
+  MemoryAllocationLib
-- 
2.29.2.windows.2

回复: [edk2-devel] [stable/202011][Patch 0/2] MdeModulePkg/Variable/RuntimeDxe:Restore Variable Lock Protocol behavior

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: bounce+27952+69674+4905953+8761045@groups.io
> <bounce+27952+69674+4905953+8761045@groups.io> 代表 Michael D
> Kinney
> 发送时间: 2021年1月5日 13:05
> 收件人: devel@edk2.groups.io
> 抄送: Hao A Wu <hao.a.wu@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Bret Barkelew <Bret.Barkelew@microsoft.com>
> 主题: [edk2-devel] [stable/202011][Patch 0/2]
> MdeModulePkg/Variable/RuntimeDxe:Restore Variable Lock Protocol
> behavior
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3111
> 
> Cherry-pick the following commits from edk2/master to stable/202011.
> 
> (cherry picked from commit dcaa93936591883aa7826eb45ef00416ad82ef08)
> (cherry picked from commit a18a9bde36d2ffc12df29cdced1efa1f8f9f2021)
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> Bret Barkelew (1):
>   MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol
>     behavior
> 
> Michael D Kinney (1):
>   MdeModulePkg/Variable/RuntimeDxe: Add Variable Lock Protocol Unit
>     Tests
> 
>  MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
>  .../VariableLockRequestToLockUnitTest.c       | 565
> ++++++++++++++++++
>  .../VariableLockRequestToLockUnitTest.inf     |  36 ++
>  .../RuntimeDxe/VariableLockRequestToLock.c    |  95 +--
>  4 files changed, 671 insertions(+), 36 deletions(-)
>  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> leLockRequestToLockUnitTest.c
>  create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Variab
> leLockRequestToLockUnitTest.inf
> 
> --
> 2.29.2.windows.2
> 
> 
> 
> 
>