MdeModulePkg: DebugImageInfoTable: Fix Array Maintenance

The DebugImageInfoTable contains an array of image info
structures. The current implementation removes an entry by
freeing the info structure and putting NULL in that entry of
the array. It then decrements the table size tracked in the table.
However, the array is invalid at this point, it contains a NULL
entry, which the UEFI spec does not envision and it contains a valid
entry past the end of the array as tracked in the spec defined config
table. If the table is consumed at this point it can lead to an
invalid assessment of the image state, which defeats the purpose of
the table.

When a new info structure is added, it then scans for the first NULL
entry adds a pointer to the new info structure there and increments
the table size to cover the entrythat was formerly past the end of
the array.

The current implementation requires that once an unload happens,
more loads happen than unloads and that the last operation is not
an unload (which won't be true in the shell, e.g.). This is
needlessly complex, as the order of the table doesn't matter
(and in fact this implementation doesn't preserve image loading
order either).

This patch updates the removal function to free the desired
info structure, move the last entry of the array to this freed
spot, mark the last entry as NULL, and decrement the table count.
The entry addition function then just always puts a new entry at
the end of the array, expanding it as necessary. This simplifies
the logic and covers the gaps that were present.

Signed-off-by: Oliver Smith-Denny <[email protected]>

Author: os-d

MdeModulePkg/Core/Dxe/Misc/DebugImageInfo.c

@@ -177,20 +177,7 @@ CoreNewDebugImageInfoEntry (
 
   Table = mDebugInfoTableHeader.EfiDebugImageInfoTable;
 
-  if (mDebugInfoTableHeader.TableSize < mMaxTableEntries) {
-    //
-    // We still have empty entires in the Table, find the first empty entry.
-    //
-    Index = 0;
-    while (Table[Index].NormalImage != NULL) {
-      Index++;
-    }
-
-    //
-    // There must be an empty entry in the in the table.
-    //
-    ASSERT (Index < mMaxTableEntries);
-  } else {
+  if (mDebugInfoTableHeader.TableSize >= mMaxTableEntries) {
     //
     //  Table is full, so re-allocate another page for a larger table...
     //
@@ -218,10 +205,12 @@ CoreNewDebugImageInfoEntry (
     // Enlarge the max table entries and set the first empty entry index to
     // be the original max table entries.
     //
-    Index             = mMaxTableEntries;
     mMaxTableEntries += EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE;
   }
 
+  // We always put the next entry at the end of the currently consumed table (i.e. first free entry)
+  Index = mDebugInfoTableHeader.TableSize;
+
   //
   // Allocate data for new entry
   //
@@ -264,11 +253,13 @@ CoreRemoveDebugImageInfoEntry (
   for (Index = 0; Index < mMaxTableEntries; Index++) {
     if ((Table[Index].NormalImage != NULL) && (Table[Index].NormalImage->ImageHandle == ImageHandle)) {
       //
-      // Found a match. Free up the record, then NULL the pointer to indicate the slot
-      // is free.
+      // Found a match. Free up the record, then move the final entry down to this slot; we don't care about the
+      // order of the array
       //
       CoreFreePool (Table[Index].NormalImage);
-      Table[Index].NormalImage = NULL;
+      Table[Index].NormalImage                               = Table[mDebugInfoTableHeader.TableSize - 1].NormalImage;
+      Table[mDebugInfoTableHeader.TableSize - 1].NormalImage = NULL;
+
       //
       // Decrease the number of EFI_DEBUG_IMAGE_INFO elements and set the mDebugInfoTable in modified status.
       //