summaryrefslogtreecommitdiffstats
path: root/StandaloneMmPkg
diff options
context:
space:
mode:
authorOliver Smith-Denny <osde@microsoft.com>2025-01-03 12:51:15 -0800
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2025-01-09 12:44:46 +0000
commit21cbba1bb3789c6d225dffd15a7a638877a25fe6 (patch)
treeef59700adaeb325ad4ae3c4fbb6aef87fe942612 /StandaloneMmPkg
parent9bb11cad9db17dd790f82b1b0fdba0847086e3e1 (diff)
downloadedk2-21cbba1bb3789c6d225dffd15a7a638877a25fe6.tar.gz
StandaloneMmPkg: Call PeCoffLoaderUnloadImage When Unloading Image
Today, StandaloneMmCore calls PeCoffLoaderRelocateImage() when loading images, which calls PeCoffLoaderRelocateImageExtraAction(). On AARCH64, this sets the image memory protections accordingly, RO + E on code sections, RW + NX on data sections. However, if an image fails to start (i.e. its entry point returns a failure) StandaloneMmCore does not call the corresponding PeCoffLoaderUnloadImage, which calls PeCoffLoaderUnloadImageExtraAction, which on AARCH64 undoes the memory protections on the image, setting the whole memory region back to RW + NX. The core then frees this memory and the next allocation attempts to use it, which results in a data abort if a read only memory region is attempted to be written to. Theoretically, other instances of the PeCoffExtraActionLib could take other actions and so regardless of architecture, the contract with the PeCoffLoader should be maintained. This patch calls PeCoffLoaderUnloadImage when an image's entry point returns a failure, before freeing the image memory. This meets the contract and follows the DXE core behavior. Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>
Diffstat (limited to 'StandaloneMmPkg')
-rw-r--r--StandaloneMmPkg/Core/Dispatcher.c71
1 files changed, 42 insertions, 29 deletions
diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
index 6d9ca715a8..b686c31aaf 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -101,6 +101,7 @@ BOOLEAN gRequestDispatch = FALSE;
Loads an EFI image into SMRAM.
@param DriverEntry EFI_MM_DRIVER_ENTRY instance
+ @param ImageContext Allocated ImageContext to be filled out by this function
@return EFI_STATUS
@@ -108,33 +109,37 @@ BOOLEAN gRequestDispatch = FALSE;
EFI_STATUS
EFIAPI
MmLoadImage (
- IN OUT EFI_MM_DRIVER_ENTRY *DriverEntry
+ IN OUT EFI_MM_DRIVER_ENTRY *DriverEntry,
+ IN OUT PE_COFF_LOADER_IMAGE_CONTEXT *ImageContext
)
{
- UINTN PageCount;
- EFI_STATUS Status;
- EFI_PHYSICAL_ADDRESS DstBuffer;
- PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
+ UINTN PageCount;
+ EFI_STATUS Status;
+ EFI_PHYSICAL_ADDRESS DstBuffer;
DEBUG ((DEBUG_INFO, "MmLoadImage - %g\n", &DriverEntry->FileName));
+ if (ImageContext == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
Status = EFI_SUCCESS;
//
// Initialize ImageContext
//
- ImageContext.Handle = DriverEntry->Pe32Data;
- ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
+ ImageContext->Handle = DriverEntry->Pe32Data;
+ ImageContext->ImageRead = PeCoffLoaderImageReadFromMemory;
//
// Get information about the image being loaded
//
- Status = PeCoffLoaderGetImageInfo (&ImageContext);
+ Status = PeCoffLoaderGetImageInfo (ImageContext);
if (EFI_ERROR (Status)) {
return Status;
}
- PageCount = (UINTN)EFI_SIZE_TO_PAGES ((UINTN)ImageContext.ImageSize + ImageContext.SectionAlignment);
+ PageCount = (UINTN)EFI_SIZE_TO_PAGES ((UINTN)ImageContext->ImageSize + ImageContext->SectionAlignment);
DstBuffer = (UINTN)(-1);
Status = MmAllocatePages (
@@ -148,18 +153,18 @@ MmLoadImage (
return Status;
}
- ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)DstBuffer;
+ ImageContext->ImageAddress = (EFI_PHYSICAL_ADDRESS)DstBuffer;
//
// Align buffer on section boundary
//
- ImageContext.ImageAddress += ImageContext.SectionAlignment - 1;
- ImageContext.ImageAddress &= ~((EFI_PHYSICAL_ADDRESS)(ImageContext.SectionAlignment - 1));
+ ImageContext->ImageAddress += ImageContext->SectionAlignment - 1;
+ ImageContext->ImageAddress &= ~((EFI_PHYSICAL_ADDRESS)(ImageContext->SectionAlignment - 1));
//
// Load the image to our new buffer
//
- Status = PeCoffLoaderLoadImage (&ImageContext);
+ Status = PeCoffLoaderLoadImage (ImageContext);
if (EFI_ERROR (Status)) {
MmFreePages (DstBuffer, PageCount);
return Status;
@@ -168,8 +173,10 @@ MmLoadImage (
//
// Relocate the image in our new buffer
//
- Status = PeCoffLoaderRelocateImage (&ImageContext);
+ Status = PeCoffLoaderRelocateImage (ImageContext);
if (EFI_ERROR (Status)) {
+ // if relocate fails, we don't need to call unload image here, as the extra action that may change page attributes
+ // only is called on a successful return
MmFreePages (DstBuffer, PageCount);
return Status;
}
@@ -177,12 +184,12 @@ MmLoadImage (
//
// Flush the instruction cache so the image data are written before we execute it
//
- InvalidateInstructionCacheRange ((VOID *)(UINTN)ImageContext.ImageAddress, (UINTN)ImageContext.ImageSize);
+ InvalidateInstructionCacheRange ((VOID *)(UINTN)ImageContext->ImageAddress, (UINTN)ImageContext->ImageSize);
//
// Save Image EntryPoint in DriverEntry
//
- DriverEntry->ImageEntryPoint = ImageContext.EntryPoint;
+ DriverEntry->ImageEntryPoint = ImageContext->EntryPoint;
DriverEntry->ImageBuffer = DstBuffer;
DriverEntry->NumberOfPage = PageCount;
@@ -197,7 +204,7 @@ MmLoadImage (
DriverEntry->LoadedImage.FilePath = NULL;
DriverEntry->LoadedImage.ImageBase = (VOID *)(UINTN)DriverEntry->ImageBuffer;
- DriverEntry->LoadedImage.ImageSize = ImageContext.ImageSize;
+ DriverEntry->LoadedImage.ImageSize = ImageContext->ImageSize;
DriverEntry->LoadedImage.ImageCodeType = EfiRuntimeServicesCode;
DriverEntry->LoadedImage.ImageDataType = EfiRuntimeServicesData;
@@ -223,18 +230,18 @@ MmLoadImage (
DEBUG ((
DEBUG_INFO | DEBUG_LOAD,
"Loading MM driver at 0x%11p EntryPoint=0x%11p ",
- (VOID *)(UINTN)ImageContext.ImageAddress,
- FUNCTION_ENTRY_POINT (ImageContext.EntryPoint)
+ (VOID *)(UINTN)ImageContext->ImageAddress,
+ FUNCTION_ENTRY_POINT (ImageContext->EntryPoint)
));
//
// Print Module Name by Pdb file path.
// Windows and Unix style file path are all trimmed correctly.
//
- if (ImageContext.PdbPointer != NULL) {
+ if (ImageContext->PdbPointer != NULL) {
StartIndex = 0;
- for (Index = 0; ImageContext.PdbPointer[Index] != 0; Index++) {
- if ((ImageContext.PdbPointer[Index] == '\\') || (ImageContext.PdbPointer[Index] == '/')) {
+ for (Index = 0; ImageContext->PdbPointer[Index] != 0; Index++) {
+ if ((ImageContext->PdbPointer[Index] == '\\') || (ImageContext->PdbPointer[Index] == '/')) {
StartIndex = Index + 1;
}
}
@@ -245,7 +252,7 @@ MmLoadImage (
// If the length is bigger than 255, trim the redundant characters to avoid overflow in array boundary.
//
for (Index = 0; Index < sizeof (EfiFileName) - 4; Index++) {
- EfiFileName[Index] = ImageContext.PdbPointer[Index + StartIndex];
+ EfiFileName[Index] = ImageContext->PdbPointer[Index + StartIndex];
if (EfiFileName[Index] == 0) {
EfiFileName[Index] = '.';
}
@@ -381,11 +388,12 @@ MmDispatcher (
VOID
)
{
- EFI_STATUS Status;
- LIST_ENTRY *Link;
- EFI_MM_DRIVER_ENTRY *DriverEntry;
- BOOLEAN ReadyToRun;
- BOOLEAN PreviousMmEntryPointRegistered;
+ EFI_STATUS Status;
+ LIST_ENTRY *Link;
+ EFI_MM_DRIVER_ENTRY *DriverEntry;
+ BOOLEAN ReadyToRun;
+ BOOLEAN PreviousMmEntryPointRegistered;
+ PE_COFF_LOADER_IMAGE_CONTEXT ImageContext;
DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
@@ -424,7 +432,7 @@ MmDispatcher (
// skip the LoadImage
//
if (DriverEntry->ImageHandle == NULL) {
- Status = MmLoadImage (DriverEntry);
+ Status = MmLoadImage (DriverEntry, &ImageContext);
//
// Update the driver state to reflect that it's been loaded
@@ -461,6 +469,11 @@ MmDispatcher (
Status = ((MM_IMAGE_ENTRY_POINT)(UINTN)DriverEntry->ImageEntryPoint)(DriverEntry->ImageHandle, &gMmCoreMmst);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
+
+ // we need to unload the image before we free the pages. On some architectures (e.g. x86), this is a no-op, but
+ // on others (e.g. AARCH64) this will remove the image memory protections set on the region so that when the
+ // memory is freed, it has the default attributes set and can be used generically
+ PeCoffLoaderUnloadImage (&ImageContext);
MmFreePages (DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
if (DriverEntry->ImageHandle != NULL) {
MmUninstallProtocolInterface (