From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web09.2806.1607494684262286357 for ; Tue, 08 Dec 2020 22:18:04 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: michael.d.kinney@intel.com) IronPort-SDR: cs5WZ47Z302ObrKdYgslZL6T3fPT8/I7ug48IyJ4gY6wFhmdbqrsexuHeH2tO87/r6NFeprLCO 76rA10l2eFbQ== X-IronPort-AV: E=McAfee;i="6000,8403,9829"; a="161787466" X-IronPort-AV: E=Sophos;i="5.78,404,1599548400"; d="scan'208";a="161787466" Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Dec 2020 22:18:02 -0800 IronPort-SDR: SNSQ4jDU7ch/EqV6rmmPko1IuGggHtX/0KikTW702eFld27Yk3q368Dwigfeu8wF9qAzv9/g78 YiEFb/TqeGsQ== X-IronPort-AV: E=Sophos;i="5.78,404,1599548400"; d="scan'208";a="407929678" Received: from mdkinney-mobl2.amr.corp.intel.com ([10.209.77.73]) by orsmga001-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Dec 2020 22:18:02 -0800 From: "Michael D Kinney" To: devel@edk2.groups.io Cc: Bret Barkelew , Hao A Wu , Liming Gao , Bret Barkelew Subject: [Patch 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol compat Date: Tue, 8 Dec 2020 22:17:54 -0800 Message-Id: <20201209061754.1229-1-michael.d.kinney@intel.com> X-Mailer: git-send-email 2.29.2.windows.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit From: Bret Barkelew 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 Cc: Hao A Wu Cc: Liming Gao Signed-off-by: Bret Barkelew --- 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 { + + VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf + + 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 +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include + +#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 - #include +#include #include - -#include - -#include #include #include +#include +// +// 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