From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.8031.1687824591593495385 for ; Mon, 26 Jun 2023 17:09:51 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@linux.microsoft.com header.s=default header.b=e4vWj63Y; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 9E9CF20553F0; Mon, 26 Jun 2023 17:09:50 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 9E9CF20553F0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1687824591; bh=iiBusTZs60ghVzVUi7EJJCZ83oCIosGoZTkRpuaitEQ=; h=Date:Subject:To:References:From:In-Reply-To:From; b=e4vWj63YHUfn0/2RszYZda+0L42N5nn1j6rcvTWRNfT+j3lvaYyFxj5TqqeJ0vmWS foJWeu5J/loDfpMgCKyv3eK26gK5NmJ4RCoN2KqJB4L4ILWhiC3PcPSm72CgxKthPO rQ2I8oknzjU6CwvLpe+YBZpCi8InYlYxs4XAzHcE= Message-ID: <2c218424-e1c7-0d33-d770-25a8e5f1da9d@linux.microsoft.com> Date: Mon, 26 Jun 2023 20:09:49 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: =?UTF-8?B?UmU6IOWbnuWkjTogW2VkazItZGV2ZWxdIFtQQVRDSCB2MSAxLzFdIE1kZU1vZHVsZVBrZy9WYXJpYWJsZTogVGNnTW9yTG9ja1NtbSBLZXkgTWlzbWF0Y2ggY2hhbmdlcyBsb2NrIHN0YXRl?= To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, Abhi.Singh@arm.com References: <20230412212505.538013-1-Abhi.Singh@arm.com> <03bd01d9a705$150e6400$3f2b2c00$@byosoft.com.cn> From: "Michael Kubacki" In-Reply-To: <03bd01d9a705$150e6400$3f2b2c00$@byosoft.com.cn> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable On 6/24/2023 9:33 PM, gaoliming via groups.io wrote: > Abhi: > Sorry for the missing patch. I agree Michael comment. Can you help upd= ate the patch? If yes, you can add my Reviewed-by: Liming Gao >=20 > Thanks > Liming >> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- >> =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Michael >> Kubacki >> =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2023=E5=B9=B46=E6=9C=889=E6=97=A5 = 4:58 >> =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; Abhi.Singh@arm.com >> =E4=B8=BB=E9=A2=98: Re: [edk2-devel] [PATCH v1 1/1] MdeModulePkg/Variabl= e: >> TcgMorLockSmm Key Mismatch changes lock state >> >> Acked-by: Michael Kubacki >> >> Inline code comment below. >> >> On 4/12/2023 5:25 PM, Abhimanyu Singh wrote: >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4410 >>> >>> Inside TcgMorLockSmm.c, the SetVariableCheckHandlerMorLock() function >>> contains a scenario to prevent a possible dictionary attack on the MorL= ock >>> 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 >>> Cc: Liming Gao >>> Signed-off-by: Abhi Singh >>> --- >>> 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 =3D MorLockStateLocked; >>> >>> mMorLockKeyEmpty =3D 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. >> >=20 > Do you mean ASSERT_EFI_ERROR (Status) return from SetMorLockVariable () A= PI? > I agree this suggestion. >=20 Correct > Thanks > Liming >>> >>> return EFI_ACCESS_DENIED; >>> >>> } >>> >>> } >>> >> >> >> >> >=20 >=20 >=20 >=20 >=20 >=20 >=20