* [Patch 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol compat
@ 2020-12-09 6:17 Michael D Kinney
0 siblings, 0 replies; only message in thread
From: Michael D Kinney @ 2020-12-09 6:17 UTC (permalink / raw)
To: devel; +Cc: Bret Barkelew, Hao A Wu, Liming Gao, Bret Barkelew
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.
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>
---
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
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2020-12-09 6:18 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-09 6:17 [Patch 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol compat Michael D Kinney
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox