回复: [edk2-devel] [Patch v4 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior
gaoliming
gaoliming at byosoft.com.cn
Mon Dec 14 01:39:22 UTC 2020
Reviewed-by: Liming Gao <gaoliming at byosoft.com.cn>
> -----邮件原件-----
> 发件人: bounce+27952+68702+4905953+8761045 at groups.io
> <bounce+27952+68702+4905953+8761045 at groups.io> 代表 Michael D
> Kinney
> 发送时间: 2020年12月11日 16:01
> 收件人: devel at edk2.groups.io
> 抄送: Bret Barkelew <bret.barkelew at microsoft.com>; Hao A Wu
> <hao.a.wu at intel.com>; Liming Gao <gaoliming at byosoft.com.cn>; Bret
> Barkelew <Bret.Barkelew at microsoft.com>
> 主题: [edk2-devel] [Patch v4 1/2] 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.
>
> 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>
> ---
> .../RuntimeDxe/VariableLockRequestToLock.c | 95 ++++++++++++-------
> 1 file changed, 59 insertions(+), 36 deletions(-)
>
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> index 4aa854aaf260..7d87e50efdcd 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo
> ck.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, 0,
NULL);
> + 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
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68747): https://edk2.groups.io/g/devel/message/68747
Mute This Topic: https://groups.io/mt/78941834/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