[NTOS:MM]

- In MiDeletePte, check the ReferenceCount of transition PTEs, not the ShareCount (which is actually u2.Blink, since the page is in a modified/standby list). Also don't reset the PageLocation, since MiDecrementReferenceCount expects it to be anything but ActiveAndValid.
Fixes physical page leaks when using DPH, or other code that sets PAGE_NOACCESS.
CORE-13311 #resolve

svn path=/trunk/; revision=75150

diff --git a/reactos/ntoskrnl/mm/ARM3/virtual.c b/reactos/ntoskrnl/mm/ARM3/virtual.c
index 19debba..e280ef1 100644 (file)
--- a/reactos/ntoskrnl/mm/ARM3/virtual.c
+++ b/reactos/ntoskrnl/mm/ARM3/virtual.c
@@ -419,6 +419,9 @@ MiDeletePte(IN PMMPTE PointerPte,
 
             DPRINT("Pte %p is transitional!\n", PointerPte);
 
+            /* Make sure the saved PTE address is valid */
+            ASSERT((PMMPTE)((ULONG_PTR)Pfn1->PteAddress & ~0x1) == PointerPte);
+
             /* Destroy the PTE */
             MI_ERASE_PTE(PointerPte);
 
@@ -428,17 +431,14 @@ MiDeletePte(IN PMMPTE PointerPte,
             ASSERT(Pfn1->u3.e1.PrototypePte == 0);
 
             /* Make the page free. For prototypes, it will be made free when deleting the section object */
-            if (Pfn1->u2.ShareCount == 0)
+            if (Pfn1->u3.e2.ReferenceCount == 0)
             {
-                ASSERT(Pfn1->u3.e2.ReferenceCount == 0);
-
                 /* And it should be in standby or modified list */
                 ASSERT((Pfn1->u3.e1.PageLocation == ModifiedPageList) || (Pfn1->u3.e1.PageLocation == StandbyPageList));
 
-                /* Unlink it and temporarily mark it as active */
+                /* Unlink it and set its reference count to one */
                 MiUnlinkPageFromList(Pfn1);
                 Pfn1->u3.e2.ReferenceCount++;
-                Pfn1->u3.e1.PageLocation = ActiveAndValid;
 
                 /* This will put it back in free list and clean properly up */
                 MI_SET_PFN_DELETED(Pfn1);

diff --git a/rostests/apitests/ntdll/NtProtectVirtualMemory.c b/rostests/apitests/ntdll/NtProtectVirtualMemory.c
index e492ece..60ef9a0 100644 (file)
--- a/rostests/apitests/ntdll/NtProtectVirtualMemory.c
+++ b/rostests/apitests/ntdll/NtProtectVirtualMemory.c
@@ -2,6 +2,8 @@
  * PROJECT:         ReactOS API Tests
  * LICENSE:         GPLv2+ - See COPYING in the top level directory
  * PURPOSE:         Test for the NtProtectVirtualMemory API
+ * PROGRAMMERS:     Jérôme Gardou <jerome.gardou@reactos.org>
+ *                  Thomas Faber <thomas.faber@reactos.org>
  */
 
 #include <apitest.h>
@@ -10,13 +12,15 @@
 #include <ndk/rtlfuncs.h>
 #include <ndk/mmfuncs.h>
 
-START_TEST(NtProtectVirtualMemory)
+static
+void
+TestReadWrite(void)
 {
     ULONG* allocationStart = NULL;
     NTSTATUS status;
     SIZE_T allocationSize;
     ULONG oldProtection;
-    
+
     /* Reserve a page */
     allocationSize = PAGE_SIZE;
     status = NtAllocateVirtualMemory(NtCurrentProcess(),
@@ -26,7 +30,7 @@ START_TEST(NtProtectVirtualMemory)
         MEM_RESERVE,
         PAGE_NOACCESS);
     ok(NT_SUCCESS(status), "Reserving memory failed\n");
-    
+
     /* Commit the page (RW) */
     status = NtAllocateVirtualMemory(NtCurrentProcess(),
         (void**)&allocationStart,
@@ -35,19 +39,19 @@ START_TEST(NtProtectVirtualMemory)
         MEM_COMMIT,
         PAGE_READWRITE);
     ok(NT_SUCCESS(status), "Commiting memory failed\n");
-    
+
     /* Try writing it */
     StartSeh()
     {
         *allocationStart = 0xFF;
     } EndSeh(STATUS_SUCCESS);
-    
+
     /* Try reading it */
     StartSeh()
     {
         ok(*allocationStart == 0xFF, "Memory was not written\n");
     } EndSeh(STATUS_SUCCESS);
-    
+
     /* Set it as read only */
     status = NtProtectVirtualMemory(NtCurrentProcess(),
         (void**)&allocationStart,
@@ -56,19 +60,19 @@ START_TEST(NtProtectVirtualMemory)
         &oldProtection);
     ok(NT_SUCCESS(status), "NtProtectVirtualMemory failed.\n");
     ok(oldProtection == PAGE_READWRITE, "Expected PAGE_READWRITE, got %08lx.\n", oldProtection);
-    
+
     /* Try writing it */
     StartSeh()
     {
         *allocationStart = 0xAA;
     } EndSeh(STATUS_ACCESS_VIOLATION);
-    
+
     /* Try reading it */
     StartSeh()
     {
         ok(*allocationStart == 0xFF, "read-only memory were changed.\n");
     } EndSeh(STATUS_SUCCESS);
-    
+
     /* Set it as no access */
     status = NtProtectVirtualMemory(NtCurrentProcess(),
         (void**)&allocationStart,
@@ -77,13 +81,13 @@ START_TEST(NtProtectVirtualMemory)
         &oldProtection);
     ok(NT_SUCCESS(status), "NtProtectVirtualMemory failed.\n");
     ok(oldProtection == PAGE_READONLY, "Expected PAGE_READONLY, got %08lx.\n", oldProtection);
-    
+
     /* Try writing it */
     StartSeh()
     {
         *allocationStart = 0xAA;
     } EndSeh(STATUS_ACCESS_VIOLATION);
-    
+
     /* Try reading it */
     StartSeh()
     {
@@ -110,7 +114,7 @@ START_TEST(NtProtectVirtualMemory)
     {
         ok(*allocationStart == 0xFF, "Memory content was not preserved.\n");
     } EndSeh(STATUS_SUCCESS);
-    
+
     /* Free memory */
     status = NtFreeVirtualMemory(NtCurrentProcess(),
         (void**)&allocationStart,
@@ -118,3 +122,64 @@ START_TEST(NtProtectVirtualMemory)
         MEM_RELEASE);
     ok(NT_SUCCESS(status), "Failed freeing memory.\n");
 }
+
+/* Regression test for CORE-13311 */
+static
+void
+TestFreeNoAccess(void)
+{
+    PVOID Mem;
+    SIZE_T Size;
+    NTSTATUS Status;
+    ULONG Iteration, PageNumber;
+    PUCHAR Page;
+    ULONG OldProtection;
+
+    for (Iteration = 0; Iteration < 50000; Iteration++)
+    {
+        Mem = NULL;
+        Size = 16 * PAGE_SIZE;
+        Status = NtAllocateVirtualMemory(NtCurrentProcess(),
+                                         &Mem,
+                                         0,
+                                         &Size,
+                                         MEM_COMMIT,
+                                         PAGE_READWRITE);
+        ok_ntstatus(Status, STATUS_SUCCESS);
+        if (!NT_SUCCESS(Status))
+        {
+            break;
+        }
+
+        for (PageNumber = 0; PageNumber < 16; PageNumber++)
+        {
+            Page = Mem;
+            Page += PageNumber * PAGE_SIZE;
+            ok(*Page == 0,
+               "[%lu, %lu] Got non-zero memory. %x at %p\n",
+               Iteration, PageNumber, *Page, Page);
+            *Page = 123;
+        }
+
+        Status = NtProtectVirtualMemory(NtCurrentProcess(),
+                                        &Mem,
+                                        &Size,
+                                        PAGE_NOACCESS,
+                                        &OldProtection);
+        ok_ntstatus(Status, STATUS_SUCCESS);
+        ok_hex(OldProtection, PAGE_READWRITE);
+
+        Size = 0;
+        Status = NtFreeVirtualMemory(NtCurrentProcess(),
+                                     &Mem,
+                                     &Size,
+                                     MEM_RELEASE);
+        ok_ntstatus(Status, STATUS_SUCCESS);
+    }
+}
+
+START_TEST(NtProtectVirtualMemory)
+{
+    TestReadWrite();
+    TestFreeNoAccess();
+}