Could you help make the code consistent to call VariableServiceSetVariable() for the Attributes? One original for MorLock is using real attributes, the new you added for Mor will use 0 attributes. How about to both use 0 attributes.
Another question, why empty MorLockInitAtEndOfDxe() needs to be implemented in TcgMorLockDxe.c and called in VariableDxe.c? Thanks, Star -----Original Message----- From: Laszlo Ersek [mailto:[email protected]] Sent: Tuesday, October 10, 2017 9:25 PM To: edk2-devel-01 <[email protected]> Cc: Dong, Eric <[email protected]>; Yao, Jiewen <[email protected]>; Zeng, Star <[email protected]> Subject: [PATCH] MdeModulePkg/Variable/RuntimeDxe: delete & lock MOR in the absence of SMM VariableRuntimeDxe deletes and locks the MorLock variable in MorLockInit(), with the argument that any protection provided by MorLock can be circumvented if MorLock can be overwritten by unprivileged code (i.e., outside of SMM). Extend the argument and the logic to the MOR variable, which is supposed to be protected by MorLock. This change was suggested by Star; it is inspired by earlier VariableSmm commit fda8f631edbb ("MdeModulePkg/Variable/RuntimeDxe: delete and lock OS-created MOR variable", 2017-10-03). Cc: Eric Dong <[email protected]> Cc: Jiewen Yao <[email protected]> Cc: Star Zeng <[email protected]> Suggested-by: Star Zeng <[email protected]> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <[email protected]> --- Notes: Repo: https://github.com/lersek/edk2.git Branch: del_and_lock_mor_without_smm MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 24 ++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c index 7142e2da2073..1610c7aa0706 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c @@ -69,29 +69,53 @@ EFI_STATUS MorLockInit ( VOID ) { // // Always clear variable to report unsupported to OS. // The reason is that the DXE version is not proper to provide *protection*. // BIOS should use SMM version variable driver to provide such capability. // VariableServiceSetVariable ( MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, &gEfiMemoryOverwriteRequestControlLockGuid, EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS, 0, NULL ); // // 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 + // secure without SMM support in the platform firmware (see above). + // + // Thus, delete the MOR variable, should it exist for any reason (some OSes + // are known to create MOR unintentionally, in an attempt to set it), then + // also lock the MOR variable, in order to prevent other modules from + // creating it. + // + VariableServiceSetVariable ( + MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, + &gEfiMemoryOverwriteControlDataGuid, + 0, // Attributes + 0, // DataSize + NULL // Data + ); + VariableLockRequestToLock ( + &mVariableLock, + MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME, + &gEfiMemoryOverwriteControlDataGuid + ); + return EFI_SUCCESS; } /** Delayed initialization for MOR Control Lock at EndOfDxe. This function performs any operations queued by MorLockInit(). **/ -- 2.14.1.3.gb7cf6e02401b _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
