Bump. This specific patch needs Reviews. - Bret ________________________________ From: devel@edk2.groups.io on behalf of Bret Barkelew via groups.io Sent: Tuesday, June 2, 2020 11:58 PM To: devel@edk2.groups.io Cc: Jian J Wang ; Hao A Wu ; liming.gao Subject: [EXTERNAL] [edk2-devel] [PATCH v5 12/14] MdeModulePkg: Change TCG MOR variables to use VariablePolicy https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2522&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&sdata=NAVaoXnmIgu2F0YobNn7oC5XNGudCoalxrB3nPZDl98%3D&reserved=0 These were previously using VarLock, which is being deprecated. Cc: Jian J Wang Cc: Hao A Wu Cc: Liming Gao Cc: Bret Barkelew Signed-off-by: Bret Barkelew --- MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 52 ++++++++++++++------ MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 52 +++++++++++++++----- MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf | 2 + MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf | 1 + 4 files changed, 82 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c index e7accf4ed806..b85f08c48c11 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c @@ -5,6 +5,7 @@ MOR lock control unsupported. Copyright (c) 2016, Intel Corporation. All rights reserved.
+Copyright (c) Microsoft Corporation. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -17,7 +18,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include #include "Variable.h" -extern EDKII_VARIABLE_LOCK_PROTOCOL mVariableLock; +#include +#include /** This service is an MOR/MorLock checker handler for the SetVariable(). @@ -77,11 +79,6 @@ MorLockInit ( NULL // Data ); - // - // Need set this variable to be read-only to prevent other module set it. - // - VariableLockRequestToLock (&mVariableLock, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, &gEfiMemoryOverwriteRequestControlLockGuid); - // // The MOR variable can effectively improve platform security only when the // MorLock variable protects the MOR variable. In turn MorLock cannot be made @@ -99,11 +96,6 @@ MorLockInit ( 0, // DataSize NULL // Data ); - VariableLockRequestToLock ( - &mVariableLock, - MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, - &gEfiMemoryOverwriteControlDataGuid - ); return EFI_SUCCESS; } @@ -118,7 +110,39 @@ MorLockInitAtEndOfDxe ( VOID ) { - // - // Do nothing. - // + EFI_STATUS Status; + EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy; + + // First, we obviously need to locate the VariablePolicy protocol. + Status = gBS->LocateProtocol( &gEdkiiVariablePolicyProtocolGuid, NULL, (VOID**)&VariablePolicy ); + if (EFI_ERROR( Status )) { + DEBUG(( DEBUG_ERROR, "%a - Could not locate VariablePolicy protocol! %r\n", __FUNCTION__, Status )); + return; + } + + // If we're successful, go ahead and set the policies to protect the target variables. + Status = RegisterBasicVariablePolicy( VariablePolicy, + &gEfiMemoryOverwriteRequestControlLockGuid, + MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, + 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 ); + if (EFI_ERROR( Status )) { + DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status )); + } + Status = RegisterBasicVariablePolicy( VariablePolicy, + &gEfiMemoryOverwriteControlDataGuid, + MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, + 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 ); + if (EFI_ERROR( Status )) { + DEBUG(( DEBUG_ERROR, "%a - Could not lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status )); + } + + return; } diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c index 085f82035f4b..ee37942a6b0c 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c @@ -19,7 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "Variable.h" #include - +#include #include typedef struct { @@ -422,6 +422,8 @@ MorLockInitAtEndOfDxe ( { UINTN MorSize; EFI_STATUS MorStatus; + EFI_STATUS Status; + VARIABLE_POLICY_ENTRY *NewPolicy; if (!mMorLockInitializationRequired) { // @@ -494,11 +496,25 @@ MorLockInitAtEndOfDxe ( // The MOR variable is absent; the platform firmware does not support it. // Lock the variable so that no other module may create it. // - VariableLockRequestToLock ( - NULL, // This - MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, - &gEfiMemoryOverwriteControlDataGuid - ); + NewPolicy = NULL; + Status = CreateBasicVariablePolicy( &gEfiMemoryOverwriteControlDataGuid, + MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, + 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 ); + } + if (EFI_ERROR( Status )) { + DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, Status )); + ASSERT_EFI_ERROR( Status ); + } + if (NewPolicy != NULL) { + FreePool( NewPolicy ); + } // // Delete the MOR Control Lock variable too (should it exists for some @@ -514,9 +530,23 @@ MorLockInitAtEndOfDxe ( ); mMorLockPassThru = FALSE; - VariableLockRequestToLock ( - NULL, // This - MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, - &gEfiMemoryOverwriteRequestControlLockGuid - ); + NewPolicy = NULL; + Status = CreateBasicVariablePolicy( &gEfiMemoryOverwriteRequestControlLockGuid, + MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, + 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 ); + } + if (EFI_ERROR( Status )) { + DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTION__, MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, Status )); + ASSERT_EFI_ERROR( Status ); + } + if (NewPolicy != NULL) { + FreePool( NewPolicy ); + } } diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf index 48ac167906f7..8debc560e6dc 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf @@ -71,6 +71,7 @@ [LibraryClasses] AuthVariableLib VarCheckLib VariablePolicyLib + VariablePolicyHelperLib [Protocols] gEfiFirmwareVolumeBlockProtocolGuid ## CONSUMES @@ -80,6 +81,7 @@ [Protocols] gEfiVariableWriteArchProtocolGuid ## PRODUCES gEfiVariableArchProtocolGuid ## PRODUCES gEdkiiVariableLockProtocolGuid ## PRODUCES + gEdkiiVariablePolicyProtocolGuid ## CONSUMES gEdkiiVarCheckProtocolGuid ## PRODUCES [Guids] diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf index d8f480be27cc..62f2f9252f43 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf @@ -76,6 +76,7 @@ [LibraryClasses] SynchronizationLib VarCheckLib VariablePolicyLib + VariablePolicyHelperLib [Protocols] gEfiSmmFirmwareVolumeBlockProtocolGuid ## CONSUMES -- 2.26.2.windows.1.8.g01c50adf56.20200515075929 -=-=-=-=-=-= Groups.io Links: You receive all messages sent to this group. View/Reply Online (#60649): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F60649&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&sdata=7J764sVxeEu973uDLn7KBpquLZp7j9A0QzxlCyeaOFM%3D&reserved=0 Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F74646438%2F1852292&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730722710&sdata=I51sthKt1j%2FdJExzKr2jwopeDgdDimIBPmOBU55KxoM%3D&reserved=0 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&data=02%7C01%7Cbret.barkelew%40microsoft.com%7Cc4837d136ec548d67adc08d807a3e192%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637267747730732700&sdata=XQ8938isA26a%2BDE1oLinYkb49yAq2BDCPnBhyiECgLg%3D&reserved=0 [bret.barkelew@microsoft.com] -=-=-=-=-=-=