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


Marvin Häuser
 

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

Join devel@edk2.groups.io to automatically receive all group messages.