From: "Oliver Smith-Denny" <osde@linux.microsoft.com>
To: devel@edk2.groups.io
Cc: Liming Gao <gaoliming@byosoft.com.cn>,
Taylor Beebe <taylor.d.beebe@gmail.com>,
Michael D Kinney <michael.d.kinney@intel.com>,
Ard Biesheuvel <ardb@kernel.org>
Subject: [edk2-devel] [PATCH v3 2/3] MdeModulePkg: ImagePropertiesRecordLib: Consolidate\r Usage
Date: Fri, 29 Mar 2024 13:21:28 -0700 [thread overview]
Message-ID: <20240329202129.12988-3-osde@linux.microsoft.com> (raw)
In-Reply-To: <20240329202129.12988-1-osde@linux.microsoft.com>
Currently, there are multiple instances of code create image
records. ImagePropertiesRecordLib was created to only have
this code in one place. Update the lib to use additional
logic from the copy in MemoryProtection.c before converging
that code to use the lib.
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Taylor Beebe <taylor.d.beebe@gmail.com>
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Oliver Smith-Denny <osde@linux.microsoft.com>
---
MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c | 82 +++++++++++++++-----
1 file changed, 63 insertions(+), 19 deletions(-)
diff --git a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
index 763a8d65d565..3ac043f98098 100644
--- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
+++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
@@ -60,6 +60,39 @@ EfiSizeToPages (
return RShiftU64 (Size, EFI_PAGE_SHIFT) + ((((UINTN)Size) & EFI_PAGE_MASK) ? 1 : 0);
}
+/**
+ Frees the memory for each ImageRecordCodeSection within an ImageRecord
+ and removes the entries from the list. It does not free the ImageRecord
+ itself.
+
+ @param[in] ImageRecord The ImageRecord in which to free code sections
+**/
+STATIC
+VOID
+FreeImageRecordCodeSections (
+ IMAGE_PROPERTIES_RECORD *ImageRecord
+ )
+{
+ LIST_ENTRY *CodeSegmentListHead;
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
+
+ if (ImageRecord == NULL) {
+ return;
+ }
+
+ CodeSegmentListHead = &ImageRecord->CodeSegmentList;
+ while (!IsListEmpty (CodeSegmentListHead)) {
+ ImageRecordCodeSection = CR (
+ CodeSegmentListHead->ForwardLink,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION,
+ Link,
+ IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
+ );
+ RemoveEntryList (&ImageRecordCodeSection->Link);
+ FreePool (ImageRecordCodeSection);
+ }
+}
+
/**
Sort memory map entries based upon PhysicalStart from low to high.
@@ -993,6 +1026,7 @@ CreateImagePropertiesRecord (
UINT8 *Name;
UINT32 SectionAlignment;
UINT32 PeCoffHeaderOffset;
+ CHAR8 *PdbPointer;
if ((ImageRecord == NULL) || (ImageBase == NULL)) {
return EFI_INVALID_PARAMETER;
@@ -1016,6 +1050,11 @@ CreateImagePropertiesRecord (
InitializeListHead (&ImageRecord->Link);
InitializeListHead (&ImageRecord->CodeSegmentList);
+ PdbPointer = PeCoffLoaderGetPdbPointer ((VOID *)(UINTN)ImageBase);
+ if (PdbPointer != NULL) {
+ DEBUG ((DEBUG_ERROR, " Image - %a\n", PdbPointer));
+ }
+
// Check PE/COFF image
DosHdr = (EFI_IMAGE_DOS_HEADER *)(UINTN)ImageBase;
PeCoffHeaderOffset = 0;
@@ -1084,7 +1123,8 @@ CreateImagePropertiesRecord (
// Record code section(s)
ImageRecordCodeSection = AllocatePool (sizeof (*ImageRecordCodeSection));
if (ImageRecordCodeSection == NULL) {
- return EFI_OUT_OF_RESOURCES;
+ Status = EFI_OUT_OF_RESOURCES;
+ goto CreateImagePropertiesRecordEnd;
}
ImageRecordCodeSection->Signature = IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE;
@@ -1103,6 +1143,27 @@ CreateImagePropertiesRecord (
SortImageRecordCodeSection (ImageRecord);
}
+ //
+ // Check overlap all section in ImageBase/Size
+ //
+ if (!IsImageRecordCodeSectionValid (ImageRecord)) {
+ DEBUG ((DEBUG_ERROR, "IsImageRecordCodeSectionValid - FAIL\n"));
+ Status = EFI_INVALID_PARAMETER;
+ goto CreateImagePropertiesRecordEnd;
+ }
+
+ //
+ // Round up the ImageSize, some CPU arch may return EFI_UNSUPPORTED if ImageSize is not aligned.
+ // Given that the loader always allocates full pages, we know the space after the image is not used.
+ //
+ ImageRecord->ImageSize = ALIGN_VALUE (ImageRecord->ImageSize, EFI_PAGE_SIZE);
+
+CreateImagePropertiesRecordEnd:
+ if (EFI_ERROR (Status)) {
+ // we failed to create a valid record, free the section memory that was allocated
+ FreeImageRecordCodeSections (ImageRecord);
+ }
+
return Status;
}
@@ -1119,24 +1180,7 @@ DeleteImagePropertiesRecord (
IN IMAGE_PROPERTIES_RECORD *ImageRecord
)
{
- LIST_ENTRY *CodeSegmentListHead;
- IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-
- if (ImageRecord == NULL) {
- return;
- }
-
- CodeSegmentListHead = &ImageRecord->CodeSegmentList;
- while (!IsListEmpty (CodeSegmentListHead)) {
- ImageRecordCodeSection = CR (
- CodeSegmentListHead->ForwardLink,
- IMAGE_PROPERTIES_RECORD_CODE_SECTION,
- Link,
- IMAGE_PROPERTIES_RECORD_CODE_SECTION_SIGNATURE
- );
- RemoveEntryList (&ImageRecordCodeSection->Link);
- FreePool (ImageRecordCodeSection);
- }
+ FreeImageRecordCodeSections (ImageRecord);
if (!IsListEmpty (&ImageRecord->Link)) {
RemoveEntryList (&ImageRecord->Link);
--
2.40.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117241): https://edk2.groups.io/g/devel/message/117241
Mute This Topic: https://groups.io/mt/105223004/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2024-03-29 20:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-29 20:21 [edk2-devel] [PATCH v3 0/3] MdeModulePkg: ImagePropertiesRecordLib Fixes Oliver Smith-Denny
2024-03-29 20:21 ` [edk2-devel] [PATCH v3 1/3] MdeModulePkg: ImagePropertiesRecordLib: Use SectionAlignment for CodeSize Oliver Smith-Denny
2024-03-29 20:21 ` Oliver Smith-Denny [this message]
2024-03-29 20:21 ` [edk2-devel] [PATCH v3 3/3] MdeModulePkg: MemoryProtection: Use ImageRecordPropertiesLib Oliver Smith-Denny
2024-03-30 0:06 ` [edk2-devel] [PATCH v3 0/3] MdeModulePkg: ImagePropertiesRecordLib Fixes Michael D Kinney
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=20240329202129.12988-3-osde@linux.microsoft.com \
--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