public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marvin Häuser" <mhaeuser@posteo.de>
To: devel@edk2.groups.io
Cc: Jian J Wang <jian.j.wang@intel.com>,
	Hao A Wu <hao.a.wu@intel.com>, Dandan Bi <dandan.bi@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Vitaly Cheptsov <vit9696@protonmail.com>
Subject: [PATCH] MdeModulePkg/CoreDxe: Drop caller-allocated image buffers
Date: Sun,  8 Aug 2021 19:39:39 +0000	[thread overview]
Message-ID: <376e8f0af0ecac7debed5975f0741d880382c866.1628364368.git.mhaeuser@posteo.de> (raw)
In-Reply-To: <5df11a13422732b9c03c120775a2b4dd0a49182f.1628444003.git.mhaeuser@posteo.de>

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


  parent reply	other threads:[~2021-08-08 19:40 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-08 19:39 [PATCH] ArmPkg/DefaultExceptionHandlerLib: Fix DebugImageInfoTable lookup Marvin Häuser
2021-08-08 19:39 ` [PATCH] BaseTools: Define the read-only data section name per toolchain Marvin Häuser
2021-08-08 19:39   ` [PATCH] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name Marvin Häuser
2021-08-08 19:39 ` [PATCH] BaseTools/tools_def: Fix CLANGPDB X64 RCPATH Marvin Häuser
2021-08-08 19:39 ` [PATCH] EmulatorPkg/Host/Unix: Drop dlopen() usage Marvin Häuser
2021-08-08 19:39 ` [PATCH] EmulatorPkg/Host/Unix: Remove unused declarations Marvin Häuser
2021-08-08 19:39 ` Marvin Häuser [this message]
2021-08-08 19:39 ` [PATCH] MdeModulePkg/DxeCore: Consistent DebugImageInfoTable updates Marvin Häuser
2021-08-08 19:39   ` [PATCH] MdeModulePkg/DxeCore: Fix DebugImageInfoTable size report Marvin Häuser
2021-08-08 19:39   ` [PATCH] EmbeddedPkg/GdbStub: Check DebugImageInfoTable type safely Marvin Häuser
2021-08-08 19:39   ` [PATCH] ArmPkg/DefaultExceptionHandlerLib: " Marvin Häuser
2021-08-08 19:40   ` [PATCH] MdeModulePkg/CoreDxe: Mandatory LoadedImage for DebugImageInfoTable Marvin Häuser
2021-08-08 19:40   ` [PATCH] EmbeddedPkg/GdbStub: " Marvin Häuser
2021-08-08 19:40   ` [PATCH] ArmPkg/DefaultExceptionHandlerLib: " Marvin Häuser
2021-08-09  6:10   ` [PATCH] MdeModulePkg/DxeCore: Consistent DebugImageInfoTable updates Wu, Hao A
2021-08-09  6:15     ` Marvin Häuser
2021-08-09  6:52       ` [edk2-devel] " Wu, Hao A
2021-08-09  6:55         ` Wu, Hao A
2021-08-09  7:21         ` Marvin Häuser
2021-08-09  7:26           ` Wu, Hao A
2021-08-08 19:39 ` [PATCH] MdeModulePkg/DxeCore: Drop unnecessary pointer indirection Marvin Häuser
2021-08-08 19:39 ` [PATCH] MdeModulePkg/DxeCore: Use the correct source for fixed load address Marvin Häuser
2021-08-08 19:39 ` [PATCH] MdeModulePkg/PiSmmCore: Drop deprecated image profiling commands Marvin Häuser
2021-08-09  4:23   ` Ni, Ray
2021-08-09  5:33     ` Yao, Jiewen
2021-08-09  5:43       ` [edk2-devel] " Marvin Häuser
2021-08-08 19:39 ` [PATCH] MdeModulePkg/PiSmmIpl: Correct fixed load address bounds check Marvin Häuser
2021-08-08 19:39 ` [PATCH] MdePkg/Base.h: Introduce various alignment-related macros Marvin Häuser
2021-08-13  7:27   ` Wu, Hao A
2021-08-13  8:41     ` [edk2-devel] " Marvin Häuser
2021-08-13  8:45       ` Wu, Hao A
2021-08-08 19:39 ` [PATCH] MdePkg/BaseLib: Fix unaligned API prototypes Marvin Häuser
2021-08-08 19:39   ` [PATCH] BaseTools/CommonLib: " Marvin Häuser
2021-08-08 19:39 ` [PATCH] SecurityPkg/DxeImageVerificationLib: Always lookup SHA-256 hash in dbx Marvin Häuser
2021-08-09  0:02   ` Min Xu
2021-08-09  5:25     ` [edk2-devel] " Marvin Häuser
2021-08-09  2:48   ` Yao, Jiewen
2021-08-09  5:42     ` [edk2-devel] " Marvin Häuser
2021-08-08 19:39 ` [PATCH] SecurityPkg/DxeImageVerificationLib: Fix certificate lookup algorithm Marvin Häuser
2021-08-08 19:39   ` [PATCH] SecurityPkg/SecureBootConfigDxe: " Marvin Häuser
2021-08-08 19:39 ` [PATCH] StandaloneMmPkg/FvLib: Correct FV section data size Marvin Häuser
2021-08-08 19:39 ` [PATCH] StandaloneMmPkg/StandaloneMmCore: Drop code for traditional drivers Marvin Häuser
2021-08-08 19:39 ` [PATCH] StandaloneMmPkg/StandaloneMmCore: Drop unused fixed address feature Marvin Häuser
2021-08-08 19:39 ` [PATCH] StandaloneMmPkg: Support CLANGPDB X64 builds Marvin Häuser
2021-10-11  1:04   ` [edk2-devel] " Steven Shi
2021-08-08 19:39 ` [PATCH] UefiPayloadPkg/UefiPayloadEntry: Fix memory corruption Marvin Häuser
2021-08-09  4:20   ` Ni, Ray
2021-08-09  5:47     ` Marvin Häuser
2021-08-10 19:13   ` Guo Dong

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=376e8f0af0ecac7debed5975f0741d880382c866.1628364368.git.mhaeuser@posteo.de \
    --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