public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Guomin Jiang" <guomin.jiang@intel.com>
To: devel@edk2.groups.io
Cc: Jiewen Yao <jiewen.yao@intel.com>,
	Jian J Wang <jian.j.wang@intel.com>,
	Chao Zhang <chao.b.zhang@intel.com>
Subject: [PATCH v2 1/6] SecurityPkg/TPM: measure UEFI images without associated device paths again
Date: Thu, 16 Apr 2020 15:33:49 +0800	[thread overview]
Message-ID: <20200416073354.2232-2-guomin.jiang@intel.com> (raw)
In-Reply-To: <20200416073354.2232-1-guomin.jiang@intel.com>

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


  reply	other threads:[~2020-04-16  7:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-16  7:33 [PATCH v2 0/6] Mark the File parameter as OPTIONAL Guomin Jiang
2020-04-16  7:33 ` Guomin Jiang [this message]
2020-04-16  7:33 ` [PATCH v2 2/6] SecurityPkg/DxeImageAuth: Mark the File parameter as option Guomin Jiang
2020-04-16  7:33 ` [PATCH v2 3/6] SecurityPkg/DxeImageVerificationLib: Mark the File parameter as OPTIONAL Guomin Jiang
2020-04-16  7:33 ` [PATCH v2 4/6] MdeModulePkg/SecurityManagementLib: " Guomin Jiang
2020-04-20 11:31   ` [edk2-devel] " Laszlo Ersek
2020-04-16  7:33 ` [PATCH v2 5/6] MdeModulePkg/SecurityStubDxe: " Guomin Jiang
2020-04-16  7:33 ` [PATCH v2 6/6] MdePkg/Security2: " Guomin Jiang
2020-04-20 11:27   ` [edk2-devel] " Laszlo Ersek
2020-04-21 14:21   ` Liming Gao
2020-04-20 11:39 ` [edk2-devel] [PATCH v2 0/6] " Laszlo Ersek
2020-04-21  2:15   ` Guomin Jiang
2020-04-21  8:52 ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200416073354.2232-2-guomin.jiang@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox