[tfaber] 75150: [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 re...

Author: tfaber
Date: Tue Jun 20 15:51:47 2017
New Revision: 75150

URL: http://svn.reactos.org/svn/reactos?rev=75150&view=rev
Log:
[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

Modified:
    trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
    trunk/rostests/apitests/ntdll/NtProtectVirtualMemory.c

Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c	[iso-8859-1] Tue Jun 20 15:51:47 2017
@@ -419,6 +419,9 @@
 
             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 @@
             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)
-            {
-                ASSERT(Pfn1->u3.e2.ReferenceCount == 0);
-
+            if (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);

Modified: trunk/rostests/apitests/ntdll/NtProtectVirtualMemory.c
URL:
http://svn.reactos.org/svn/reactos/trunk/rostests/apitests/ntdll/NtProtectV…
==============================================================================
--- trunk/rostests/apitests/ntdll/NtProtectVirtualMemory.c	[iso-8859-1] (original)
+++ trunk/rostests/apitests/ntdll/NtProtectVirtualMemory.c	[iso-8859-1] Tue Jun 20
15:51:47 2017
@@ -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(a)reactos.org>
+ *                  Thomas Faber <thomas.faber(a)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 @@
         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 @@
         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 @@
         &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 @@
         &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 @@
     {
         ok(*allocationStart == 0xFF, "Memory content was not preserved.\n");
     } EndSeh(STATUS_SUCCESS);
-    
+
     /* Free memory */
     status = NtFreeVirtualMemory(NtCurrentProcess(),
         (void**)&allocationStart,
@@ -118,3 +122,64 @@
         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();
+}