[edk2-devel] [Patch 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol compat

Michael D Kinney michael.d.kinney at intel.com
Wed Dec 9 06:17:54 UTC 2020


From: Bret Barkelew <bret.barkelew at 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.

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 at intel.com>
Cc: Hao A Wu <hao.a.wu at intel.com>
Cc: Liming Gao <gaoliming at byosoft.com.cn>
Signed-off-by: Bret Barkelew <Bret.Barkelew at microsoft.com>
---
 MdeModulePkg/Test/MdeModulePkgHostTest.dsc    |  11 +
 .../VariableLockRequestToLockUnitTest.c       | 434 ++++++++++++++++++
 .../VariableLockRequestToLockUnitTest.inf     |  36 ++
 .../RuntimeDxe/VariableLockRequestToLock.c    | 360 +++++++++++++--
 4 files changed, 806 insertions(+), 35 deletions(-)
 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..2f4c4d2f79f4
--- /dev/null
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/VariableLockRequestToLockUnitTest.c
@@ -0,0 +1,434 @@
+/** @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 Procol.
+  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 succced
+  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);
+
+  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 succced 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,
+    "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
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
index 4aa854aaf260..7941b39b6c67 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c
@@ -1,67 +1,357 @@
-/** @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/BaseMemoryLib.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>
 
+//
+// NOTE: DO NOT USE THESE MACROS on any structure that has not been validated.
+//       Current table data has already been sanitized.
+//
+#define GET_NEXT_POLICY(CurPolicy)  (VARIABLE_POLICY_ENTRY*)((UINT8*)CurPolicy + CurPolicy->Size)
+#define GET_POLICY_NAME(CurPolicy)  (CHAR16*)((UINTN)CurPolicy + CurPolicy->OffsetToName)
+
+#define MATCH_PRIORITY_EXACT  0
+#define MATCH_PRIORITY_MIN    MAX_UINT8
+
+/**
+  This helper function evaluates a policy and determines whether it matches the
+  target variable. If matched, will also return a value corresponding to the
+  priority of the match.
+
+  The rules for "best match" are listed in the Variable Policy Spec.
+  Perfect name matches will return 0.
+  Single wildcard characters will return the number of wildcard characters.
+  Full namespaces will return MAX_UINT8.
+
+  @param[in]  EvalEntry      Pointer to the policy entry being evaluated.
+  @param[in]  VariableName   Same as EFI_SET_VARIABLE.
+  @param[in]  VendorGuid     Same as EFI_SET_VARIABLE.
+  @param[out] MatchPriority  [Optional] On finding a match, this value contains
+                             the priority of the match. Lower number == higher
+                             priority. Only valid if a match found.
+
+  @retval  TRUE   Current entry matches the target variable.
+  @retval  FALSE  Current entry does not match at all.
+
+**/
+STATIC
+BOOLEAN
+EvaluatePolicyMatch (
+  IN CONST VARIABLE_POLICY_ENTRY  *EvalEntry,
+  IN CONST CHAR16                 *VariableName,
+  IN CONST EFI_GUID               *VendorGuid,
+  OUT UINT8                       *MatchPriority  OPTIONAL
+  )
+{
+  BOOLEAN  Result;
+  CHAR16   *PolicyName;
+  UINT8    CalculatedPriority;
+  UINTN    Index;
+
+  Result = FALSE;
+  CalculatedPriority = MATCH_PRIORITY_EXACT;
+
+  //
+  // Step 1: If the GUID doesn't match, we're done. No need to evaluate anything else.
+  //
+  if (!CompareGuid (&EvalEntry->Namespace, VendorGuid)) {
+    goto Exit;
+  }
+
+  //
+  // If the GUID matches, check to see whether there is a Name associated
+  // with the policy. If not, this policy matches the entire namespace.
+  // Missing Name is indicated by size being equal to name.
+  //
+  if (EvalEntry->Size == EvalEntry->OffsetToName) {
+    CalculatedPriority = MATCH_PRIORITY_MIN;
+    Result = TRUE;
+    goto Exit;
+  }
+
+  //
+  // Now that we know the name exists, get it.
+  //
+  PolicyName = GET_POLICY_NAME (EvalEntry);
+
+  //
+  // Evaluate the name against the policy name and check for a match.
+  // Account for any wildcards.
+  //
+  Index = 0;
+  Result = TRUE;
+  //
+  // Keep going until the end of both strings.
+  //
+  while (PolicyName[Index] != CHAR_NULL || VariableName[Index] != CHAR_NULL) {
+    //
+    // If we don't have a match...
+    //
+    if (PolicyName[Index] != VariableName[Index] || PolicyName[Index] == '#') {
+      //
+      // If this is a numerical wildcard, we can consider it a match if we alter
+      // the priority.
+      //
+      if (PolicyName[Index] == L'#' &&
+            ((L'0' <= VariableName[Index] && VariableName[Index] <= L'9') ||
+             (L'A' <= VariableName[Index] && VariableName[Index] <= L'F') ||
+             (L'a' <= VariableName[Index] && VariableName[Index] <= L'f'))) {
+        if (CalculatedPriority < MATCH_PRIORITY_MIN) {
+          CalculatedPriority++;
+        }
+      //
+      // Otherwise, not a match.
+      //
+      } else {
+        Result = FALSE;
+        goto Exit;
+      }
+    }
+    Index++;
+  }
+
+Exit:
+  if (Result && MatchPriority != NULL) {
+    *MatchPriority = CalculatedPriority;
+  }
+  return Result;
+}
+
+/**
+  This helper function walks the current policy table and returns a pointer
+  to the best match, if any are found. Leverages EvaluatePolicyMatch() to
+  determine "best".
+
+  @param[in]  PolicyTable      Pointer to current policy table.
+  @param[in]  PolicyTableSize  Size of current policy table.
+  @param[in]  VariableName     Same as EFI_SET_VARIABLE.
+  @param[in]  VendorGuid       Same as EFI_SET_VARIABLE.
+  @param[out] ReturnPriority   [Optional] If pointer is provided, return the
+                               priority of the match. Same as EvaluatePolicyMatch().
+                               Only valid if a match is returned.
+
+  @retval     VARIABLE_POLICY_ENTRY*    Best match that was found.
+  @retval     NULL                      No match was found.
+
+**/
+STATIC
+VARIABLE_POLICY_ENTRY*
+GetBestPolicyMatch (
+  IN UINT8           *PolicyTable,
+  IN UINT32          PolicyTableSize,
+  IN CONST CHAR16    *VariableName,
+  IN CONST EFI_GUID  *VendorGuid,
+  OUT UINT8          *ReturnPriority  OPTIONAL
+  )
+{
+  VARIABLE_POLICY_ENTRY  *BestResult;
+  VARIABLE_POLICY_ENTRY  *CurrentEntry;
+  UINT8                  MatchPriority;
+  UINT8                  CurrentPriority;
+
+  BestResult = NULL;
+  MatchPriority = MATCH_PRIORITY_EXACT;
+
+  //
+  // Walk all entries in the table, looking for matches.
+  //
+  CurrentEntry = (VARIABLE_POLICY_ENTRY*)PolicyTable;
+  while ((UINTN)CurrentEntry < (UINTN)((UINT8*)PolicyTable + PolicyTableSize)) {
+    //
+    // Check for a match.
+    //
+    if (EvaluatePolicyMatch (CurrentEntry, VariableName, VendorGuid, &CurrentPriority)) {
+      //
+      // If match is better, take it.
+      //
+      if (BestResult == NULL || CurrentPriority < MatchPriority) {
+        BestResult = CurrentEntry;
+        MatchPriority = CurrentPriority;
+      }
+
+      //
+      // If you've hit the highest-priority match, can exit now.
+      //
+      if (MatchPriority == 0) {
+        break;
+      }
+    }
+
+    //
+    // If we're still in the loop, move to the next entry.
+    //
+    CurrentEntry = GET_NEXT_POLICY (CurrentEntry);
+  }
+
+  //
+  // If a return priority was requested, return it.
+  //
+  if (ReturnPriority != NULL) {
+    *ReturnPriority = MatchPriority;
+  }
+
+  return BestResult;
+}
+
+/**
+  This helper function will dump and walk the current policy tables to determine
+  whether a matching policy already exists that satisfies the lock request.
+
+  @param[in] VariableName  A pointer to the variable name that is being searched.
+  @param[in] VendorGuid    A pointer to the vendor GUID that is being searched.
+
+  @retval  TRUE   We can safely assume this variable is locked.
+  @retval  FALSE  An error has occurred or we cannot prove that the variable is
+                  locked.
+
+**/
+STATIC
+BOOLEAN
+IsVariableAlreadyLocked (
+  IN CHAR16    *VariableName,
+  IN EFI_GUID  *VendorGuid
+  )
+{
+  EFI_STATUS             Status;
+  UINT8                  *PolicyTable;
+  UINT32                 PolicyTableSize;
+  BOOLEAN                Result;
+  VARIABLE_POLICY_ENTRY  *MatchPolicy;
+  UINT8                  MatchPriority;
+
+  Result = TRUE;
+
+  //
+  // First, we need to dump the existing policy table.
+  //
+  PolicyTableSize = 0;
+  PolicyTable = NULL;
+  Status = DumpVariablePolicy (PolicyTable, &PolicyTableSize);
+  if (Status != EFI_BUFFER_TOO_SMALL) {
+    DEBUG ((DEBUG_ERROR, "%a - Failed to determine policy table size! %r\n", __FUNCTION__, Status));
+    return FALSE;
+  }
+  PolicyTable = AllocateZeroPool (PolicyTableSize);
+  if (PolicyTable == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a - Failed to allocated space for policy table! 0x%X\n", __FUNCTION__, PolicyTableSize));
+    return FALSE;
+  }
+  Status = DumpVariablePolicy (PolicyTable, &PolicyTableSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a - Failed to dump policy table! %r\n", __FUNCTION__, Status));
+    Result = FALSE;
+    goto Exit;
+  }
+
+  //
+  // Now we need to walk the table looking for a match.
+  //
+  MatchPolicy = GetBestPolicyMatch (
+                  PolicyTable,
+                  PolicyTableSize,
+                  VariableName,
+                  VendorGuid,
+                  &MatchPriority
+                  );
+  if (MatchPolicy != NULL && MatchPriority != MATCH_PRIORITY_EXACT) {
+    DEBUG ((DEBUG_ERROR, "%a - We would not have expected a non-exact match! %d\n", __FUNCTION__, MatchPriority));
+    Result = FALSE;
+    goto Exit;
+  }
+
+  //
+  // Now we can check to see whether this variable is currently locked.
+  //
+  if (MatchPolicy->LockPolicyType != VARIABLE_POLICY_TYPE_LOCK_NOW) {
+    DEBUG ((DEBUG_INFO, "%a - Policy may not lock variable! %d\n", __FUNCTION__, MatchPolicy->LockPolicyType));
+    Result = FALSE;
+    goto Exit;
+  }
+
+Exit:
+  if (PolicyTable != NULL) {
+    FreePool (PolicyTable);
+  }
+
+  return Result;
+}
 
 /**
   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.
+    //
+    if (Status == EFI_ALREADY_STARTED) {
+      if (IsVariableAlreadyLocked (VariableName, VendorGuid)) {
+        Status = EFI_SUCCESS;
+      }
+    }
   }
-  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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68489): https://edk2.groups.io/g/devel/message/68489
Mute This Topic: https://groups.io/mt/78823865/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list