MdeModulePkg: Fix MAT SplitTable() Logic

SplitTable() does not properly handle the case where there is
an odd number of code regions within a loaded image. When there are
an odd number of code regions, at least one image region descriptor
is overwritten with uninitialized memory which has caused crashes
in the right conditions.

This failure cases is documented extensively in the following bugzilla:
https://bugzilla.tianocore.org/show_bug.cgi?id=4492

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Dandan Bi <dandan.bi@intel.com>
Signed-off-by: Taylor Beebe <taylor.d.beebe@gmail.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

1 files changed, 19 insertions, 19 deletions

diff --git a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
index 9d4082280b..379eb0c6cc 100644
--- a/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
+++ b/MdeModulePkg/Library/ImagePropertiesRecordLib/ImagePropertiesRecordLib.c
@@ -463,11 +463,12 @@ SplitTable (
 {

   INTN   IndexOld;

   INTN   IndexNew;

+  INTN   IndexNewStarting;

   UINTN  MaxSplitRecordCount;

   UINTN  RealSplitRecordCount;

-  UINTN  TotalSplitRecordCount;

+  UINTN  TotalSkippedRecords;

 

-  TotalSplitRecordCount = 0;

+  TotalSkippedRecords = 0;

   //

   // Let old record point to end of valid MemoryMap buffer.

   //

@@ -475,7 +476,8 @@ SplitTable (
   //

   // Let new record point to end of full MemoryMap buffer.

   //

-  IndexNew = ((*MemoryMapSize) / DescriptorSize) - 1 + NumberOfAdditionalDescriptors;

+  IndexNew         = ((*MemoryMapSize) / DescriptorSize) - 1 + NumberOfAdditionalDescriptors;

+  IndexNewStarting = IndexNew;

   for ( ; IndexOld >= 0; IndexOld--) {

     MaxSplitRecordCount = GetMaxSplitRecordCount ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + IndexOld * DescriptorSize), ImageRecordList);

     //

@@ -489,16 +491,14 @@ SplitTable (
                              DescriptorSize,

                              ImageRecordList

                              );

-    //

-    // Adjust IndexNew according to real split.

-    //

-    CopyMem (

-      ((UINT8 *)MemoryMap + (IndexNew + MaxSplitRecordCount - RealSplitRecordCount) * DescriptorSize),

-      ((UINT8 *)MemoryMap + IndexNew * DescriptorSize),

-      RealSplitRecordCount * DescriptorSize

-      );

-    IndexNew               = IndexNew + MaxSplitRecordCount - RealSplitRecordCount;

-    TotalSplitRecordCount += RealSplitRecordCount;

+

+    // If we didn't utilize all the extra allocated descriptor slots, set the physical address of the unused slots

+    // to MAX_ADDRESS so they are moved to the bottom of the list when sorting.

+    for ( ; RealSplitRecordCount < MaxSplitRecordCount; RealSplitRecordCount++) {

+      ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)MemoryMap + ((IndexNew + RealSplitRecordCount + 1) * DescriptorSize)))->PhysicalStart = MAX_ADDRESS;

+      TotalSkippedRecords++;

+    }

+

     IndexNew--;

   }

 

@@ -507,16 +507,16 @@ SplitTable (
   //

   CopyMem (

     MemoryMap,

-    (UINT8 *)MemoryMap + (NumberOfAdditionalDescriptors - TotalSplitRecordCount) * DescriptorSize,

-    (*MemoryMapSize) + TotalSplitRecordCount * DescriptorSize

+    (UINT8 *)MemoryMap + ((IndexNew + 1) * DescriptorSize),

+    (IndexNewStarting - IndexNew) * DescriptorSize

     );

 

-  *MemoryMapSize = (*MemoryMapSize) + DescriptorSize * TotalSplitRecordCount;

-

   //

-  // Sort from low to high (Just in case)

+  // Sort from low to high to filter out the MAX_ADDRESS records.

   //

-  SortMemoryMap (MemoryMap, *MemoryMapSize, DescriptorSize);

+  SortMemoryMap (MemoryMap, (IndexNewStarting - IndexNew) * DescriptorSize, DescriptorSize);

+

+  *MemoryMapSize = (IndexNewStarting - IndexNew - TotalSkippedRecords) * DescriptorSize;

 

   return;

 }