(+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?
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 (#62329): https://edk2.groups.io/g/devel/message/62329
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]
-=-=-=-=-=-=-=-=-=-=-=-
