[edk2-devel] [PATCH v5 34/38] MdeModulePkg/DxeCore: Deal with failure in UefiProtectImage()

[Ard Biesheuvel]

In preparation for adding support for a more restrictive NX memory
policy, update the prototype of UefiProtectImage() so it returns a
EFI_STATUS, and deal with its failure in CoreLoadImage.

This should never fail for the DxeCore itself or for drivers that are
loaded before the CPU arch protocol is dispatched, so in those cases, an
ASSERT() is sufficient.

Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
---
 MdeModulePkg/Core/Dxe/DxeMain.h               |  6 +++++-
 MdeModulePkg/Core/Dxe/Image/Image.c           |  8 ++++++--
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 21 ++++++++++++--------
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 815a6b4bd844..b618feded39e 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -2733,8 +2733,12 @@ RemoveImageRecord (
 
   @param[in]  LoadedImage              The loaded image protocol
   @param[in]  LoadedImageDevicePath    The loaded image device path protocol
+
+  @return EFI_SUCCESS       Image protection was configured according to the
+                            applicable policy.
+  @return other             Image protection could not be applied.
 **/
-VOID
+EFI_STATUS
 ProtectUefiImage (
   IN EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage,
   IN EFI_DEVICE_PATH_PROTOCOL   *LoadedImageDevicePath
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 8704ebea9a7c..df2afbc299e3 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -270,7 +270,8 @@ CoreInitializeImageServices (
 
   InitializeListHead (&mAvailableEmulators);
 
-  ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+  Status = ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+  ASSERT_EFI_ERROR (Status);
 
   return Status;
 }
@@ -1448,7 +1449,10 @@ CoreLoadImageCommon (
     }
   }
 
-  ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+  Status = ProtectUefiImage (&Image->Info, Image->LoadedImageDevicePath);
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
 
   //
   // Success.  Return the image handle
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 045e2f391bc0..301ddd6eb053 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -341,8 +341,12 @@ FreeImageRecord (
 
   @param[in]  LoadedImage              The loaded image protocol
   @param[in]  LoadedImageDevicePath    The loaded image device path protocol
+
+  @return EFI_SUCCESS       Image protection was configured according to the
+                            applicable policy.
+  @return other             Image protection could not be applied.
 **/
-VOID
+EFI_STATUS
 ProtectUefiImage (
   IN EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage,
   IN EFI_DEVICE_PATH_PROTOCOL   *LoadedImageDevicePath
@@ -365,23 +369,23 @@ ProtectUefiImage (
   DEBUG ((DEBUG_INFO, "  - 0x%016lx - 0x%016lx\n", (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase, LoadedImage->ImageSize));
 
   if (gCpuSetMemoryAttributes == NULL) {
-    return;
+    return EFI_SUCCESS;
   }
 
   ProtectionPolicy = GetUefiImageProtectionPolicy (LoadedImage, LoadedImageDevicePath);
   switch (ProtectionPolicy) {
     case DO_NOT_PROTECT:
-      return;
+      return EFI_SUCCESS;
     case PROTECT_IF_ALIGNED_ELSE_ALLOW:
       break;
     default:
       ASSERT (FALSE);
-      return;
+      return EFI_SUCCESS;
   }
 
   ImageRecord = AllocateZeroPool (sizeof (*ImageRecord));
   if (ImageRecord == NULL) {
-    return;
+    return EFI_SUCCESS;
   }
 
   ImageRecord->Signature = IMAGE_PROPERTIES_RECORD_SIGNATURE;
@@ -481,7 +485,7 @@ ProtectUefiImage (
       //
       ImageRecordCodeSection = AllocatePool (sizeof (*ImageRecordCodeSection));
       if (ImageRecordCodeSection == NULL) {
-        return;
+        return EFI_SUCCESS;
       }
 
       ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
@@ -538,7 +542,7 @@ ProtectUefiImage (
   InsertTailList (&mProtectedImageRecordList, &ImageRecord->Link);
 
 Finish:
-  return;
+  return EFI_SUCCESS;
 }
 
 /**
@@ -988,7 +992,8 @@ MemoryProtectionCpuArchProtocolNotify (
       LoadedImageDevicePath = NULL;
     }
 
-    ProtectUefiImage (LoadedImage, LoadedImageDevicePath);
+    Status = ProtectUefiImage (LoadedImage, LoadedImageDevicePath);
+    ASSERT_EFI_ERROR (Status);
   }
 
   FreePool (HandleBuffer);
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101138): https://edk2.groups.io/g/devel/message/101138
Mute This Topic: https://groups.io/mt/97586055/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-