Abhi: Sorry for the missing patch. I agree Michael comment. Can you help update the patch? If yes, you can add my Reviewed-by: Liming Gao <gaolim...@byosoft.com.cn>
Thanks Liming > -----邮件原件----- > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Michael > Kubacki > 发送时间: 2023年6月9日 4:58 > 收件人: devel@edk2.groups.io; abhi.si...@arm.com > 主题: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variable: > TcgMorLockSmm Key Mismatch changes lock state > > Acked-by: Michael Kubacki <michael.kuba...@microsoft.com> > > Inline code comment below. > > On 4/12/2023 5:25 PM, Abhimanyu Singh wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4410 > > > > Inside TcgMorLockSmm.c, the SetVariableCheckHandlerMorLock() function > > contains a scenario to prevent a possible dictionary attack on the MorLock > > Key in accordance with the TCG Platform Reset Mitigation Spec v1.10. > > > > The mechanism to prevent this attack must also change the MorLock > Variable > > Value to 0x01 to indicate Locked Without Key. > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > Cc: Liming Gao <gaolim...@byosoft.com.cn> > > Signed-off-by: Abhi Singh <abhi.si...@arm.com> > > --- > > MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c | 4 > ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c > > index da1105ff073e..a76db18ef877 100644 > > --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c > > +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c > > @@ -312,6 +312,10 @@ SetVariableCheckHandlerMorLock ( > > mMorLockState = MorLockStateLocked; > > > > mMorLockKeyEmpty = TRUE; > > > > ZeroMem (mMorLockKey, sizeof (mMorLockKey)); > > > > + // > > > > + // Update value to reflect locked without key > > > > + // > > > > + SetMorLockVariable (MOR_LOCK_DATA_LOCKED_WITHOUT_KEY); > > I know the TCG Reset Attack Mitigation Specification requires > EFI_ACCESS_DENIED to be returned from this function in this case but > SetMorLockVariable() returns a status code. > > I suggest capturing that followed by an ASSERT_EFI_ERROR (Status) to at > least help raise visibility of unexpected errors in builds with asserts > enabled. > Do you mean ASSERT_EFI_ERROR (Status) return from SetMorLockVariable () API? I agree this suggestion. Thanks Liming > > > > return EFI_ACCESS_DENIED; > > > > } > > > > } > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#106315): https://edk2.groups.io/g/devel/message/106315 Mute This Topic: https://groups.io/mt/99763325/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
