[edk2-devel] [PATCH v2] SecurityPkg: Change default value source

Maggie Chu posted 1 patch 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20200515082743.1768-1-maggie.chu@intel.com
.../Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 2 +-
.../Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c  | 2 +-
.../PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf        | 3 +++
.../Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c  | 5 ++++-
.../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf        | 1 +
5 files changed, 10 insertions(+), 3 deletions(-)

[edk2-devel] [PATCH v2] SecurityPkg: Change default value source

Posted by Maggie Chu 1 week ago
https://bugzilla.tianocore.org/show_bug.cgi?id=2713
In current code, If TCG2_PHYSICAL_PRESENCE_FLAGS_VARIABLE variable
is not exist, code will get default value from two places.
This fix is to make the default value comes from the PCD
gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags

Signed-off-by: Maggie Chu <maggie.chu@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
---
v2 change:
Change patch title.

 .../Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 2 +-
 .../Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c  | 2 +-
 .../PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf        | 3 +++
 .../Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c  | 5 ++++-
 .../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf        | 1 +
 5 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
index 80e2e37bf4..bf793555aa 100644
--- a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c
@@ -1228,7 +1228,7 @@ Tcg2PhysicalPresenceLibGetManagementFlags (
                   &PpiFlags
                   );
   if (EFI_ERROR (Status)) {
-    PpiFlags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
+    PpiFlags.PPFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
   }
   return PpiFlags.PPFlags;
 }
diff --git a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
index a111351516..a2c157d8a7 100644
--- a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
@@ -47,7 +47,7 @@ Tcg2PhysicalPresenceLibGetManagementFlags (
                           &PpiFlags
                           );
   if (EFI_ERROR (Status)) {
-    PpiFlags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
+    PpiFlags.PPFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
   }
   return PpiFlags.PPFlags;
 }
diff --git a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf
index d34f232022..11ebaa9263 100644
--- a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf
@@ -43,5 +43,8 @@
 [Ppis]
   gEfiPeiReadOnlyVariable2PpiGuid       ## CONSUMES
 
+[Pcd]
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags       ## SOMETIMES_CONSUMES
+
 [Depex]
   gEfiPeiReadOnlyVariable2PpiGuid
diff --git a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
index 3827df9663..08ef5416d8 100644
--- a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
@@ -31,6 +31,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 EFI_SMM_VARIABLE_PROTOCOL  *mTcg2PpSmmVariable;
 BOOLEAN                    mIsTcg2PPVerLowerThan_1_3 = FALSE;
+UINT32                     mTcg2PhysicalPresenceFlags;
 
 /**
   The handler for TPM physical presence function:
@@ -162,7 +163,7 @@ Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
                                    &Flags
                                    );
     if (EFI_ERROR (Status)) {
-      Flags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
+      Flags.PPFlags = mTcg2PhysicalPresenceFlags;
     }
     ReturnCode = Tcg2PpVendorLibSubmitRequestToPreOSFunction (*OperationRequest, Flags.PPFlags, *RequestParameter);
   }
@@ -396,5 +397,7 @@ Tcg2PhysicalPresenceLibConstructor (
   Status = gSmst->SmmLocateProtocol (&gEfiSmmVariableProtocolGuid, NULL, (VOID**)&mTcg2PpSmmVariable);
   ASSERT_EFI_ERROR (Status);
 
+  mTcg2PhysicalPresenceFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
+
   return EFI_SUCCESS;
 }
diff --git a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
index e0e5fef5f1..1f40629e3b 100644
--- a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
@@ -50,6 +50,7 @@
 
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ## CONSUMES
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags        ## SOMETIMES_CONSUMES
 
 [Depex]
   gEfiSmmVariableProtocolGuid
-- 
2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59662): https://edk2.groups.io/g/devel/message/59662
Mute This Topic: https://groups.io/mt/74223343/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] SecurityPkg: Change default value source

Posted by Yao, Jiewen 1 week ago
Looks good to me.

Then can we remove TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT definition ?
It seems no longer useful.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Chu, Maggie <maggie.chu@intel.com>
> Sent: Friday, May 15, 2020 4:28 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2] SecurityPkg: Change default value source
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=2713
> In current code, If TCG2_PHYSICAL_PRESENCE_FLAGS_VARIABLE variable
> is not exist, code will get default value from two places.
> This fix is to make the default value comes from the PCD
> gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags
> 
> Signed-off-by: Maggie Chu <maggie.chu@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> ---
> v2 change:
> Change patch title.
> 
>  .../Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.c  | 2 +-
>  .../Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c  | 2 +-
>  .../PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.inf        | 3 +++
>  .../Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c  | 5
> ++++-
>  .../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf        | 1 +
>  5 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git
> a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib
> .c
> b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib
> .c
> index 80e2e37bf4..bf793555aa 100644
> ---
> a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib
> .c
> +++
> b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib
> .c
> @@ -1228,7 +1228,7 @@ Tcg2PhysicalPresenceLibGetManagementFlags (
>                    &PpiFlags
> 
>                    );
> 
>    if (EFI_ERROR (Status)) {
> 
> -    PpiFlags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
> 
> +    PpiFlags.PPFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
> 
>    }
> 
>    return PpiFlags.PPFlags;
> 
>  }
> 
> diff --git
> a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
> b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
> index a111351516..a2c157d8a7 100644
> ---
> a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
> +++
> b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.c
> @@ -47,7 +47,7 @@ Tcg2PhysicalPresenceLibGetManagementFlags (
>                            &PpiFlags
> 
>                            );
> 
>    if (EFI_ERROR (Status)) {
> 
> -    PpiFlags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
> 
> +    PpiFlags.PPFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
> 
>    }
> 
>    return PpiFlags.PPFlags;
> 
>  }
> 
> diff --git
> a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.i
> nf
> b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.i
> nf
> index d34f232022..11ebaa9263 100644
> ---
> a/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.i
> nf
> +++
> b/SecurityPkg/Library/PeiTcg2PhysicalPresenceLib/PeiTcg2PhysicalPresenceLib.i
> nf
> @@ -43,5 +43,8 @@
>  [Ppis]
> 
>    gEfiPeiReadOnlyVariable2PpiGuid       ## CONSUMES
> 
> 
> 
> +[Pcd]
> 
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags       ##
> SOMETIMES_CONSUMES
> 
> +
> 
>  [Depex]
> 
>    gEfiPeiReadOnlyVariable2PpiGuid
> 
> diff --git
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.c
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.c
> index 3827df9663..08ef5416d8 100644
> ---
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.c
> +++
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.c
> @@ -31,6 +31,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
>  EFI_SMM_VARIABLE_PROTOCOL  *mTcg2PpSmmVariable;
> 
>  BOOLEAN                    mIsTcg2PPVerLowerThan_1_3 = FALSE;
> 
> +UINT32                     mTcg2PhysicalPresenceFlags;
> 
> 
> 
>  /**
> 
>    The handler for TPM physical presence function:
> 
> @@ -162,7 +163,7 @@
> Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx (
>                                     &Flags
> 
>                                     );
> 
>      if (EFI_ERROR (Status)) {
> 
> -      Flags.PPFlags = TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT;
> 
> +      Flags.PPFlags = mTcg2PhysicalPresenceFlags;
> 
>      }
> 
>      ReturnCode = Tcg2PpVendorLibSubmitRequestToPreOSFunction
> (*OperationRequest, Flags.PPFlags, *RequestParameter);
> 
>    }
> 
> @@ -396,5 +397,7 @@ Tcg2PhysicalPresenceLibConstructor (
>    Status = gSmst->SmmLocateProtocol (&gEfiSmmVariableProtocolGuid, NULL,
> (VOID**)&mTcg2PpSmmVariable);
> 
>    ASSERT_EFI_ERROR (Status);
> 
> 
> 
> +  mTcg2PhysicalPresenceFlags = PcdGet32(PcdTcg2PhysicalPresenceFlags);
> 
> +
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> diff --git
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.inf
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.inf
> index e0e5fef5f1..1f40629e3b 100644
> ---
> a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.inf
> +++
> b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresence
> Lib.inf
> @@ -50,6 +50,7 @@
> 
> 
>  [Pcd]
> 
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ##
> CONSUMES
> 
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcg2PhysicalPresenceFlags        ##
> SOMETIMES_CONSUMES
> 
> 
> 
>  [Depex]
> 
>    gEfiSmmVariableProtocolGuid
> 
> --
> 2.16.2.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59710): https://edk2.groups.io/g/devel/message/59710
Mute This Topic: https://groups.io/mt/74223343/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-