[edk2-devel] [PATCH 2/5] MdeModulePkg/SmmCore: Remove UEFI LOADED_IMAGE from SMM.

Zhiguang Liu zhiguang.liu at intel.com
Tue Jun 16 09:04:31 UTC 2020


REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2317

This patch removes EFI_LOADED_IMAGE_PROTOCOL from DXE database.
The SmmCore only install EFI_LOADED_IMAGE_PROTOCOL to SMM database.

Exposing LOADED_IMAGE_PROTOCOL may cause SMM information leakage
to non-SMM component.

Cc: Feng Tian <feng.tian at intel.com>
Cc: Star Zeng <star.zeng at intel.com>
Cc: Michael D Kinney <michael.d.kinney at intel.com>
Cc: Laszlo Ersek <lersek at redhat.com>
Signed-off-by: Jiewen Yao <jiewen.yao at intel.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu at intel.com>
---
 MdeModulePkg/Core/PiSmmCore/Dispatcher.c | 104 +++++++++++---------------------------------------------------------------------------------------------
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c  |  34 ----------------------------------
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h  |   4 ++--
 3 files changed, 13 insertions(+), 129 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
index 76ee9e0b89..2be2866234 100644
--- a/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
+++ b/MdeModulePkg/Core/PiSmmCore/Dispatcher.c
@@ -316,7 +316,7 @@ SmmLoadImage (
   EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
   PE_COFF_LOADER_IMAGE_CONTEXT   ImageContext;
 
-  PERF_LOAD_IMAGE_BEGIN (DriverEntry->ImageHandle);
+  PERF_LOAD_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
 
   Buffer               = NULL;
   Size                 = 0;
@@ -546,51 +546,14 @@ SmmLoadImage (
   DriverEntry->ImageBuffer      = DstBuffer;
   DriverEntry->NumberOfPage     = PageCount;
 
-  //
-  // Allocate a Loaded Image Protocol in EfiBootServicesData
-  //
-  Status = gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_LOADED_IMAGE_PROTOCOL), (VOID **)&DriverEntry->LoadedImage);
-  if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      gBS->FreePool (Buffer);
-    }
-    SmmFreePages (DstBuffer, PageCount);
-    return Status;
-  }
-
-  ZeroMem (DriverEntry->LoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));
   //
   // Fill in the remaining fields of the Loaded Image Protocol instance.
-  // Note: ImageBase is an SMRAM address that can not be accessed outside of SMRAM if SMRAM window is closed.
   //
-  DriverEntry->LoadedImage->Revision      = EFI_LOADED_IMAGE_PROTOCOL_REVISION;
-  DriverEntry->LoadedImage->ParentHandle  = gSmmCorePrivate->SmmIplImageHandle;
-  DriverEntry->LoadedImage->SystemTable   = gST;
-  DriverEntry->LoadedImage->DeviceHandle  = DeviceHandle;
-
   DriverEntry->SmmLoadedImage.Revision     = EFI_LOADED_IMAGE_PROTOCOL_REVISION;
   DriverEntry->SmmLoadedImage.ParentHandle = gSmmCorePrivate->SmmIplImageHandle;
   DriverEntry->SmmLoadedImage.SystemTable  = gST;
   DriverEntry->SmmLoadedImage.DeviceHandle = DeviceHandle;
 
-  //
-  // Make an EfiBootServicesData buffer copy of FilePath
-  //
-  Status = gBS->AllocatePool (EfiBootServicesData, GetDevicePathSize (FilePath), (VOID **)&DriverEntry->LoadedImage->FilePath);
-  if (EFI_ERROR (Status)) {
-    if (Buffer != NULL) {
-      gBS->FreePool (Buffer);
-    }
-    SmmFreePages (DstBuffer, PageCount);
-    return Status;
-  }
-  CopyMem (DriverEntry->LoadedImage->FilePath, FilePath, GetDevicePathSize (FilePath));
-
-  DriverEntry->LoadedImage->ImageBase     = (VOID *)(UINTN) ImageContext.ImageAddress;
-  DriverEntry->LoadedImage->ImageSize     = ImageContext.ImageSize;
-  DriverEntry->LoadedImage->ImageCodeType = EfiRuntimeServicesCode;
-  DriverEntry->LoadedImage->ImageDataType = EfiRuntimeServicesData;
-
   //
   // Make a buffer copy of FilePath
   //
@@ -599,7 +562,6 @@ SmmLoadImage (
     if (Buffer != NULL) {
       gBS->FreePool (Buffer);
     }
-    gBS->FreePool (DriverEntry->LoadedImage->FilePath);
     SmmFreePages (DstBuffer, PageCount);
     return Status;
   }
@@ -610,16 +572,6 @@ SmmLoadImage (
   DriverEntry->SmmLoadedImage.ImageCodeType = EfiRuntimeServicesCode;
   DriverEntry->SmmLoadedImage.ImageDataType = EfiRuntimeServicesData;
 
-  //
-  // Create a new image handle in the UEFI handle database for the SMM Driver
-  //
-  DriverEntry->ImageHandle = NULL;
-  Status = gBS->InstallMultipleProtocolInterfaces (
-                  &DriverEntry->ImageHandle,
-                  &gEfiLoadedImageProtocolGuid, DriverEntry->LoadedImage,
-                  NULL
-                  );
-
   //
   // Create a new image handle in the SMM handle database for the SMM Driver
   //
@@ -631,7 +583,7 @@ SmmLoadImage (
              &DriverEntry->SmmLoadedImage
              );
 
-  PERF_LOAD_IMAGE_END (DriverEntry->ImageHandle);
+  PERF_LOAD_IMAGE_END (DriverEntry->SmmImageHandle);
 
   //
   // Print the load address and the PDB file name if it is available
@@ -856,7 +808,7 @@ SmmDispatcher (
       // Untrused to Scheduled it would have already been loaded so we may need to
       // skip the LoadImage
       //
-      if (DriverEntry->ImageHandle == NULL) {
+      if (DriverEntry->SmmImageHandle == NULL) {
         Status = SmmLoadImage (DriverEntry);
 
         //
@@ -885,8 +837,8 @@ SmmDispatcher (
       REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
         EFI_PROGRESS_CODE,
         EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_BEGIN,
-        &DriverEntry->ImageHandle,
-        sizeof (DriverEntry->ImageHandle)
+        &DriverEntry->SmmImageHandle,
+        sizeof (DriverEntry->SmmImageHandle)
         );
 
       //
@@ -898,9 +850,10 @@ SmmDispatcher (
       // For each SMM driver, pass NULL as ImageHandle
       //
       RegisterSmramProfileImage (DriverEntry, TRUE);
-      PERF_START_IMAGE_BEGIN (DriverEntry->ImageHandle);
-      Status = ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoint)(DriverEntry->ImageHandle, gST);
-      PERF_START_IMAGE_END (DriverEntry->ImageHandle);
+      DEBUG ((DEBUG_INFO, "SMM StartImage - 0x%11p\n", DriverEntry->ImageEntryPoint));
+      PERF_START_IMAGE_BEGIN (DriverEntry->SmmImageHandle);
+      Status = ((EFI_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoint)(DriverEntry->SmmImageHandle, gST);
+      PERF_START_IMAGE_END (DriverEntry->SmmImageHandle);
       if (EFI_ERROR(Status)){
         DEBUG ((
           DEBUG_ERROR,
@@ -910,20 +863,6 @@ SmmDispatcher (
           ));
         UnregisterSmramProfileImage (DriverEntry, TRUE);
         SmmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
-        //
-        // Uninstall LoadedImage
-        //
-        Status = gBS->UninstallProtocolInterface (
-                        DriverEntry->ImageHandle,
-                        &gEfiLoadedImageProtocolGuid,
-                        DriverEntry->LoadedImage
-                        );
-        if (!EFI_ERROR (Status)) {
-          if (DriverEntry->LoadedImage->FilePath != NULL) {
-            gBS->FreePool (DriverEntry->LoadedImage->FilePath);
-          }
-          gBS->FreePool (DriverEntry->LoadedImage);
-        }
         Status = SmmUninstallProtocolInterface (
                    DriverEntry->SmmImageHandle,
                    &gEfiLoadedImageProtocolGuid,
@@ -939,8 +878,8 @@ SmmDispatcher (
       REPORT_STATUS_CODE_WITH_EXTENDED_DATA (
         EFI_PROGRESS_CODE,
         EFI_SOFTWARE_SMM_DRIVER | EFI_SW_PC_INIT_END,
-        &DriverEntry->ImageHandle,
-        sizeof (DriverEntry->ImageHandle)
+        &DriverEntry->SmmImageHandle,
+        sizeof (DriverEntry->SmmImageHandle)
         );
 
       if (!PreviousSmmEntryPointRegistered && gSmmCorePrivate->SmmEntryPointRegistered) {
@@ -1344,27 +1283,6 @@ SmmDriverDispatchHandler (
             //
             // If this is the SMM core fill in it's DevicePath & DeviceHandle
             //
-            if (mSmmCoreLoadedImage->FilePath == NULL) {
-              //
-              // Maybe one special FV contains only one SMM_CORE module, so its device path must
-              // be initialized completely.
-              //
-              EfiInitializeFwVolDevicepathNode (&mFvDevicePath.File, &NameGuid);
-              SetDevicePathEndNode (&mFvDevicePath.End);
-
-              //
-              // Make an EfiBootServicesData buffer copy of FilePath
-              //
-              Status = gBS->AllocatePool (
-                              EfiBootServicesData,
-                              GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL *)&mFvDevicePath),
-                              (VOID **)&mSmmCoreLoadedImage->FilePath
-                              );
-              ASSERT_EFI_ERROR (Status);
-              CopyMem (mSmmCoreLoadedImage->FilePath, &mFvDevicePath, GetDevicePathSize ((EFI_DEVICE_PATH_PROTOCOL *)&mFvDevicePath));
-
-              mSmmCoreLoadedImage->DeviceHandle = FvHandle;
-            }
             if (mSmmCoreDriverEntry->SmmLoadedImage.FilePath == NULL) {
               //
               // Maybe one special FV contains only one SMM_CORE module, so its device path must
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index cfa9922cbd..c1b18b047e 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -104,8 +104,6 @@ EFI_SMRAM_DESCRIPTOR            *mFullSmramRanges;
 
 EFI_SMM_DRIVER_ENTRY            *mSmmCoreDriverEntry;
 
-EFI_LOADED_IMAGE_PROTOCOL       *mSmmCoreLoadedImage;
-
 /**
   Place holder function until all the SMM System Table Service are available.
 
@@ -756,38 +754,6 @@ SmmCoreInstallLoadedImage (
   )
 {
   EFI_STATUS                 Status;
-  EFI_HANDLE                 Handle;
-
-  //
-  // Allocate a Loaded Image Protocol in EfiBootServicesData
-  //
-  Status = gBS->AllocatePool (EfiBootServicesData, sizeof(EFI_LOADED_IMAGE_PROTOCOL), (VOID **)&mSmmCoreLoadedImage);
-  ASSERT_EFI_ERROR (Status);
-
-  ZeroMem (mSmmCoreLoadedImage, sizeof (EFI_LOADED_IMAGE_PROTOCOL));
-  //
-  // Fill in the remaining fields of the Loaded Image Protocol instance.
-  // Note: ImageBase is an SMRAM address that can not be accessed outside of SMRAM if SMRAM window is closed.
-  //
-  mSmmCoreLoadedImage->Revision      = EFI_LOADED_IMAGE_PROTOCOL_REVISION;
-  mSmmCoreLoadedImage->ParentHandle  = gSmmCorePrivate->SmmIplImageHandle;
-  mSmmCoreLoadedImage->SystemTable   = gST;
-
-  mSmmCoreLoadedImage->ImageBase     = (VOID *)(UINTN)gSmmCorePrivate->PiSmmCoreImageBase;
-  mSmmCoreLoadedImage->ImageSize     = gSmmCorePrivate->PiSmmCoreImageSize;
-  mSmmCoreLoadedImage->ImageCodeType = EfiRuntimeServicesCode;
-  mSmmCoreLoadedImage->ImageDataType = EfiRuntimeServicesData;
-
-  //
-  // Create a new image handle in the UEFI handle database for the SMM Driver
-  //
-  Handle = NULL;
-  Status = gBS->InstallMultipleProtocolInterfaces (
-                  &Handle,
-                  &gEfiLoadedImageProtocolGuid, mSmmCoreLoadedImage,
-                  NULL
-                  );
-  ASSERT_EFI_ERROR (Status);
 
   //
   // Allocate a Loaded Image Protocol in SMM
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index 50a7fc0000..3bbd1e1603 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -122,8 +122,8 @@ typedef struct {
   BOOLEAN                         Initialized;
   BOOLEAN                         DepexProtocolError;
 
-  EFI_HANDLE                      ImageHandle;
-  EFI_LOADED_IMAGE_PROTOCOL       *LoadedImage;
+  EFI_HANDLE                      ImageHandle_Reserved1;
+  EFI_LOADED_IMAGE_PROTOCOL       *LoadedImage_Reserved2;
   //
   // Image EntryPoint in SMRAM
   //
-- 
2.25.1.windows.1


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

View/Reply Online (#61325): https://edk2.groups.io/g/devel/message/61325
Mute This Topic: https://groups.io/mt/74912558/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list