[edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
Wu, Hao A
hao.a.wu at intel.com
Thu Dec 10 02:24:36 UTC 2020
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Michael
> D Kinney
> Sent: Thursday, December 10, 2020 2:06 AM
> To: devel at edk2.groups.io
> Cc: Bret Barkelew <bret.barkelew at microsoft.com>; Wu, Hao A
> <hao.a.wu at intel.com>; Liming Gao <gaoliming at byosoft.com.cn>; Bret
> Barkelew <Bret.Barkelew at microsoft.com>
> Subject: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/RuntimeDxe:
> Restore Variable Lock Protocol behavior
>
> 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 and generate a DEBUG_ERROR message so
> the duplicate lock can be reported in a debug log and removed.
Hello,
Is it possible to reuse:
a) EvaluatePolicyMatch() and GetBestPolicyMatch() functions
b) Macros like GET_NEXT_POLICY, GET_POLICY_NAME and etc.
under MdeModulePkg\Library\VariablePolicyLib to reduce duplicate codes?
A couple of minor inline comments below:
>
> 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 | 363 +++++++++++++--
> 4 files changed, 809 insertions(+), 35 deletions(-) create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.c
> create mode 100644
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.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/MockUefiRuntimeSer
> vicesTableLib.inf
>
> @@ -30,3 +33,11 @@ [Components]
>
> ResetSystemLib|MdeModulePkg/Library/DxeResetSystemLib/DxeResetSyst
> emLib.inf
>
> UefiRuntimeServicesTableLib|MdeModulePkg/Library/DxeResetSystemLib/
> UnitTest/MockUefiRuntimeServicesTableLib.inf
> }
> +
> +
> MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Vari
> ableLockRequestToLockUnitTest.inf {
> + <LibraryClasses>
> +
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLi
> b.inf
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va
> riablePolicyHelperLib.inf
> + <PcdsFixedAtBuild>
> +
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdAllowVariablePolicyEnforcementDis
> abl
> + e|TRUE
> + }
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Va
> riableLockRequestToLockUnitTest.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Va
> riableLockRequestToLockUnitTest.c
> new file mode 100644
> index 000000000000..2f4c4d2f79f4
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Va
> ri
> +++ ableLockRequestToLockUnitTest.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.
Minor comment, typo for 'Procol' -> 'Protocol'
> + 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
'succced' -> 'succeed'
> + 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
'succced' -> 'succeed'
Best Regards,
Hao Wu
> +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/Va
> riableLockRequestToLockUnitTest.inf
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Va
> riableLockRequestToLockUnitTest.inf
> new file mode 100644
> index 000000000000..2a659d7e1370
> --- /dev/null
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/RuntimeDxeUnitTest/Va
> ri
> +++ ableLockRequestToLockUnitTest.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/VariableLockRequestToL
> ock.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToL
> ock.c
> index 4aa854aaf260..191de6b907c5 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToL
> ock.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToL
> o
> +++ ck.c
> @@ -1,67 +1,360 @@
> -/** @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, but also generate a DEBUG_ERROR message so
> the
> + // duplicate lock can be removed.
> + //
> + if (Status == EFI_ALREADY_STARTED) {
> + if (IsVariableAlreadyLocked (VariableName, VendorGuid)) {
> + DEBUG ((DEBUG_ERROR, " Variable: %g %s is already locked!\n",
> VendorGuid, VariableName));
> + 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 (#68620): https://edk2.groups.io/g/devel/message/68620
Mute This Topic: https://groups.io/mt/78835468/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