From 37b5981bf7eb94314b62810da495d724873d904a Mon Sep 17 00:00:00 2001
From: Laszlo Ersek <lersek@redhat.com>
Date: Fri, 31 Jan 2020 12:42:40 +0100
Subject: [PATCH 04/12] SecurityPkg/DxeImageVerificationHandler: keep PE/COFF
 info status internal
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RH-Author: Laszlo Ersek <lersek@redhat.com>
Message-id: <20200131124248.22369-5-lersek@redhat.com>
Patchwork-id: 93609
O-Subject: [RHEL-8.2.0 edk2 PATCH 04/12] SecurityPkg/DxeImageVerificationHandler: keep PE/COFF info status internal
Bugzilla: 1751993
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

The PeCoffLoaderGetImageInfo() function may return various error codes,
such as RETURN_INVALID_PARAMETER and RETURN_UNSUPPORTED.

Such error values should not be assigned to our "Status" variable in the
DxeImageVerificationHandler() function, because "Status" generally stands
for the main exit value of the function. And
SECURITY2_FILE_AUTHENTICATION_HANDLER functions are expected to return one
of EFI_SUCCESS, EFI_SECURITY_VIOLATION, and EFI_ACCESS_DENIED only.

Introduce the "PeCoffStatus" helper variable for keeping the return value
of PeCoffLoaderGetImageInfo() internal to the function. If
PeCoffLoaderGetImageInfo() fails, we'll jump to the "Done" label with
"Status" being EFI_ACCESS_DENIED, inherited from the top of the function.

Note that this is consistent with the subsequent PE/COFF Signature check,
where we jump to the "Done" label with "Status" having been re-set to
EFI_ACCESS_DENIED.

As a consequence, we can at once remove the

  Status = EFI_ACCESS_DENIED;

assignment right after the "PeCoffStatus" check.

This patch does not change the control flow in the function, it only
changes the "Status" outcome from API-incompatible error codes to
EFI_ACCESS_DENIED, under some circumstances.

Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200116190705.18816-4-lersek@redhat.com>
Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>
[lersek@redhat.com: push with Mike's R-b due to Chinese New Year
 Holiday: <https://edk2.groups.io/g/devel/message/53429>; msgid
 <d3fbb76dabed4e1987c512c328c82810@intel.com>]
(cherry picked from commit 61a9fa589a15e9005bec293f9766c78b60fbc9fc)

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
 .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c      | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 8204c9c..e6c8a54 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1580,6 +1580,7 @@ DxeImageVerificationHandler (
   EFI_IMAGE_DATA_DIRECTORY             *SecDataDir;
   UINT32                               OffSet;
   CHAR16                               *NameStr;
+  RETURN_STATUS                        PeCoffStatus;
 
   SignatureList     = NULL;
   SignatureListSize = 0;
@@ -1669,8 +1670,8 @@ DxeImageVerificationHandler (
   //
   // Get information about the image being loaded
   //
-  Status = PeCoffLoaderGetImageInfo (&ImageContext);
-  if (EFI_ERROR (Status)) {
+  PeCoffStatus = PeCoffLoaderGetImageInfo (&ImageContext);
+  if (RETURN_ERROR (PeCoffStatus)) {
     //
     // The information can't be got from the invalid PeImage
     //
@@ -1678,8 +1679,6 @@ DxeImageVerificationHandler (
     goto Done;
   }
 
-  Status = EFI_ACCESS_DENIED;
-
   DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase;
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
     //
-- 
1.8.3.1