Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision

["Gao, Zhichao" <zhichao.gao@intel.com>]

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;
       }
      }

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.gao@intel.com>
> Cc: Terry Lee <terry.lee@hpe.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Zhang, Chao B <chao.b.zhang@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.lee@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.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Chao Zhang <chao.b.zhang@intel.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@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
> 
> 
>