回复: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variable: TcgMorLockSmm Key Mismatch changes lock state
gaoliming via groups.io
gaoliming=byosoft.com.cn at groups.io
Sun Jun 25 01:33:45 UTC 2023
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 <gaoliming at byosoft.com.cn>
Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Michael
> Kubacki
> 发送时间: 2023年6月9日 4:58
> 收件人: devel at edk2.groups.io; Abhi.Singh at arm.com
> 主题: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variable:
> TcgMorLockSmm Key Mismatch changes lock state
>
> Acked-by: Michael Kubacki <michael.kubacki at 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.wang at intel.com>
> > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > Signed-off-by: Abhi Singh <Abhi.Singh at 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/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