On 10/25/17 03:33, Zeng, Star wrote: > Reviewed-by: Star Zeng <[email protected]>
Commit 704b71d7e11f. Thank you, Star! Laszlo > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Laszlo > Ersek > Sent: Tuesday, October 24, 2017 11:38 PM > To: edk2-devel-01 <[email protected]> > Cc: Yao, Jiewen <[email protected]>; Dong, Eric <[email protected]>; > Zeng, Star <[email protected]> > Subject: [edk2] [PATCH v2] 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. Pass Attributes=0 when deleting MorLock and > MOR both. > > 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: > v2: > - Use Attributes=0 for deleting MorLock too [Star] > - Branch: del_and_lock_mor_without_smm_v2 > > v1: > - Branch: del_and_lock_mor_without_smm > > Repo: https://github.com/lersek/edk2.git > > MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c | 30 > ++++++++++++++++++-- > 1 file changed, 27 insertions(+), 3 deletions(-) > > diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c > index 7142e2da2073..fb4e13ab25a7 100644 > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c > @@ -78,15 +78,39 @@ MorLockInit ( > VariableServiceSetVariable ( > MEMORY_OVERWRITE_REQUEST_CONTROL_LOCK_NAME, > &gEfiMemoryOverwriteRequestControlLockGuid, > - EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | > EFI_VARIABLE_RUNTIME_ACCESS, > - 0, > - NULL > + 0, // Attributes > + 0, // DataSize > + 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 > + // 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; > } > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel

