Date
1 - 3 of 3
[Patch v3 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
|
Michael D Kinney
toggle quoted message
Show quoted text
-----Original Message-----Thank you! You are exactly right. We do not want this logic used because we do not know what the valid DataSize and Attribute values are for the existing policy. For the call to ValidateSetVariable(), we should use DataSize=0 and Attributes=0. Status = ValidateSetVariable (VariableName, VendorGuid, 0, 0, NULL);
|
|
|
|
Wu, Hao A
toggle quoted message
Show quoted text
-----Original Message----- Hello Mike, Sorry for one thing to confirm. Is it possible when passing "0" as the "Attributes" parameter to function ValidateSetVariable() causes the below case in ValidateSetVariable(): // Check for attribute constraints. if ((ActivePolicy->AttributesMustHave & Attributes) != ActivePolicy->AttributesMustHave || (ActivePolicy->AttributesCantHave & Attributes) != 0) { ReturnStatus = EFI_INVALID_PARAMETER; DEBUG(( DEBUG_VERBOSE, "%a - Bad Attributes. 0x%X <> 0x%X:0x%X\n", __FUNCTION__, Attributes, ActivePolicy->AttributesMustHave, ActivePolicy->AttributesCantHave )); goto Exit; } if "ActivePolicy->AttributesMustHave" have any bit set? This will lead to VariableLockRequestToLock() to return an error status. Best Regards, Hao Wu + if (Status == EFI_WRITE_PROTECTED) { |
|
|
|
Michael D Kinney
From: Bret Barkelew <bret.barkelew@...>
https://bugzilla.tianocore.org/show_bug.cgi?id=3111 The VariableLock shim currently fails if called twice because the underlying Variable Policy engine returns an error if a policy is set on an existing variable. This breaks existing code which expect it to silently pass if a variable is locked multiple times (because it should "be locked"). Refactor the shim to confirm that the variable is indeed locked and then change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so the duplicate lock can be reported in a debug log and removed. Cc: Michael D Kinney <michael.d.kinney@...> Cc: Hao A Wu <hao.a.wu@...> Cc: Liming Gao <gaoliming@...> Signed-off-by: Bret Barkelew <Bret.Barkelew@...> --- .../RuntimeDxe/VariableLockRequestToLock.c | 95 ++++++++++++------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c index 4aa854aaf260..0715b527a0f7 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c @@ -1,67 +1,90 @@ -/** @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/MemoryAllocationLib.h> - -#include <Protocol/VariableLock.h> - -#include <Protocol/VariablePolicy.h> #include <Library/VariablePolicyLib.h> #include <Library/VariablePolicyHelperLib.h> - +#include <Protocol/VariableLock.h> /** 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) { + Status = ValidateSetVariable (VariableName, VendorGuid, 0, sizeof(""), ""); + if (Status == EFI_WRITE_PROTECTED) { + DEBUG ((DEBUG_ERROR, " Variable: %g %s is already locked!\n", VendorGuid, VariableName)); + Status = EFI_SUCCESS; + } else { + DEBUG ((DEBUG_ERROR, " Variable: %g %s can not be locked!\n", VendorGuid, VariableName)); + Status = EFI_ACCESS_DENIED; + } + } } - 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 |
|
|