[ros-diffs] [cgutman] 54565: [NTOSKRNL] NtFreeVirtualMemory: - Handle the case where a region size of 0 is passed in - Return the correct error status for failure - Copy back the rounded values - Add checks an...

Author: cgutman
Date: Sat Dec  3 10:30:02 2011
New Revision: 54565

URL: http://svn.reactos.org/svn/reactos?rev=54565&view=rev
Log:
[NTOSKRNL]
NtFreeVirtualMemory:
- Handle the case where a region size of 0 is passed in
- Return the correct error status for failure
- Copy back the rounded values
- Add checks and prints to catch callers doing nasty things  (one is commented out because csrsrv triggers it trying to release the first 1 MB of RAM during video init, not sure what to do there)

- There is a heap bug where calling RtlReAllocateHeap on a large block allocation (HEAP_ENTRY_VIRTUAL_ALLOC is set) will cause a call to NtFreeVirtualMemory with an offset base (illegal) and a length smaller than the total pages reserved in the VM region (also illegal). This bug is exposed by setupapi when it parses large INF files (like the PRO/1000 driver INF).

Modified:
    trunk/reactos/ntoskrnl/mm/anonmem.c

Modified: trunk/reactos/ntoskrnl/mm/anonmem.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/anonmem.c?rev=5...
==============================================================================
--- trunk/reactos/ntoskrnl/mm/anonmem.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/anonmem.c [iso-8859-1] Sat Dec  3 10:30:02 2011
@@ -1090,8 +1090,6 @@
     }

BaseAddress = (PVOID)PAGE_ROUND_DOWN((PBaseAddress));
-    RegionSize = PAGE_ROUND_UP((ULONG_PTR)(PBaseAddress) + (PRegionSize)) -
-        PAGE_ROUND_DOWN((PBaseAddress));

AddressSpace = &Process->Vm;

@@ -1099,26 +1097,69 @@
     MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace, BaseAddress);
     if (MemoryArea == NULL)
     {
-        Status = STATUS_UNSUCCESSFUL;
+        DPRINT1("Unable to find memory area from address 0x%p\n", BaseAddress);
+        Status = STATUS_UNABLE_TO_FREE_VM;
         goto unlock_deref_and_return;
     }

+    if (PRegionSize != 0)
+    {
+        /* Use the region the caller wanted, rounded to whole pages */
+        RegionSize = PAGE_ROUND_UP((ULONG_PTR)(PBaseAddress) + (PRegionSize)) -
+        PAGE_ROUND_DOWN((PBaseAddress));
+    }
+    else
+    {
+        /* The caller wanted the whole memory area */
+        RegionSize = (ULONG_PTR)MemoryArea->EndingAddress -
+                     (ULONG_PTR)MemoryArea->StartingAddress;
+    }
+
     switch (FreeType)
     {
     case MEM_RELEASE:
-        /* We can only free a memory area in one step. */
-        if (MemoryArea->StartingAddress != BaseAddress ||
-                MemoryArea->Type != MEMORY_AREA_VIRTUAL_MEMORY)
-        {
-            Status = STATUS_UNSUCCESSFUL;
+        /* MEM_RELEASE must be used with the exact base and length
+         * that was returned by NtAllocateVirtualMemory */
+            
+        /* Verify the base address is correct */
+        if (MemoryArea->StartingAddress != BaseAddress)
+        {
+            DPRINT1("Base address is illegal for MEM_RELEASE (0x%p != 0x%p)\n",
+                    MemoryArea->StartingAddress,
+                    BaseAddress);
+            Status = STATUS_UNABLE_TO_FREE_VM;
+            goto unlock_deref_and_return;
+        }
+
+        /* Verify the region size is correct */
+        if ((ULONG_PTR)MemoryArea->StartingAddress + RegionSize !=
+            (ULONG_PTR)MemoryArea->EndingAddress)
+        {
+            DPRINT1("Region size is illegal for MEM_RELEASE (0x%x)\n", RegionSize);
+            Status = STATUS_UNABLE_TO_FREE_VM;
+            //goto unlock_deref_and_return;
+        }
+
+        if (MemoryArea->Type != MEMORY_AREA_VIRTUAL_MEMORY)
+        {
+            DPRINT1("Memory area is not VM\n");
+            Status = STATUS_UNABLE_TO_FREE_VM;
             goto unlock_deref_and_return;
         }

MmFreeVirtualMemory(Process, MemoryArea);
         Status = STATUS_SUCCESS;
-        goto unlock_deref_and_return;
+        break;

case MEM_DECOMMIT:
+        if ((ULONG_PTR)BaseAddress + RegionSize >
+            (ULONG_PTR)MemoryArea->EndingAddress)
+        {
+            DPRINT1("Invald base address (0x%p) and region size (0x%x) for MEM_DECOMMIT\n",
+                    BaseAddress, RegionSize);
+            Status = STATUS_UNABLE_TO_FREE_VM;
+            goto unlock_deref_and_return;
+        }
         Status = MmAlterRegion(AddressSpace,
             MemoryArea->StartingAddress,
             (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW) ?
@@ -1129,13 +1170,24 @@
             MEM_RESERVE,
             PAGE_NOACCESS,
             MmModifyAttributes);
+        if (!NT_SUCCESS(Status))
+        {
+            DPRINT1("MmAlterRegion failed with status 0x%x\n", Status);
+            Status = STATUS_UNABLE_TO_FREE_VM;
+            goto unlock_deref_and_return;
+        }
+        break;
+
+    default:
+        Status = STATUS_NOT_IMPLEMENTED;
         goto unlock_deref_and_return;
     }

-    Status = STATUS_NOT_IMPLEMENTED;
-
-    unlock_deref_and_return:
-
+    /* Copy rounded values back in success case */
+    *UBaseAddress = BaseAddress;
+    *URegionSize = RegionSize;
+
+unlock_deref_and_return:
     MmUnlockAddressSpace(AddressSpace);
     if (Attached) KeUnstackDetachProcess(&ApcState);
     if (ProcessHandle != NtCurrentProcess()) ObDereferenceObject(Process);