From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.7757.1607570191011080894 for ; Wed, 09 Dec 2020 19:16:32 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Thu, 10 Dec 2020 11:16:25 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Wu, Hao A'" , , "'Kinney, Michael D'" Cc: "'Bret Barkelew'" References: <20201209180605.1409-1-michael.d.kinney@intel.com> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BhdGNoIHYyIDEvMV0gTWRlTW9kdWxlUGtnL1ZhcmlhYmxlL1J1bnRpbWVEeGU6IFJlc3RvcmUgVmFyaWFibGUgTG9jayBQcm90b2NvbCBiZWhhdmlvcg==?= Date: Thu, 10 Dec 2020 11:16:28 +0800 Message-ID: <004d01d6cea2$d9b15670$8d140350$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQHQSuV2IQ1nv7KDY/FBKjKC86AZyQH0Kw6fqe0N1eA= Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Mike: I agree Hao comment. There is the similar code logic in VariablePolicyLi= b library. They can be shared.=20 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.=20 Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: Wu, Hao A > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA12=D4=C210=C8=D5 10:25 > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io; Kinney, Michael D > > =B3=AD=CB=CD: Bret Barkelew ; Liming Gao > > =D6=F7=CC=E2: RE: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/Runt= imeDxe: > Restore Variable Lock Protocol behavior >=20 > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Michael > > D Kinney > > Sent: Thursday, December 10, 2020 2:06 AM > > To: devel@edk2.groups.io > > Cc: Bret Barkelew ; Wu, Hao A > > ; Liming Gao ; Bret > > Barkelew > > Subject: [edk2-devel] [Patch v2 1/1] MdeModulePkg/Variable/RuntimeDxe: > > Restore Variable Lock Protocol behavior > > > > From: Bret Barkelew > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D3111 > > > > 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 variab= le is > locked > > multiple times (because it should "be locked"). > > > > Refactor the shim to confirm that the variable is indeed locked and th= en > > 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. >=20 >=20 > Hello, >=20 > 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? >=20 > A couple of minor inline comments below: >=20 >=20 > > > > Add host based unit tests for the multiple lock case using Variable Lo= ck > > Protocol, Variable Policy Protocol, and mixes of Variable Lock Protoco= l 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 | 363 > +++++++++++++-- > > 4 files changed, 809 insertions(+), 35 deletions(-) create mode 1006= 44 > > 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 { > > + > > + > > > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyL= i > > b.inf > > + > > > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Va > > riablePolicyHelperLib.inf > > + > > + > > + > > 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 > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include #include > > + #include #inclu= de > > + > > + > > +#include > > + > > +#define UNIT_TEST_NAME "VarPol/VarLock Shim Unit Test" > > +#define UNIT_TEST_VERSION "1.0" > > + > > +///=3D=3D=3D CODE UNDER TEST > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > +=3D=3D=3D=3D > > + > > +EFI_STATUS > > +EFIAPI > > +VariableLockRequestToLock ( > > + IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This, > > + IN CHAR16 *VariableName, > > + IN EFI_GUID *VendorGuid > > + ); > > + > > +///=3D=3D=3D TEST DATA > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > + > > +// > > +// Test GUID 1 {F955BA2D-4A2C-480C-BFD1-3CC522610592} > > +// > > +EFI_GUID mTestGuid1 =3D { > > + 0xf955ba2d, 0x4a2c, 0x480c, {0xbf, 0xd1, 0x3c, 0xc5, 0x22, 0x61, 0x= 5, > > +0x92} }; > > + > > +// > > +// Test GUID 2 {2DEA799E-5E73-43B9-870E-C945CE82AF3A} > > +// > > +EFI_GUID mTestGuid2 =3D { > > + 0x2dea799e, 0x5e73, 0x43b9, {0x87, 0xe, 0xc9, 0x45, 0xce, 0x82, 0xa= f, > > +0x3a} }; > > + > > +// > > +// Test GUID 3 {698A2BFD-A616-482D-B88C-7100BD6682A9} > > +// > > +EFI_GUID mTestGuid3 =3D { > > + 0x698a2bfd, 0xa616, 0x482d, {0xb8, 0x8c, 0x71, 0x0, 0xbd, 0x66, 0x8= 2, > > +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 > > + > > +///=3D=3D=3D HELPER FUNCTIONS > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > +=3D=3D=3D=3D > > + > > +/** > > + 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 =3D (UINT32)mock(); > > + MockedDataSize =3D (UINTN)mock(); > > + MockedData =3D (VOID*)(UINTN)mock(); > > + MockedReturn =3D (EFI_STATUS)mock(); > > + > > + if (Attributes !=3D NULL) { > > + *Attributes =3D MockedAttr; > > + } > > + if (Data !=3D NULL && !EFI_ERROR (MockedReturn)) { > > + CopyMem (Data, MockedData, MockedDataSize); } > > + > > + *DataSize =3D 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(); > > +} > > + > > +///=3D=3D=3D TEST CASES > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > + > > +///=3D=3D=3D=3D=3D SHIM SUITE > > +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > > =3D=3D > > + > > +/** > > + 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 =3D 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 Loc= k > Procol. >=20 >=20 > Minor comment, typo for 'Procol' -> 'Protocol' >=20 >=20 > > + 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 =3D VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, > > + &mTestGuid1); UT_ASSERT_NOT_EFI_ERROR (Status); > > + > > + Status =3D 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 =3D 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 =3D RegisterVariablePolicy (NewEntry); > > + > > + Status =3D 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 >=20 >=20 > 'succced' -> 'succeed' >=20 >=20 > > + 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 =3D 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 =3D RegisterVariablePolicy (NewEntry); > > + > > + Status =3D 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 >=20 >=20 > 'succced' -> 'succeed' >=20 > Best Regards, > Hao Wu >=20 >=20 > > +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 =3D VariableLockRequestToLock (NULL, TEST_VAR_1_NAME, > > + &mTestGuid1); UT_ASSERT_NOT_EFI_ERROR (Status); > > + > > + // Create a variable policy that locks the variable. > > + Status =3D 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 =3D 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 =3D 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 =3D InitUnitTestFramework (&Framework, UNIT_TEST_NAME, > > + gEfiCallerBaseName, UNIT_TEST_VERSION); if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "Failed in InitUnitTestFramework. Status > > =3D %r\n", Status)); > > + goto EXIT; > > + } > > + > > + // > > + // Add all test suites and tests. > > + // > > + Status =3D CreateUnitTestSuite ( > > + &ShimTests, Framework, > > + "Variable Lock Shim Tests", "VarPolicy.VarLockShim", NUL= L, > NULL > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestSuite for > > ShimTests\n")); > > + Status =3D 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 shoul= d > 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 =3D RunAllTestSuites (Framework); > > + > > +EXIT: > > + if (Framework !=3D 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 =3D 0x00010017 > > + BASE_NAME =3D VariableLockRequestToLockUnitTest > > + FILE_GUID =3D A7388B6C-7274-4717-9649-BDC5DFD1FCBE > > + VERSION_STRING =3D 1.0 > > + MODULE_TYPE =3D HOST_APPLICATION > > + > > +# > > +# The following information is for reference only and not required by the > > build tools. > > +# > > +# VALID_ARCHITECTURES =3D 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 > > - > > #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 valu= e > > contains > > + the priority of the match. Lower > number =3D=3D 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 =3D FALSE; > > + CalculatedPriority =3D MATCH_PRIORITY_EXACT; > > + > > + // > > + // Step 1: If the GUID doesn't match, we're done. No need to evalua= te > > 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 enti= re > > namespace. > > + // Missing Name is indicated by size being equal to name. > > + // > > + if (EvalEntry->Size =3D=3D EvalEntry->OffsetToName) { > > + CalculatedPriority =3D MATCH_PRIORITY_MIN; > > + Result =3D TRUE; > > + goto Exit; > > + } > > + > > + // > > + // Now that we know the name exists, get it. > > + // > > + PolicyName =3D GET_POLICY_NAME (EvalEntry); > > + > > + // > > + // Evaluate the name against the policy name and check for a match. > > + // Account for any wildcards. > > + // > > + Index =3D 0; > > + Result =3D TRUE; > > + // > > + // Keep going until the end of both strings. > > + // > > + while (PolicyName[Index] !=3D CHAR_NULL || VariableName[Index] !=3D > > CHAR_NULL) { > > + // > > + // If we don't have a match... > > + // > > + if (PolicyName[Index] !=3D VariableName[Index] || PolicyName[Inde= x] > =3D=3D > > '#') { > > + // > > + // If this is a numerical wildcard, we can consider it a match = if we > alter > > + // the priority. > > + // > > + if (PolicyName[Index] =3D=3D L'#' && > > + ((L'0' <=3D VariableName[Index] && VariableName[Index] <= =3D > L'9') || > > + (L'A' <=3D VariableName[Index] && VariableName[Index] <= =3D > L'F') || > > + (L'a' <=3D VariableName[Index] && VariableName[Index] <= =3D > L'f'))) { > > + if (CalculatedPriority < MATCH_PRIORITY_MIN) { > > + CalculatedPriority++; > > + } > > + // > > + // Otherwise, not a match. > > + // > > + } else { > > + Result =3D FALSE; > > + goto Exit; > > + } > > + } > > + Index++; > > + } > > + > > +Exit: > > + if (Result && MatchPriority !=3D NULL) { > > + *MatchPriority =3D 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 =3D NULL; > > + MatchPriority =3D MATCH_PRIORITY_EXACT; > > + > > + // > > + // Walk all entries in the table, looking for matches. > > + // > > + CurrentEntry =3D (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 =3D=3D NULL || CurrentPriority < MatchPriority) = { > > + BestResult =3D CurrentEntry; > > + MatchPriority =3D CurrentPriority; > > + } > > + > > + // > > + // If you've hit the highest-priority match, can exit now. > > + // > > + if (MatchPriority =3D=3D 0) { > > + break; > > + } > > + } > > + > > + // > > + // If we're still in the loop, move to the next entry. > > + // > > + CurrentEntry =3D GET_NEXT_POLICY (CurrentEntry); } > > + > > + // > > + // If a return priority was requested, return it. > > + // > > + if (ReturnPriority !=3D NULL) { > > + *ReturnPriority =3D MatchPriority; > > + } > > + > > + return BestResult; > > +} > > + > > +/** > > + This helper function will dump and walk the current policy tables t= o > > +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 =3D TRUE; > > + > > + // > > + // First, we need to dump the existing policy table. > > + // > > + PolicyTableSize =3D 0; > > + PolicyTable =3D NULL; > > + Status =3D DumpVariablePolicy (PolicyTable, &PolicyTableSize); if > > + (Status !=3D EFI_BUFFER_TOO_SMALL) { > > + DEBUG ((DEBUG_ERROR, "%a - Failed to determine policy table > > size! %r\n", __FUNCTION__, Status)); > > + return FALSE; > > + } > > + PolicyTable =3D AllocateZeroPool (PolicyTableSize); if (PolicyTabl= e =3D=3D > > + NULL) { > > + DEBUG ((DEBUG_ERROR, "%a - Failed to allocated space for policy > table! > > 0x%X\n", __FUNCTION__, PolicyTableSize)); > > + return FALSE; > > + } > > + Status =3D DumpVariablePolicy (PolicyTable, &PolicyTableSize); if > > + (EFI_ERROR (Status)) { > > + DEBUG ((DEBUG_ERROR, "%a - Failed to dump policy table! %r\n", > > __FUNCTION__, Status)); > > + Result =3D FALSE; > > + goto Exit; > > + } > > + > > + // > > + // Now we need to walk the table looking for a match. > > + // > > + MatchPolicy =3D GetBestPolicyMatch ( > > + PolicyTable, > > + PolicyTableSize, > > + VariableName, > > + VendorGuid, > > + &MatchPriority > > + ); > > + if (MatchPolicy !=3D NULL && MatchPriority !=3D MATCH_PRIORITY_EXAC= T) > { > > + DEBUG ((DEBUG_ERROR, "%a - We would not have expected a > non-exact > > match! %d\n", __FUNCTION__, MatchPriority)); > > + Result =3D FALSE; > > + goto Exit; > > + } > > + > > + // > > + // Now we can check to see whether this variable is currently locke= d. > > + // > > + if (MatchPolicy->LockPolicyType !=3D > VARIABLE_POLICY_TYPE_LOCK_NOW) { > > + DEBUG ((DEBUG_INFO, "%a - Policy may not lock variable! %d\n", > > __FUNCTION__, MatchPolicy->LockPolicyType)); > > + Result =3D FALSE; > > + goto Exit; > > + } > > + > > +Exit: > > + if (PolicyTable !=3D 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 b= e > > 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 b= e > > 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 =3D NULL; > > - Status =3D 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 =3D 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 =3D RegisterVariablePolicy( NewPolicy ); > > + Status =3D 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 =3D=3D EFI_ALREADY_STARTED) { > > + if (IsVariableAlreadyLocked (VariableName, VendorGuid)) { > > + DEBUG ((DEBUG_ERROR, " Variable: %g %s is already > locked!\n", > > VendorGuid, VariableName)); > > + Status =3D 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 !=3D NULL) { > > FreePool( NewPolicy ); > > -- > > 2.29.2.windows.2 > > > > > > > >=20 > >