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
next prev 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