MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior

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>

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c

@@ -1,40 +1,41 @@
-/** @file -- VariableLockRequestToLock.c
+/** @file
-Temporary location of the RequestToLock shim code while
+  Temporary location of the RequestToLock shim code while projects
-projects are moved to VariablePolicy. Should be removed when deprecated.
+  are moved to VariablePolicy. Should be removed when deprecated.
 
-Copyright (c) Microsoft Corporation.
+  Copyright (c) Microsoft Corporation.
-SPDX-License-Identifier: BSD-2-Clause-Patent
+  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.
+  Mark a variable that will become read-only after leaving the DXE phase of
-  Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTOCOL is allowed.
+  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] VariableName  A pointer to the variable name that will be made
-  @param[in] VendorGuid    A pointer to the vendor GUID that will be made read-only subsequently.
+                           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
+  @retval EFI_SUCCESS           The variable specified by the VariableName and
-                                as pending to be read-only.
+                                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
+  @retval EFI_ACCESS_DENIED     EFI_END_OF_DXE_EVENT_GROUP_GUID or
-                                already been signaled.
+                                EFI_EVENT_GROUP_READY_TO_BOOT has already been
-  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock request.
+                                signaled.
+  @retval EFI_OUT_OF_RESOURCES  There is not enough resource to hold the lock
+                                request.
 **/
 EFI_STATUS
 EFIAPI
@@ -47,21 +48,43 @@ VariableLockRequestToLock (
   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,
+  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 );
+             &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 );