On 7/10/20 1:43 AM, Laszlo Ersek wrote:
(+Marc-André, Stefan)
On 07/10/20 02:44, Gao, Zhichao wrote:
This bug is not obeserved by me. But I view the code. The condition is
incorrect and it would affect the TCG operation:
if (!mIsTcg2PPVerLowerThan_1_3) {
if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
//
// TCG2 PP1.3 spec defined operations that are reserved or
un-implemented
//
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
} else {
//
// TCG PP lower than 1.3. (1.0, 1.1, 1.2)
//
if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
RequestConfirmed = TRUE;
} else if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
}
I've found that code myself, but I'm not familiar enough with TPM PPI
stuff to understand immediately the effects of this change. I can see
that where we used to return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED before, we could now assign
"RequestConfirmed = TRUE", and vice versa, due to
"mIsTcg2PPVerLowerThan_1_3" being potentially inverted.
But what does that *mean*? What is the behavioral change that human
end-users, or software components, will experience?
The above code snipped is located in a default branch of a large switch
statement that handles most of the common PPI operations independent of
this change, so that at least is good.
I would say that in the worst case some of the operations not otherwise
handled may have mistakenly failed or could have been executed without
user confirmation/interaction. On Linux at least PPI requests can only
be sent by root.
Thanks
Laszlo
So I think it should be fixed.
Thanks,
Zhichao
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Thursday, July 9, 2020 6:02 PM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao....@intel.com>
Cc: Terry Lee <terry....@hpe.com>; Yao, Jiewen <jiewen....@intel.com>; Wang,
Jian J <jian.j.w...@intel.com>; Zhang, Chao B <chao.b.zh...@intel.com>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision
On 07/09/20 04:46, Gao, Zhichao wrote:
From: Terry Lee <terry....@hpe.com>
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
Tcg2PhysicalPresenceLibConstructor set the module variable
mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
Cc: Jiewen Yao <jiewen....@intel.com>
Cc: Jian J Wang <jian.j.w...@intel.com>
Cc: Chao Zhang <chao.b.zh...@intel.com>
Signed-off-by: Zhichao Gao <zhichao....@intel.com>
---
.../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
index 1c46d5e69d..8afaa0a785 100644
---
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
+++ esenceLib.c
@@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor ( {
EFI_STATUS Status;
- if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
*)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
+ if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
+ *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
+ sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
mIsTcg2PPVerLowerThan_1_3 = TRUE;
}
What is the practical impact of this bug / fix?
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62347): https://edk2.groups.io/g/devel/message/62347
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
