public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/2] Add and use FirmwareVolumeShadowPpi
@ 2023-03-22  2:06 Michael D Kinney
  2023-03-22  2:06 ` [Patch 1/2] MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi Michael D Kinney
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Michael D Kinney @ 2023-03-22  2:06 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Liming Gao, Patel Umang

Add FirmwareVolumeShadow PPI to shadow an FV to memory.
and use it to shadow FVs to memory.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Patel Umang <umang.patel@intel.com>

Umang Patel (2):
  MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
  SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi

 .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                 |  3 +
 SecurityPkg/FvReportPei/FvReportPei.c         | 37 ++++++++---
 SecurityPkg/FvReportPei/FvReportPei.h         |  1 +
 SecurityPkg/FvReportPei/FvReportPei.inf       |  1 +
 5 files changed, 96 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h

-- 
2.39.1.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Patch 1/2] MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
  2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
@ 2023-03-22  2:06 ` Michael D Kinney
  2023-03-22  2:06 ` [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi Michael D Kinney
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Michael D Kinney @ 2023-03-22  2:06 UTC (permalink / raw)
  To: devel; +Cc: Umang Patel, Jian J Wang, Liming Gao

From: Umang Patel <umang.patel@intel.com>

Add FirmwareVolumeShadow PPI to shadow an FV to memory.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Patel Umang <umang.patel@intel.com>
---
 .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                 |  3 +
 2 files changed, 64 insertions(+)
 create mode 100644 MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h

diff --git a/MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h b/MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
new file mode 100644
index 000000000000..a2756cb0ab1e
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
@@ -0,0 +1,61 @@
+/** @file
+  Define PPI to shadow Firmware Volume from flash to Permanent Memory.
+
+Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef PEI_FIRMWARE_VOLUME_SHADOW_PPI_H_
+#define PEI_FIRMWARE_VOLUME_SHADOW_PPI_H_
+
+//
+// Firmware Volume Shadow PPI GUID value
+//
+#define EDKII_FIRMWARE_VOLUME_SHADOW_PPI_GUID \
+  { \
+    0x7dfe756c, 0xed8d, 0x4d77, { 0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } \
+  }
+
+/**
+  Copy FV to Destination.  Length of copy is FV length from FV Header.
+
+  @param[in]  FirmwareVolumeBase  Base address of FV to shadow. Length of FV
+                                  is in FV Header.
+  @param[in]  Destination         Pointer to the Buffer in system memory to
+                                  shadow FV.
+  @param[in]  DestinationLength   Size of Destination buffer in bytes.
+
+  @retval EFI_SUCCESS            Shadow complete
+  @retval EFI_INVALID_PARAMETER  Destination is NULL
+  @retval EFI_INVALID_PARAMETER  DestinationLength = 0.
+  @retval EFI_INVALID_PARAMETER  FV does not have valid FV Header.
+  @retval EFI_INVALID_PARAMETER  FV overlaps Destination.
+  @retval EFI_INVALID_PARAMETER  Destination + DestinationLength rolls over 4GB
+                                 for 32-bit or 64-bit rollover.
+  @retval EFI_BUFFER_TOO_SMALL   DestinationLength less than FV length from FV
+                                 Header.
+  @retval EFI_UNSUPPORTED        FirmwareVolumeBase to FVBase + FVLength does
+                                 not support shadow.  Caller should fallback to
+                                 CopyMem().
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PEI_FIRMWARE_VOLUME_SHADOW)(
+  IN EFI_PHYSICAL_ADDRESS  FirmwareVolumeBase,
+  IN VOID                  *Destination,
+  IN UINTN                 DestinationLength
+  );
+
+///
+/// This PPI provides a service to shadow a FV from one location to another
+///
+typedef struct {
+  EDKII_PEI_FIRMWARE_VOLUME_SHADOW    FirmwareVolumeShadow;
+} EDKII_PEI_FIRMWARE_VOLUME_SHADOW_PPI;
+
+extern EFI_GUID  gEdkiiPeiFirmwareVolumeShadowPpiGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index e8058c8bfaec..3eb4a79bf7a0 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -442,6 +442,9 @@ [Guids]
   gBootDiscoveryPolicyMgrFormsetGuid = { 0x5b6f7107, 0xbb3c, 0x4660, { 0x92, 0xcd, 0x54, 0x26, 0x90, 0x28, 0x0b, 0xbd } }
 
 [Ppis]
+  ## Include/Ppi/FirmwareVolumeShadowPpi.h
+  gEdkiiPeiFirmwareVolumeShadowPpiGuid = { 0x7dfe756c, 0xed8d, 0x4d77, {0x9e, 0xc4, 0x39, 0x9a, 0x8a, 0x81, 0x51, 0x16 } }
+
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid       = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
 
-- 
2.39.1.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
  2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
  2023-03-22  2:06 ` [Patch 1/2] MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi Michael D Kinney
@ 2023-03-22  2:06 ` Michael D Kinney
  2023-03-22 16:04   ` [edk2-devel] " Oliver Smith-Denny
  2023-03-22 16:05 ` [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi Oliver Smith-Denny
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Michael D Kinney @ 2023-03-22  2:06 UTC (permalink / raw)
  To: devel; +Cc: Umang Patel, Jiewen Yao, Jian J Wang

From: Umang Patel <umang.patel@intel.com>

If FirmwareVolumeShadow PPI is available, then use it to
shadow FVs to memory.  Otherwise fallback to CopyMem().

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Patel Umang <umang.patel@intel.com>
---
 SecurityPkg/FvReportPei/FvReportPei.c   | 37 ++++++++++++++++++++-----
 SecurityPkg/FvReportPei/FvReportPei.h   |  1 +
 SecurityPkg/FvReportPei/FvReportPei.inf |  1 +
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/SecurityPkg/FvReportPei/FvReportPei.c b/SecurityPkg/FvReportPei/FvReportPei.c
index 846605cda1e4..6288dde16b2a 100644
--- a/SecurityPkg/FvReportPei/FvReportPei.c
+++ b/SecurityPkg/FvReportPei/FvReportPei.c
@@ -114,12 +114,13 @@ VerifyHashedFv (
   IN EFI_BOOT_MODE   BootMode
   )
 {
-  UINTN                FvIndex;
-  CONST HASH_ALG_INFO  *AlgInfo;
-  UINT8                *HashValue;
-  UINT8                *FvHashValue;
-  VOID                 *FvBuffer;
-  EFI_STATUS           Status;
+  UINTN                                 FvIndex;
+  CONST HASH_ALG_INFO                   *AlgInfo;
+  UINT8                                 *HashValue;
+  UINT8                                 *FvHashValue;
+  VOID                                  *FvBuffer;
+  EDKII_PEI_FIRMWARE_VOLUME_SHADOW_PPI  *FvShadowPpi;
+  EFI_STATUS                            Status;
 
   if ((HashInfo == NULL) ||
       (HashInfo->HashSize == 0) ||
@@ -191,8 +192,30 @@ VerifyHashedFv (
     // Copy FV to permanent memory to avoid potential TOC/TOU.
     //
     FvBuffer = AllocatePages (EFI_SIZE_TO_PAGES ((UINTN)FvInfo[FvIndex].Length));
+
     ASSERT (FvBuffer != NULL);
-    CopyMem (FvBuffer, (CONST VOID *)(UINTN)FvInfo[FvIndex].Base, (UINTN)FvInfo[FvIndex].Length);
+    Status = PeiServicesLocatePpi (
+               &gEdkiiPeiFirmwareVolumeShadowPpiGuid,
+               0,
+               NULL,
+               (VOID **)&FvShadowPpi
+               );
+
+    if (!EFI_ERROR (Status)) {
+      Status = FvShadowPpi->FirmwareVolumeShadow (
+                              (EFI_PHYSICAL_ADDRESS)FvInfo[FvIndex].Base,
+                              FvBuffer,
+                              (UINTN)FvInfo[FvIndex].Length
+                              );
+    }
+
+    if (EFI_ERROR (Status)) {
+      CopyMem (
+        FvBuffer,
+        (CONST VOID *)(UINTN)FvInfo[FvIndex].Base,
+        (UINTN)FvInfo[FvIndex].Length
+        );
+    }
 
     if (!AlgInfo->HashAll (FvBuffer, (UINTN)FvInfo[FvIndex].Length, FvHashValue)) {
       Status = EFI_ABORTED;
diff --git a/SecurityPkg/FvReportPei/FvReportPei.h b/SecurityPkg/FvReportPei/FvReportPei.h
index 92504a3c51e1..07ffb2f5768c 100644
--- a/SecurityPkg/FvReportPei/FvReportPei.h
+++ b/SecurityPkg/FvReportPei/FvReportPei.h
@@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <IndustryStandard/Tpm20.h>
 
 #include <Ppi/FirmwareVolumeInfoStoredHashFv.h>
+#include <Ppi/FirmwareVolumeShadowPpi.h>
 
 #include <Library/PeiServicesLib.h>
 #include <Library/PcdLib.h>
diff --git a/SecurityPkg/FvReportPei/FvReportPei.inf b/SecurityPkg/FvReportPei/FvReportPei.inf
index 408406889765..4246fb75ebaa 100644
--- a/SecurityPkg/FvReportPei/FvReportPei.inf
+++ b/SecurityPkg/FvReportPei/FvReportPei.inf
@@ -46,6 +46,7 @@ [LibraryClasses]
 [Ppis]
   gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid   ## PRODUCES
   gEdkiiPeiFirmwareVolumeInfoStoredHashFvPpiGuid  ## CONSUMES
+  gEdkiiPeiFirmwareVolumeShadowPpiGuid            ## CONSUMES
 
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass
-- 
2.39.1.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
  2023-03-22  2:06 ` [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi Michael D Kinney
@ 2023-03-22 16:04   ` Oliver Smith-Denny
  2023-03-27 15:19     ` Michael D Kinney
  0 siblings, 1 reply; 9+ messages in thread
From: Oliver Smith-Denny @ 2023-03-22 16:04 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Umang Patel, Jiewen Yao, Jian J Wang

One comment below, thanks!

On 3/21/2023 7:06 PM, Michael D Kinney wrote:
> From: Umang Patel <umang.patel@intel.com>
> 
> If FirmwareVolumeShadow PPI is available, then use it to
> shadow FVs to memory.  Otherwise fallback to CopyMem().
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Signed-off-by: Patel Umang <umang.patel@intel.com>
> ---
>   SecurityPkg/FvReportPei/FvReportPei.c   | 37 ++++++++++++++++++++-----
>   SecurityPkg/FvReportPei/FvReportPei.h   |  1 +
>   SecurityPkg/FvReportPei/FvReportPei.inf |  1 +
>   3 files changed, 32 insertions(+), 7 deletions(-)
> 
> diff --git a/SecurityPkg/FvReportPei/FvReportPei.c b/SecurityPkg/FvReportPei/FvReportPei.c
> index 846605cda1e4..6288dde16b2a 100644
> --- a/SecurityPkg/FvReportPei/FvReportPei.c
> +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> @@ -114,12 +114,13 @@ VerifyHashedFv (
>     IN EFI_BOOT_MODE   BootMode
>     )
>   {
> -  UINTN                FvIndex;
> -  CONST HASH_ALG_INFO  *AlgInfo;
> -  UINT8                *HashValue;
> -  UINT8                *FvHashValue;
> -  VOID                 *FvBuffer;
> -  EFI_STATUS           Status;
> +  UINTN                                 FvIndex;
> +  CONST HASH_ALG_INFO                   *AlgInfo;
> +  UINT8                                 *HashValue;
> +  UINT8                                 *FvHashValue;
> +  VOID                                  *FvBuffer;
> +  EDKII_PEI_FIRMWARE_VOLUME_SHADOW_PPI  *FvShadowPpi;
> +  EFI_STATUS                            Status;
>   
>     if ((HashInfo == NULL) ||
>         (HashInfo->HashSize == 0) ||
> @@ -191,8 +192,30 @@ VerifyHashedFv (
>       // Copy FV to permanent memory to avoid potential TOC/TOU.
>       //
>       FvBuffer = AllocatePages (EFI_SIZE_TO_PAGES ((UINTN)FvInfo[FvIndex].Length));
> +
>       ASSERT (FvBuffer != NULL);

While we are here, should we make this more robust (and amenable to 
static analysis) and add error handling if FvBuffer is NULL, not just 
assert?

> -    CopyMem (FvBuffer, (CONST VOID *)(UINTN)FvInfo[FvIndex].Base, (UINTN)FvInfo[FvIndex].Length);
> +    Status = PeiServicesLocatePpi (
> +               &gEdkiiPeiFirmwareVolumeShadowPpiGuid,
> +               0,
> +               NULL,
> +               (VOID **)&FvShadowPpi
> +               );
> +
> +    if (!EFI_ERROR (Status)) {
> +      Status = FvShadowPpi->FirmwareVolumeShadow (
> +                              (EFI_PHYSICAL_ADDRESS)FvInfo[FvIndex].Base,
> +                              FvBuffer,
> +                              (UINTN)FvInfo[FvIndex].Length
> +                              );
> +    }
> +
> +    if (EFI_ERROR (Status)) {
> +      CopyMem (
> +        FvBuffer,
> +        (CONST VOID *)(UINTN)FvInfo[FvIndex].Base,
> +        (UINTN)FvInfo[FvIndex].Length
> +        );
> +    }
>   
>       if (!AlgInfo->HashAll (FvBuffer, (UINTN)FvInfo[FvIndex].Length, FvHashValue)) {
>         Status = EFI_ABORTED;
> diff --git a/SecurityPkg/FvReportPei/FvReportPei.h b/SecurityPkg/FvReportPei/FvReportPei.h
> index 92504a3c51e1..07ffb2f5768c 100644
> --- a/SecurityPkg/FvReportPei/FvReportPei.h
> +++ b/SecurityPkg/FvReportPei/FvReportPei.h
> @@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>   #include <IndustryStandard/Tpm20.h>
>   
>   #include <Ppi/FirmwareVolumeInfoStoredHashFv.h>
> +#include <Ppi/FirmwareVolumeShadowPpi.h>
>   
>   #include <Library/PeiServicesLib.h>
>   #include <Library/PcdLib.h>
> diff --git a/SecurityPkg/FvReportPei/FvReportPei.inf b/SecurityPkg/FvReportPei/FvReportPei.inf
> index 408406889765..4246fb75ebaa 100644
> --- a/SecurityPkg/FvReportPei/FvReportPei.inf
> +++ b/SecurityPkg/FvReportPei/FvReportPei.inf
> @@ -46,6 +46,7 @@ [LibraryClasses]
>   [Ppis]
>     gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid   ## PRODUCES
>     gEdkiiPeiFirmwareVolumeInfoStoredHashFvPpiGuid  ## CONSUMES
> +  gEdkiiPeiFirmwareVolumeShadowPpiGuid            ## CONSUMES
>   
>   [Pcd]
>     gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi
  2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
  2023-03-22  2:06 ` [Patch 1/2] MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi Michael D Kinney
  2023-03-22  2:06 ` [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi Michael D Kinney
@ 2023-03-22 16:05 ` Oliver Smith-Denny
  2023-03-23 19:17 ` Michael D Kinney
  2023-03-27  4:55 ` Wang, Jian J
  4 siblings, 0 replies; 9+ messages in thread
From: Oliver Smith-Denny @ 2023-03-22 16:05 UTC (permalink / raw)
  To: devel, michael.d.kinney; +Cc: Jiewen Yao, Jian J Wang, Liming Gao, Patel Umang

For the patchset:

Reviewed-by: Oliver Smith-Denny <osd@smith-denny.com>

Thanks!

On 3/21/2023 7:06 PM, Michael D Kinney wrote:
> Add FirmwareVolumeShadow PPI to shadow an FV to memory.
> and use it to shadow FVs to memory.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Patel Umang <umang.patel@intel.com>
> 
> Umang Patel (2):
>    MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
>    SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
> 
>   .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
>   MdeModulePkg/MdeModulePkg.dec                 |  3 +
>   SecurityPkg/FvReportPei/FvReportPei.c         | 37 ++++++++---
>   SecurityPkg/FvReportPei/FvReportPei.h         |  1 +
>   SecurityPkg/FvReportPei/FvReportPei.inf       |  1 +
>   5 files changed, 96 insertions(+), 7 deletions(-)
>   create mode 100644 MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi
  2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
                   ` (2 preceding siblings ...)
  2023-03-22 16:05 ` [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi Oliver Smith-Denny
@ 2023-03-23 19:17 ` Michael D Kinney
  2023-03-27  4:55 ` Wang, Jian J
  4 siblings, 0 replies; 9+ messages in thread
From: Michael D Kinney @ 2023-03-23 19:17 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D
  Cc: Yao, Jiewen, Wang, Jian J, Gao, Liming, Patel, Umang,
	Kinney, Michael D

Series Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D Kinney
> Sent: Tuesday, March 21, 2023 7:06 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Patel, Umang
> <umang.patel@intel.com>
> Subject: [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi
> 
> Add FirmwareVolumeShadow PPI to shadow an FV to memory.
> and use it to shadow FVs to memory.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Patel Umang <umang.patel@intel.com>
> 
> Umang Patel (2):
>   MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
>   SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
> 
>  .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec                 |  3 +
>  SecurityPkg/FvReportPei/FvReportPei.c         | 37 ++++++++---
>  SecurityPkg/FvReportPei/FvReportPei.h         |  1 +
>  SecurityPkg/FvReportPei/FvReportPei.inf       |  1 +
>  5 files changed, 96 insertions(+), 7 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
> 
> --
> 2.39.1.windows.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch 0/2] Add and use FirmwareVolumeShadowPpi
  2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
                   ` (3 preceding siblings ...)
  2023-03-23 19:17 ` Michael D Kinney
@ 2023-03-27  4:55 ` Wang, Jian J
  2023-03-27 16:26   ` Michael D Kinney
  4 siblings, 1 reply; 9+ messages in thread
From: Wang, Jian J @ 2023-03-27  4:55 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io
  Cc: Yao, Jiewen, Gao, Liming, Patel, Umang

For the patch series,

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian



> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, March 22, 2023 10:06 AM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Patel, Umang
> <umang.patel@intel.com>
> Subject: [Patch 0/2] Add and use FirmwareVolumeShadowPpi
> 
> Add FirmwareVolumeShadow PPI to shadow an FV to memory.
> and use it to shadow FVs to memory.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Patel Umang <umang.patel@intel.com>
> 
> Umang Patel (2):
>   MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
>   SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
> 
>  .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec                 |  3 +
>  SecurityPkg/FvReportPei/FvReportPei.c         | 37 ++++++++---
>  SecurityPkg/FvReportPei/FvReportPei.h         |  1 +
>  SecurityPkg/FvReportPei/FvReportPei.inf       |  1 +
>  5 files changed, 96 insertions(+), 7 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
> 
> --
> 2.39.1.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
  2023-03-22 16:04   ` [edk2-devel] " Oliver Smith-Denny
@ 2023-03-27 15:19     ` Michael D Kinney
  0 siblings, 0 replies; 9+ messages in thread
From: Michael D Kinney @ 2023-03-27 15:19 UTC (permalink / raw)
  To: Oliver Smith-Denny, devel@edk2.groups.io
  Cc: Patel, Umang, Yao, Jiewen, Wang, Jian J, Kinney, Michael D

I agree with your comment.  Can you please enter a new BZ to address it?

Thanks,

Mike

> -----Original Message-----
> From: Oliver Smith-Denny <osd@smith-denny.com>
> Sent: Wednesday, March 22, 2023 9:05 AM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Patel, Umang <umang.patel@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
> Subject: Re: [edk2-devel] [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
> 
> One comment below, thanks!
> 
> On 3/21/2023 7:06 PM, Michael D Kinney wrote:
> > From: Umang Patel <umang.patel@intel.com>
> >
> > If FirmwareVolumeShadow PPI is available, then use it to
> > shadow FVs to memory.  Otherwise fallback to CopyMem().
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Signed-off-by: Patel Umang <umang.patel@intel.com>
> > ---
> >   SecurityPkg/FvReportPei/FvReportPei.c   | 37 ++++++++++++++++++++-----
> >   SecurityPkg/FvReportPei/FvReportPei.h   |  1 +
> >   SecurityPkg/FvReportPei/FvReportPei.inf |  1 +
> >   3 files changed, 32 insertions(+), 7 deletions(-)
> >
> > diff --git a/SecurityPkg/FvReportPei/FvReportPei.c b/SecurityPkg/FvReportPei/FvReportPei.c
> > index 846605cda1e4..6288dde16b2a 100644
> > --- a/SecurityPkg/FvReportPei/FvReportPei.c
> > +++ b/SecurityPkg/FvReportPei/FvReportPei.c
> > @@ -114,12 +114,13 @@ VerifyHashedFv (
> >     IN EFI_BOOT_MODE   BootMode
> >     )
> >   {
> > -  UINTN                FvIndex;
> > -  CONST HASH_ALG_INFO  *AlgInfo;
> > -  UINT8                *HashValue;
> > -  UINT8                *FvHashValue;
> > -  VOID                 *FvBuffer;
> > -  EFI_STATUS           Status;
> > +  UINTN                                 FvIndex;
> > +  CONST HASH_ALG_INFO                   *AlgInfo;
> > +  UINT8                                 *HashValue;
> > +  UINT8                                 *FvHashValue;
> > +  VOID                                  *FvBuffer;
> > +  EDKII_PEI_FIRMWARE_VOLUME_SHADOW_PPI  *FvShadowPpi;
> > +  EFI_STATUS                            Status;
> >
> >     if ((HashInfo == NULL) ||
> >         (HashInfo->HashSize == 0) ||
> > @@ -191,8 +192,30 @@ VerifyHashedFv (
> >       // Copy FV to permanent memory to avoid potential TOC/TOU.
> >       //
> >       FvBuffer = AllocatePages (EFI_SIZE_TO_PAGES ((UINTN)FvInfo[FvIndex].Length));
> > +
> >       ASSERT (FvBuffer != NULL);
> 
> While we are here, should we make this more robust (and amenable to
> static analysis) and add error handling if FvBuffer is NULL, not just
> assert?
> 
> > -    CopyMem (FvBuffer, (CONST VOID *)(UINTN)FvInfo[FvIndex].Base, (UINTN)FvInfo[FvIndex].Length);
> > +    Status = PeiServicesLocatePpi (
> > +               &gEdkiiPeiFirmwareVolumeShadowPpiGuid,
> > +               0,
> > +               NULL,
> > +               (VOID **)&FvShadowPpi
> > +               );
> > +
> > +    if (!EFI_ERROR (Status)) {
> > +      Status = FvShadowPpi->FirmwareVolumeShadow (
> > +                              (EFI_PHYSICAL_ADDRESS)FvInfo[FvIndex].Base,
> > +                              FvBuffer,
> > +                              (UINTN)FvInfo[FvIndex].Length
> > +                              );
> > +    }
> > +
> > +    if (EFI_ERROR (Status)) {
> > +      CopyMem (
> > +        FvBuffer,
> > +        (CONST VOID *)(UINTN)FvInfo[FvIndex].Base,
> > +        (UINTN)FvInfo[FvIndex].Length
> > +        );
> > +    }
> >
> >       if (!AlgInfo->HashAll (FvBuffer, (UINTN)FvInfo[FvIndex].Length, FvHashValue)) {
> >         Status = EFI_ABORTED;
> > diff --git a/SecurityPkg/FvReportPei/FvReportPei.h b/SecurityPkg/FvReportPei/FvReportPei.h
> > index 92504a3c51e1..07ffb2f5768c 100644
> > --- a/SecurityPkg/FvReportPei/FvReportPei.h
> > +++ b/SecurityPkg/FvReportPei/FvReportPei.h
> > @@ -14,6 +14,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> >   #include <IndustryStandard/Tpm20.h>
> >
> >   #include <Ppi/FirmwareVolumeInfoStoredHashFv.h>
> > +#include <Ppi/FirmwareVolumeShadowPpi.h>
> >
> >   #include <Library/PeiServicesLib.h>
> >   #include <Library/PcdLib.h>
> > diff --git a/SecurityPkg/FvReportPei/FvReportPei.inf b/SecurityPkg/FvReportPei/FvReportPei.inf
> > index 408406889765..4246fb75ebaa 100644
> > --- a/SecurityPkg/FvReportPei/FvReportPei.inf
> > +++ b/SecurityPkg/FvReportPei/FvReportPei.inf
> > @@ -46,6 +46,7 @@ [LibraryClasses]
> >   [Ppis]
> >     gEdkiiPeiFirmwareVolumeInfoPrehashedFvPpiGuid   ## PRODUCES
> >     gEdkiiPeiFirmwareVolumeInfoStoredHashFvPpiGuid  ## CONSUMES
> > +  gEdkiiPeiFirmwareVolumeShadowPpiGuid            ## CONSUMES
> >
> >   [Pcd]
> >     gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeFvVerificationPass

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Patch 0/2] Add and use FirmwareVolumeShadowPpi
  2023-03-27  4:55 ` Wang, Jian J
@ 2023-03-27 16:26   ` Michael D Kinney
  0 siblings, 0 replies; 9+ messages in thread
From: Michael D Kinney @ 2023-03-27 16:26 UTC (permalink / raw)
  To: Wang, Jian J, devel@edk2.groups.io
  Cc: Yao, Jiewen, Gao, Liming, Patel, Umang, Kinney, Michael D

Merged

PR: https://github.com/tianocore/edk2/pull/4159
Commit:	https://github.com/tianocore/edk2/commit/c8e631588b9591489d0219db1d14664e10367ecd
Commit: https://github.com/tianocore/edk2/commit/144028626e0072c2c4fdfcc0fe1b72de319bdd2f

Mike


> -----Original Message-----
> From: Wang, Jian J <jian.j.wang@intel.com>
> Sent: Sunday, March 26, 2023 9:55 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Patel, Umang <umang.patel@intel.com>
> Subject: RE: [Patch 0/2] Add and use FirmwareVolumeShadowPpi
> 
> For the patch series,
> 
> Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
> 
> Regards,
> Jian
> 
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, March 22, 2023 10:06 AM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Gao, Liming <gaoliming@byosoft.com.cn>; Patel, Umang
> > <umang.patel@intel.com>
> > Subject: [Patch 0/2] Add and use FirmwareVolumeShadowPpi
> >
> > Add FirmwareVolumeShadow PPI to shadow an FV to memory.
> > and use it to shadow FVs to memory.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > Signed-off-by: Patel Umang <umang.patel@intel.com>
> >
> > Umang Patel (2):
> >   MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi
> >   SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi
> >
> >  .../Include/Ppi/FirmwareVolumeShadowPpi.h     | 61 +++++++++++++++++++
> >  MdeModulePkg/MdeModulePkg.dec                 |  3 +
> >  SecurityPkg/FvReportPei/FvReportPei.c         | 37 ++++++++---
> >  SecurityPkg/FvReportPei/FvReportPei.h         |  1 +
> >  SecurityPkg/FvReportPei/FvReportPei.inf       |  1 +
> >  5 files changed, 96 insertions(+), 7 deletions(-)
> >  create mode 100644
> > MdeModulePkg/Include/Ppi/FirmwareVolumeShadowPpi.h
> >
> > --
> > 2.39.1.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-03-27 16:27 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-22  2:06 [Patch 0/2] Add and use FirmwareVolumeShadowPpi Michael D Kinney
2023-03-22  2:06 ` [Patch 1/2] MdeModulePkg/Include/Ppi: Add FirmwareVolumeShadowPpi Michael D Kinney
2023-03-22  2:06 ` [Patch 2/2] SecurityPkg/FvReportPei: Use FirmwareVolumeShadowPpi Michael D Kinney
2023-03-22 16:04   ` [edk2-devel] " Oliver Smith-Denny
2023-03-27 15:19     ` Michael D Kinney
2023-03-22 16:05 ` [edk2-devel] [Patch 0/2] Add and use FirmwareVolumeShadowPpi Oliver Smith-Denny
2023-03-23 19:17 ` Michael D Kinney
2023-03-27  4:55 ` Wang, Jian J
2023-03-27 16:26   ` Michael D Kinney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox