MdeModulePkg/MemoryProtection: split protect and unprotect paths

Currently, the PE/COFF image memory protection code uses the same code
paths for protecting and unprotecting an image. This is strange, since
unprotecting an image involves a single call into the CPU arch protocol
to clear the permission attributes of the entire range, and there is no
need to parse the PE/COFF headers again.

So let's store the ImageRecord entries in a linked list, so we can find
it again at unprotect time, and simply clear the permissions.

Note that this fixes a DEBUG hang on an ASSERT() that occurs when the
PE/COFF image fails to load, which causes UnprotectUefiImage() to be
invoked before the image is fully loaded.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
This commit is contained in:
Ard Biesheuvel 2017-03-21 13:49:08 +00:00
parent 5d5a19028a
commit 5920a9d16b
1 changed files with 35 additions and 50 deletions

View File

@ -74,6 +74,8 @@ UINT32 mImageProtectionPolicy;
extern LIST_ENTRY mGcdMemorySpaceMap; extern LIST_ENTRY mGcdMemorySpaceMap;
STATIC LIST_ENTRY mProtectedImageRecordList;
/** /**
Sort code section in image record, based upon CodeSegmentBase from low to high. Sort code section in image record, based upon CodeSegmentBase from low to high.
@ -238,13 +240,10 @@ SetUefiImageMemoryAttributes (
Set UEFI image protection attributes. Set UEFI image protection attributes.
@param[in] ImageRecord A UEFI image record @param[in] ImageRecord A UEFI image record
@param[in] Protect TRUE: Protect the UEFI image.
FALSE: Unprotect the UEFI image.
**/ **/
VOID VOID
SetUefiImageProtectionAttributes ( SetUefiImageProtectionAttributes (
IN IMAGE_PROPERTIES_RECORD *ImageRecord, IN IMAGE_PROPERTIES_RECORD *ImageRecord
IN BOOLEAN Protect
) )
{ {
IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection; IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
@ -253,7 +252,6 @@ SetUefiImageProtectionAttributes (
LIST_ENTRY *ImageRecordCodeSectionList; LIST_ENTRY *ImageRecordCodeSectionList;
UINT64 CurrentBase; UINT64 CurrentBase;
UINT64 ImageEnd; UINT64 ImageEnd;
UINT64 Attribute;
ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList; ImageRecordCodeSectionList = &ImageRecord->CodeSegmentList;
@ -276,29 +274,19 @@ SetUefiImageProtectionAttributes (
// //
// DATA // DATA
// //
if (Protect) {
Attribute = EFI_MEMORY_XP;
} else {
Attribute = 0;
}
SetUefiImageMemoryAttributes ( SetUefiImageMemoryAttributes (
CurrentBase, CurrentBase,
ImageRecordCodeSection->CodeSegmentBase - CurrentBase, ImageRecordCodeSection->CodeSegmentBase - CurrentBase,
Attribute EFI_MEMORY_XP
); );
} }
// //
// CODE // CODE
// //
if (Protect) {
Attribute = EFI_MEMORY_RO;
} else {
Attribute = 0;
}
SetUefiImageMemoryAttributes ( SetUefiImageMemoryAttributes (
ImageRecordCodeSection->CodeSegmentBase, ImageRecordCodeSection->CodeSegmentBase,
ImageRecordCodeSection->CodeSegmentSize, ImageRecordCodeSection->CodeSegmentSize,
Attribute EFI_MEMORY_RO
); );
CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize; CurrentBase = ImageRecordCodeSection->CodeSegmentBase + ImageRecordCodeSection->CodeSegmentSize;
} }
@ -310,15 +298,10 @@ SetUefiImageProtectionAttributes (
// //
// DATA // DATA
// //
if (Protect) {
Attribute = EFI_MEMORY_XP;
} else {
Attribute = 0;
}
SetUefiImageMemoryAttributes ( SetUefiImageMemoryAttributes (
CurrentBase, CurrentBase,
ImageEnd - CurrentBase, ImageEnd - CurrentBase,
Attribute EFI_MEMORY_XP
); );
} }
return ; return ;
@ -401,18 +384,15 @@ FreeImageRecord (
} }
/** /**
Protect or unprotect UEFI image common function. Protect UEFI PE/COFF image
@param[in] LoadedImage The loaded image protocol @param[in] LoadedImage The loaded image protocol
@param[in] LoadedImageDevicePath The loaded image device path protocol @param[in] LoadedImageDevicePath The loaded image device path protocol
@param[in] Protect TRUE: Protect the UEFI image.
FALSE: Unprotect the UEFI image.
**/ **/
VOID VOID
ProtectUefiImageCommon ( ProtectUefiImage (
IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage, IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath, IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
IN BOOLEAN Protect
) )
{ {
VOID *ImageAddress; VOID *ImageAddress;
@ -617,34 +597,17 @@ ProtectUefiImageCommon (
// //
// CPU ARCH present. Update memory attribute directly. // CPU ARCH present. Update memory attribute directly.
// //
SetUefiImageProtectionAttributes (ImageRecord, Protect); SetUefiImageProtectionAttributes (ImageRecord);
// //
// Clean up // Record the image record in the list so we can undo the protections later
// //
FreeImageRecord (ImageRecord); InsertTailList (&mProtectedImageRecordList, &ImageRecord->Link);
Finish: Finish:
return ; return ;
} }
/**
Protect UEFI image.
@param[in] LoadedImage The loaded image protocol
@param[in] LoadedImageDevicePath The loaded image device path protocol
**/
VOID
ProtectUefiImage (
IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage,
IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
)
{
if (PcdGet32(PcdImageProtectionPolicy) != 0) {
ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, TRUE);
}
}
/** /**
Unprotect UEFI image. Unprotect UEFI image.
@ -657,8 +620,28 @@ UnprotectUefiImage (
IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath IN EFI_DEVICE_PATH_PROTOCOL *LoadedImageDevicePath
) )
{ {
IMAGE_PROPERTIES_RECORD *ImageRecord;
LIST_ENTRY *ImageRecordLink;
if (PcdGet32(PcdImageProtectionPolicy) != 0) { if (PcdGet32(PcdImageProtectionPolicy) != 0) {
ProtectUefiImageCommon (LoadedImage, LoadedImageDevicePath, FALSE); for (ImageRecordLink = mProtectedImageRecordList.ForwardLink;
ImageRecordLink != &mProtectedImageRecordList;
ImageRecordLink = ImageRecordLink->ForwardLink) {
ImageRecord = CR (
ImageRecordLink,
IMAGE_PROPERTIES_RECORD,
Link,
IMAGE_PROPERTIES_RECORD_SIGNATURE
);
if (ImageRecord->ImageBase == (EFI_PHYSICAL_ADDRESS)(UINTN)LoadedImage->ImageBase) {
SetUefiImageMemoryAttributes (ImageRecord->ImageBase,
ImageRecord->ImageSize,
0);
FreeImageRecord (ImageRecord);
return;
}
}
} }
} }
@ -1027,6 +1010,8 @@ CoreInitializeMemoryProtection (
mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy); mImageProtectionPolicy = PcdGet32(PcdImageProtectionPolicy);
InitializeListHead (&mProtectedImageRecordList);
// //
// Sanity check the PcdDxeNxMemoryProtectionPolicy setting: // Sanity check the PcdDxeNxMemoryProtectionPolicy setting:
// - code regions should have no EFI_MEMORY_XP attribute // - code regions should have no EFI_MEMORY_XP attribute