From: Serge Gautherie <reactos-git_serge_171003@gautherie.fr>
Date: Tue, 10 Oct 2017 01:39:44 +0000 (+0200)
Subject: [FREELDR] Additional safety checks for PcMemGetBiosMemoryMap. CORE-13332
X-Git-Tag: v0.4.7~152
X-Git-Url: https://git.reactos.org/?p=reactos.git;a=commitdiff_plain;h=ba9a1c3abba3b795a55186f289704ec8eeaf1cb9

[FREELDR] Additional safety checks for PcMemGetBiosMemoryMap. CORE-13332

Cast MAX_BIOS_DESCRIPTORS to explicit ULONG from implicit int.
Comment/Add checks for PcMemoryMap/PcBiosMemoryMap arrays being full and bare handling of error cases.
---

diff --git a/boot/freeldr/freeldr/arch/i386/pcmem.c b/boot/freeldr/freeldr/arch/i386/pcmem.c
index 2c615914f12..aa5b18d226c 100644
--- a/boot/freeldr/freeldr/arch/i386/pcmem.c
+++ b/boot/freeldr/freeldr/arch/i386/pcmem.c
@@ -33,7 +33,7 @@ DBG_DEFAULT_CHANNEL(MEMORY);
 #define ULONGLONG_ALIGN_UP_BY(size, align) \
     (ULONGLONG_ALIGN_DOWN_BY(((ULONGLONG)(size) + align - 1), align))
 
-#define MAX_BIOS_DESCRIPTORS 80
+#define MAX_BIOS_DESCRIPTORS 80ul
 
 BIOS_MEMORY_MAP PcBiosMemoryMap[MAX_BIOS_DESCRIPTORS];
 ULONG PcBiosMapCount;
@@ -368,6 +368,12 @@ PcMemGetBiosMemoryMap(PFREELDR_MEMORY_DESCRIPTOR MemoryMap, ULONG MaxMemoryMapSi
         {
             ERR("PcMemoryMap is already full! (PcBiosMapCount = %lu, PcMapCount = %lu (>= %lu))\n",
                 PcBiosMapCount, PcMapCount, MaxMemoryMapSize);
+            // NotWantedForPublicBuilds: ASSERTMSG("PcMemoryMap is already full!", FALSE);
+            /* We keep previous entries, and half-retrieve current/next entries.
+             * We assume all these entries are good to use as is. If they are not, we are in trouble...
+             *
+             * FIXME: Safer = revert (half-)retrieved entries, Safest = increase MaxMemoryMapSize.
+             */
         }
         else
         {
@@ -390,6 +396,18 @@ nextRange:
             break;
         }
     }
+    /* Check whether there would be more entries to process. */
+    if (PcBiosMapCount >= MAX_BIOS_DESCRIPTORS && Regs.x.ebx != 0x00000000)
+    {
+        ERR("PcBiosMapCount is already full! (PcBiosMapCount = %lu (>= %lu), PcMapCount = %lu)\n",
+            PcBiosMapCount, MAX_BIOS_DESCRIPTORS, PcMapCount);
+        // NotWantedForPublicBuilds: ASSERTMSG("PcBiosMapCount is already full!", FALSE);
+        /* We keep retrieved entries, but ignore next entries.
+         * We assume these entries are good to use as is. If they are not, we are in trouble...
+         *
+         * FIXME: Safer = revert retrieved entries, Safest = increase MAX_BIOS_DESCRIPTORS.
+         */
+    }
 
     TRACE("PcMemGetBiosMemoryMap end: PcBiosMapCount = %lu\n", PcBiosMapCount);
     return PcBiosMapCount;