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

["Michael D Kinney" <michael.d.kinney@intel.com>]

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>
---
 .../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