[ros-diffs] [reactos] 01/01: [FREELDR] Additional safety checks for PcMemGetBiosMemoryMap. CORE-13332

[Serge Gautherie]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=ba9a1c3abba3b795a5518…

commit ba9a1c3abba3b795a55186f289704ec8eeaf1cb9
Author: Serge Gautherie <reactos-git_serge_171003(a)gautherie.fr>
AuthorDate: Tue Oct 10 03:39:44 2017 +0200

    [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.
---
 boot/freeldr/freeldr/arch/i386/pcmem.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/boot/freeldr/freeldr/arch/i386/pcmem.c
b/boot/freeldr/freeldr/arch/i386/pcmem.c
index 2c615914f1..aa5b18d226 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;