[edk2-devel] [PATCH] MdeModulePkg/CoreDxe: Drop caller-allocated image buffers

Marvin Häuser posted 1 patch 2 years, 8 months ago
Failed in applying to current master (apply log)
MdeModulePkg/Core/Dxe/Image/Image.c | 199 ++++++--------------
1 file changed, 62 insertions(+), 137 deletions(-)
[edk2-devel] [PATCH] MdeModulePkg/CoreDxe: Drop caller-allocated image buffers
Posted by Marvin Häuser 2 years, 8 months ago
The current image loading code supports using an caller-allocated
buffer to load an UEFI image into. This concept is inherently flawed
as a caller would need to parse the image itself first to retrieve
the appropriate destination size.

As the only caller does not use this functionality, remove it.
Further drop the EntryPoint parameter, as it is unused as well.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
 MdeModulePkg/Core/Dxe/Image/Image.c | 199 ++++++--------------
 1 file changed, 62 insertions(+), 137 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
index 641a5715b112..1de83f96e5ed 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -540,8 +540,6 @@ CoreIsImageTypeSupported (
                                   boot selection.

   @param  Pe32Handle              The handle of PE32 image

   @param  Image                   PE image to be loaded

-  @param  DstBuffer               The buffer to store the image

-  @param  EntryPoint              A pointer to the entry point

   @param  Attribute               The bit mask of attributes to set for the load

                                   PE image

 

@@ -557,13 +555,10 @@ CoreLoadPeImage (
   IN BOOLEAN                     BootPolicy,

   IN VOID                        *Pe32Handle,

   IN LOADED_IMAGE_PRIVATE_DATA   *Image,

-  IN EFI_PHYSICAL_ADDRESS        DstBuffer    OPTIONAL,

-  OUT EFI_PHYSICAL_ADDRESS       *EntryPoint  OPTIONAL,

   IN  UINT32                     Attribute

   )

 {

   EFI_STATUS                Status;

-  BOOLEAN                   DstBufAlocated;

   UINTN                     Size;

 

   ZeroMem (&Image->ImageContext, sizeof (Image->ImageContext));

@@ -615,93 +610,63 @@ CoreLoadPeImage (
   //

   // Allocate memory of the correct memory type aligned on the required image boundary

   //

-  DstBufAlocated = FALSE;

-  if (DstBuffer == 0) {

-    //

-    // Allocate Destination Buffer as caller did not pass it in

-    //

 

-    if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {

-      Size = (UINTN)Image->ImageContext.ImageSize + Image->ImageContext.SectionAlignment;

-    } else {

-      Size = (UINTN)Image->ImageContext.ImageSize;

-    }

-

-    Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);

-

-    //

-    // If the image relocations have not been stripped, then load at any address.

-    // Otherwise load at the address at which it was linked.

-    //

-    // Memory below 1MB should be treated reserved for CSM and there should be

-    // no modules whose preferred load addresses are below 1MB.

-    //

-    Status = EFI_OUT_OF_RESOURCES;

-    //

-    // If Loading Module At Fixed Address feature is enabled, the module should be loaded to

-    // a specified address.

-    //

-    if (PcdGet64(PcdLoadModuleAtFixAddressEnable) != 0 ) {

-      Status = GetPeCoffImageFixLoadingAssignedAddress (&(Image->ImageContext));

-

-      if (EFI_ERROR (Status))  {

-          //

-          // If the code memory is not ready, invoke CoreAllocatePage with AllocateAnyPages to load the driver.

-          //

-          DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED ERROR: Loading module at fixed address failed since specified memory is not available.\n"));

-

-          Status = CoreAllocatePages (

-                     AllocateAnyPages,

-                     (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

-                     Image->NumberOfPages,

-                     &Image->ImageContext.ImageAddress

-                     );

-      }

-    } else {

-      if (Image->ImageContext.ImageAddress >= 0x100000 || Image->ImageContext.RelocationsStripped) {

-        Status = CoreAllocatePages (

-                   AllocateAddress,

-                   (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

-                   Image->NumberOfPages,

-                   &Image->ImageContext.ImageAddress

-                   );

-      }

-      if (EFI_ERROR (Status) && !Image->ImageContext.RelocationsStripped) {

-        Status = CoreAllocatePages (

-                   AllocateAnyPages,

-                   (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

-                   Image->NumberOfPages,

-                   &Image->ImageContext.ImageAddress

-                   );

-      }

-    }

-    if (EFI_ERROR (Status)) {

-      return Status;

-    }

-    DstBufAlocated = TRUE;

+  if (Image->ImageContext.SectionAlignment > EFI_PAGE_SIZE) {

+    Size = (UINTN)Image->ImageContext.ImageSize + Image->ImageContext.SectionAlignment;

   } else {

-    //

-    // Caller provided the destination buffer

-    //

-

-    if (Image->ImageContext.RelocationsStripped && (Image->ImageContext.ImageAddress != DstBuffer)) {

+    Size = (UINTN)Image->ImageContext.ImageSize;

+  }

+

+  Image->NumberOfPages = EFI_SIZE_TO_PAGES (Size);

+

+  //

+  // If the image relocations have not been stripped, then load at any address.

+  // Otherwise load at the address at which it was linked.

+  //

+  // Memory below 1MB should be treated reserved for CSM and there should be

+  // no modules whose preferred load addresses are below 1MB.

+  //

+  Status = EFI_OUT_OF_RESOURCES;

+  //

+  // If Loading Module At Fixed Address feature is enabled, the module should be loaded to

+  // a specified address.

+  //

+  if (PcdGet64(PcdLoadModuleAtFixAddressEnable) != 0 ) {

+    Status = GetPeCoffImageFixLoadingAssignedAddress (&(Image->ImageContext));

+

+    if (EFI_ERROR (Status))  {

       //

-      // If the image relocations were stripped, and the caller provided a

-      // destination buffer address that does not match the address that the

-      // image is linked at, then the image cannot be loaded.

+      // If the code memory is not ready, invoke CoreAllocatePage with AllocateAnyPages to load the driver.

       //

-      return EFI_INVALID_PARAMETER;

+      DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED ERROR: Loading module at fixed address failed since specified memory is not available.\n"));

+

+      Status = CoreAllocatePages (

+                 AllocateAnyPages,

+                 (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

+                 Image->NumberOfPages,

+                 &Image->ImageContext.ImageAddress

+                 );

     }

-

-    if (Image->NumberOfPages != 0 &&

-        Image->NumberOfPages <

-        (EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext.ImageSize + Image->ImageContext.SectionAlignment))) {

-      Image->NumberOfPages = EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext.ImageSize + Image->ImageContext.SectionAlignment);

-      return EFI_BUFFER_TOO_SMALL;

+  } else {

+    if (Image->ImageContext.ImageAddress >= 0x100000 || Image->ImageContext.RelocationsStripped) {

+      Status = CoreAllocatePages (

+                  AllocateAddress,

+                  (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

+                  Image->NumberOfPages,

+                  &Image->ImageContext.ImageAddress

+                  );

     }

-

-    Image->NumberOfPages = EFI_SIZE_TO_PAGES ((UINTN)Image->ImageContext.ImageSize + Image->ImageContext.SectionAlignment);

-    Image->ImageContext.ImageAddress = DstBuffer;

+    if (EFI_ERROR (Status) && !Image->ImageContext.RelocationsStripped) {

+      Status = CoreAllocatePages (

+                  AllocateAnyPages,

+                  (EFI_MEMORY_TYPE) (Image->ImageContext.ImageCodeMemoryType),

+                  Image->NumberOfPages,

+                  &Image->ImageContext.ImageAddress

+                  );

+    }

+  }

+  if (EFI_ERROR (Status)) {

+    return Status;

   }

 

   Image->ImageBasePage = Image->ImageContext.ImageAddress;

@@ -783,13 +748,6 @@ CoreLoadPeImage (
     }

   }

 

-  //

-  // Fill in the entry point of the image if it is available

-  //

-  if (EntryPoint != NULL) {

-    *EntryPoint = Image->ImageContext.EntryPoint;

-  }

-

   //

   // Print the load address and the PDB file name if it is available

   //

@@ -854,11 +812,9 @@ Done:
   // Free memory.

   //

 

-  if (DstBufAlocated) {

-    CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);

-    Image->ImageContext.ImageAddress = 0;

-    Image->ImageBasePage = 0;

-  }

+  CoreFreePages (Image->ImageContext.ImageAddress, Image->NumberOfPages);

+  Image->ImageContext.ImageAddress = 0;

+  Image->ImageBasePage = 0;

 

   if (Image->ImageContext.FixupData != NULL) {

     CoreFreePool (Image->ImageContext.FixupData);

@@ -906,13 +862,11 @@ CoreLoadedImageInfo (
   Unloads EFI image from memory.

 

   @param  Image                   EFI image

-  @param  FreePage                Free allocated pages

 

 **/

 VOID

 CoreUnloadAndCloseImage (

-  IN LOADED_IMAGE_PRIVATE_DATA  *Image,

-  IN BOOLEAN                    FreePage

+  IN LOADED_IMAGE_PRIVATE_DATA  *Image

   )

 {

   EFI_STATUS                          Status;

@@ -1038,7 +992,7 @@ CoreUnloadAndCloseImage (
   //

   // Free the Image from memory

   //

-  if ((Image->ImageBasePage != 0) && FreePage) {

+  if (Image->ImageBasePage != 0) {

     CoreFreePages (Image->ImageBasePage, Image->NumberOfPages);

   }

 

@@ -1074,15 +1028,8 @@ CoreUnloadAndCloseImage (
   @param  SourceBuffer            If not NULL, a pointer to the memory location

                                   containing a copy of the image to be loaded.

   @param  SourceSize              The size in bytes of SourceBuffer.

-  @param  DstBuffer               The buffer to store the image

-  @param  NumberOfPages           If not NULL, it inputs a pointer to the page

-                                  number of DstBuffer and outputs a pointer to

-                                  the page number of the image. If this number is

-                                  not enough,  return EFI_BUFFER_TOO_SMALL and

-                                  this parameter contains the required number.

   @param  ImageHandle             Pointer to the returned image handle that is

                                   created when the image is successfully loaded.

-  @param  EntryPoint              A pointer to the entry point

   @param  Attribute               The bit mask of attributes to set for the load

                                   PE image

 

@@ -1112,10 +1059,7 @@ CoreLoadImageCommon (
   IN  EFI_DEVICE_PATH_PROTOCOL         *FilePath,

   IN  VOID                             *SourceBuffer       OPTIONAL,

   IN  UINTN                            SourceSize,

-  IN  EFI_PHYSICAL_ADDRESS             DstBuffer           OPTIONAL,

-  IN OUT UINTN                         *NumberOfPages      OPTIONAL,

   OUT EFI_HANDLE                       *ImageHandle,

-  OUT EFI_PHYSICAL_ADDRESS             *EntryPoint         OPTIONAL,

   IN  UINT32                           Attribute

   )

 {

@@ -1320,13 +1264,6 @@ CoreLoadImageCommon (
   Image->Info.FilePath     = DuplicateDevicePath (FilePath);

   Image->Info.ParentHandle = ParentImageHandle;

 

-

-  if (NumberOfPages != NULL) {

-    Image->NumberOfPages = *NumberOfPages ;

-  } else {

-    Image->NumberOfPages = 0 ;

-  }

-

   //

   // Install the protocol interfaces for this image

   // don't fire notifications yet

@@ -1343,22 +1280,13 @@ CoreLoadImageCommon (
   }

 

   //

-  // Load the image.  If EntryPoint is Null, it will not be set.

+  // Load the image.

   //

-  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, DstBuffer, EntryPoint, Attribute);

+  Status = CoreLoadPeImage (BootPolicy, &FHand, Image, Attribute);

   if (EFI_ERROR (Status)) {

-    if ((Status == EFI_BUFFER_TOO_SMALL) || (Status == EFI_OUT_OF_RESOURCES)) {

-      if (NumberOfPages != NULL) {

-        *NumberOfPages = Image->NumberOfPages;

-      }

-    }

     goto Done;

   }

 

-  if (NumberOfPages != NULL) {

-    *NumberOfPages = Image->NumberOfPages;

-  }

-

   //

   // Register the image in the Debug Image Info Table if the attribute is set

   //

@@ -1438,7 +1366,7 @@ Done:
   //

   if (EFI_ERROR (Status)) {

     if (Image != NULL) {

-      CoreUnloadAndCloseImage (Image, (BOOLEAN)(DstBuffer == 0));

+      CoreUnloadAndCloseImage (Image);

       Image = NULL;

     }

   } else if (EFI_ERROR (SecurityStatus)) {

@@ -1514,10 +1442,7 @@ CoreLoadImage (
              FilePath,

              SourceBuffer,

              SourceSize,

-             (EFI_PHYSICAL_ADDRESS) (UINTN) NULL,

-             NULL,

              ImageHandle,

-             NULL,

              EFI_LOAD_PE_IMAGE_ATTRIBUTE_RUNTIME_REGISTRATION | EFI_LOAD_PE_IMAGE_ATTRIBUTE_DEBUG_IMAGE_INFO_TABLE_REGISTRATION

              );

 

@@ -1734,7 +1659,7 @@ CoreStartImage (
   // unload it

   //

   if (EFI_ERROR (Image->Status) || Image->Type == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION) {

-    CoreUnloadAndCloseImage (Image, TRUE);

+    CoreUnloadAndCloseImage (Image);

     //

     // ImageHandle may be invalid after the image is unloaded, so use NULL handle to record perf log.

     //

@@ -1799,7 +1724,7 @@ CoreExit (
     //

     // The image has not been started so just free its resources

     //

-    CoreUnloadAndCloseImage (Image, TRUE);

+    CoreUnloadAndCloseImage (Image);

     Status = EFI_SUCCESS;

     goto Done;

   }

@@ -1901,7 +1826,7 @@ CoreUnloadImage (
     //

     // if the Image was not started or Unloaded O.K. then clean up

     //

-    CoreUnloadAndCloseImage (Image, TRUE);

+    CoreUnloadAndCloseImage (Image);

   }

 

 Done:

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78880): https://edk2.groups.io/g/devel/message/78880
Mute This Topic: https://groups.io/mt/84754052/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-