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/NtProtectVi... ============================================================================== --- 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@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 @@ 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(); +}