回复: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
gaoliming
gaoliming at byosoft.com.cn
Thu Dec 10 03:16:28 UTC 2020
Mike:
I agree Hao comment. There is the similar code logic in VariablePolicyLib
library. They can be shared.
Besides, I suggest to split this patch. One is the bug fix to restore
Variable Lock Protocol behavior, another is to add Variable driver unit
test.
Thanks
Liming
> -----邮件原件-----
> 发件人: Wu, Hao A <hao.a.wu at intel.com>
> 发送时间: 2020年12月10日 10:25
> 收件人: devel at edk2.groups.io; Kinney, Michael D
> <michael.d.kinney at intel.com>
> 抄送: Bret Barkelew <bret.barkelew at microsoft.com>; Liming Gao
> <gaoliming at byosoft.com.cn>
> 主题: RE: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/RuntimeDxe:
> Restore Variable Lock Protocol behavior
>
> > -----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 (#68629): https://edk2.groups.io/g/devel/message/68629
Mute This Topic: https://groups.io/mt/78846657/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