[edk2-devel] [PATCH v2 1/6] SecurityPkg/TPM: measure UEFI images without associated device paths again

Guomin Jiang posted 6 patches 5 years, 9 months ago
[edk2-devel] [PATCH v2 1/6] SecurityPkg/TPM: measure UEFI images without associated device paths again
Posted by Guomin Jiang 5 years, 9 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2652

DxeTpm2MeasureBootHandler() and DxeTpmMeasureBootHandler() functions may
receive a FileBuffer argument that is not associated with any particular
device path (e.g., because the UEFI image has not been loaded from any
particular device path).
Therefore rejecting (File==NULL) at the top of the function is invalid.

Fixes: 4b026f0d5af36faf3a3629a3ad49c51b5b3be12f

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: Guomin Jiang <guomin.jiang@intel.com>
---
 .../DxeTpm2MeasureBootLib.c                   | 20 +++++++++----------
 .../DxeTpmMeasureBootLib.c                    | 20 +++++++++----------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
index f0e95e5ec0..92eac71580 100644
--- a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
@@ -384,8 +384,6 @@ Finish:
   and other exception operations.  The File parameter allows for possible logging
   within the SAP of the driver.
 
-  If File is NULL, then EFI_ACCESS_DENIED is returned.
-
   If the file specified by File with an authentication status specified by
   AuthenticationStatus is safe for the DXE Core to use, then EFI_SUCCESS is returned.
 
@@ -398,6 +396,8 @@ Finish:
   might be possible to use it at a future time, then EFI_SECURITY_VIOLATION is
   returned.
 
+  If check image specified by FileBuffer and File is NULL meanwhile, return EFI_ACCESS_DENIED.
+
   @param[in]      AuthenticationStatus  This is the authentication status returned
                                         from the securitymeasurement services for the
                                         input file.
@@ -416,7 +416,7 @@ EFI_STATUS
 EFIAPI
 DxeTpm2MeasureBootHandler (
   IN  UINT32                           AuthenticationStatus,
-  IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File,
+  IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File, OPTIONAL
   IN  VOID                             *FileBuffer,
   IN  UINTN                            FileSize,
   IN  BOOLEAN                          BootPolicy
@@ -435,13 +435,6 @@ DxeTpm2MeasureBootHandler (
   EFI_PHYSICAL_ADDRESS                FvAddress;
   UINT32                              Index;
 
-  //
-  // Check for invalid parameters.
-  //
-  if (File == NULL) {
-    return EFI_ACCESS_DENIED;
-  }
-
   Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
   if (EFI_ERROR (Status)) {
     //
@@ -615,6 +608,13 @@ DxeTpm2MeasureBootHandler (
   //
   Status = PeCoffLoaderGetImageInfo (&ImageContext);
   if (EFI_ERROR (Status)) {
+    //
+    // Check for invalid parameters.
+    //
+    if (File == NULL) {
+      Status = EFI_ACCESS_DENIED;
+    }
+
     //
     // The information can't be got from the invalid PeImage
     //
diff --git a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
index d499371e7a..d990eb2ad3 100644
--- a/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c
@@ -678,8 +678,6 @@ Finish:
   and other exception operations.  The File parameter allows for possible logging
   within the SAP of the driver.
 
-  If File is NULL, then EFI_ACCESS_DENIED is returned.
-
   If the file specified by File with an authentication status specified by
   AuthenticationStatus is safe for the DXE Core to use, then EFI_SUCCESS is returned.
 
@@ -692,6 +690,8 @@ Finish:
   might be possible to use it at a future time, then EFI_SECURITY_VIOLATION is
   returned.
 
+  If check image specified by FileBuffer and File is NULL meanwhile, return EFI_ACCESS_DENIED.
+
   @param[in]      AuthenticationStatus  This is the authentication status returned
                                         from the securitymeasurement services for the
                                         input file.
@@ -710,7 +710,7 @@ EFI_STATUS
 EFIAPI
 DxeTpmMeasureBootHandler (
   IN  UINT32                           AuthenticationStatus,
-  IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File,
+  IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File, OPTIONAL
   IN  VOID                             *FileBuffer,
   IN  UINTN                            FileSize,
   IN  BOOLEAN                          BootPolicy
@@ -732,13 +732,6 @@ DxeTpmMeasureBootHandler (
   EFI_PHYSICAL_ADDRESS                FvAddress;
   UINT32                              Index;
 
-  //
-  // Check for invalid parameters.
-  //
-  if (File == NULL) {
-    return EFI_ACCESS_DENIED;
-  }
-
   Status = gBS->LocateProtocol (&gEfiTcgProtocolGuid, NULL, (VOID **) &TcgProtocol);
   if (EFI_ERROR (Status)) {
     //
@@ -912,6 +905,13 @@ DxeTpmMeasureBootHandler (
   //
   Status = PeCoffLoaderGetImageInfo (&ImageContext);
   if (EFI_ERROR (Status)) {
+    //
+    // Check for invalid parameters.
+    //
+    if (File == NULL) {
+      return EFI_ACCESS_DENIED;
+    }
+
     //
     // The information can't be got from the invalid PeImage
     //
-- 
2.25.1.windows.1


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

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