[ion] 57229: [NTOSKRNL]: Fix a stupid bug in MiProtectVirtualMemory which was causing empty PTEs whose protection was being set to remain zero PTEs, instead of demand-zero PTEs, but still acquire a...
Author: ion Date: Mon Sep 3 16:29:31 2012 New Revision: 57229
URL: http://svn.reactos.org/svn/reactos?rev=57229&view=rev Log: [NTOSKRNL]: Fix a stupid bug in MiProtectVirtualMemory which was causing empty PTEs whose protection was being set to remain zero PTEs, instead of demand-zero PTEs, but still acquire a page table reference. When they were later touched in the page fault code, and made into demand-zero PTEs, they'd get referenced again, thus Aleksey hacked away all the referencing code to work around this (causing PDEs to disappear...) [NTOSKRNL]: Restore the page table reference counting mechanism, and put it in a macro to be cleaner. Also use macros for testing PD boundaries, instead of math-by-hand.
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c trunk/reactos/ntoskrnl/mm/ARM3/section.c trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
Modified: trunk/reactos/ntoskrnl/mm/ARM3/miarm.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/miarm.h?re... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/miarm.h [iso-8859-1] Mon Sep 3 16:29:31 2012 @@ -1619,6 +1619,41 @@ } }
+FORCEINLINE +VOID +MiIncrementPageTableReferences(IN PVOID Address) +{ + PUSHORT RefCount; + + RefCount = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]; + + *RefCount += 1; + ASSERT(*RefCount <= PTE_PER_PAGE); +} + +FORCEINLINE +VOID +MiDecrementPageTableReferences(IN PVOID Address) +{ + PUSHORT RefCount; + + RefCount = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]; + + *RefCount -= 1; + ASSERT(*RefCount < PTE_PER_PAGE); +} + +FORCEINLINE +USHORT +MiQueryPageTableReferences(IN PVOID Address) +{ + PUSHORT RefCount; + + RefCount = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]; + + return *RefCount; +} + BOOLEAN NTAPI MmArmInitSystem(
Modified: trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c [iso-8859-1] Mon Sep 3 16:29:31 2012 @@ -1684,8 +1684,7 @@ if (Address <= MM_HIGHEST_USER_ADDRESS) { /* Add an additional page table reference */ - MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++; - ASSERT(MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)] <= PTE_COUNT); + MiIncrementPageTableReferences(Address); }
/* Did we get a prototype PTE back? */
Modified: trunk/reactos/ntoskrnl/mm/ARM3/section.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/section.c?... ============================================================================== --- trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/mm/ARM3/section.c [iso-8859-1] Mon Sep 3 16:29:31 2012 @@ -1821,7 +1821,6 @@ PMMPDE PointerPde; PMMPFN Pfn1; ULONG ProtectionMask, QuotaCharge = 0; - PUSHORT UsedPageTableEntries; PETHREAD Thread = PsGetCurrentThread(); PAGED_CODE();
@@ -1914,9 +1913,7 @@ // This used to be a zero PTE and it no longer is, so we must add a // reference to the pagetable. // - UsedPageTableEntries = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(MiPteToAddress(PointerPte))]; - (*UsedPageTableEntries)++; - ASSERT((*UsedPageTableEntries) <= PTE_COUNT); + MiIncrementPageTableReferences(MiPteToAddress(PointerPte));
// // Create the demand-zero prototype PTE
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] Mon Sep 3 16:29:31 2012 @@ -589,10 +589,8 @@ TempPte = *PointerPte; if (TempPte.u.Long) { - DPRINT("Decrement used PTEs by address: %lx\n", Va); - (*UsedPageTableEntries)--; + *UsedPageTableEntries -= 1; ASSERT((*UsedPageTableEntries) < PTE_COUNT); - DPRINT("Refs: %lx\n", (*UsedPageTableEntries));
/* Check if the PTE is actually mapped in */ if (TempPte.u.Long & 0xFFFFFC01) @@ -653,14 +651,10 @@ /* The PDE should still be valid at this point */ ASSERT(PointerPde->u.Hard.Valid == 1);
- DPRINT("Should check if handles for: %p are zero (PDE: %lx)\n", Va, PointerPde->u.Hard.PageFrameNumber); - if (!(*UsedPageTableEntries)) - { - DPRINT("They are!\n"); + if (*UsedPageTableEntries == 0) + { if (PointerPde->u.Long != 0) { - DPRINT("PDE active: %lx in %16s\n", PointerPde->u.Hard.PageFrameNumber, CurrentProcess->ImageFileName); - /* Delete the PTE proper */ MiDeletePte(PointerPde, MiPteToAddress(PointerPde), @@ -1880,7 +1874,6 @@ ULONG_PTR StartingAddress, EndingAddress; PMMPTE PointerPde, PointerPte, LastPte; MMPTE PteContents; - //PUSHORT UsedPageTableEntries; PMMPFN Pfn1; ULONG ProtectionMask, OldProtect; BOOLEAN Committed; @@ -2050,24 +2043,23 @@ while (PointerPte <= LastPte) { /* Check if we've crossed a PDE boundary and make the new PDE valid too */ - if ((((ULONG_PTR)PointerPte) & (SYSTEM_PD_SIZE - 1)) == 0) + if (MiIsPteOnPdeBoundary(PointerPte)) { PointerPde = MiAddressToPte(PointerPte); MiMakePdeExistAndMakeValid(PointerPde, Process, MM_NOIRQL); }
- /* Capture the PTE and see what we're dealing with */ + /* Capture the PTE and check if it was empty */ PteContents = *PointerPte; if (PteContents.u.Long == 0) { /* This used to be a zero PTE and it no longer is, so we must add a reference to the pagetable. */ - //UsedPageTableEntries = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(MiPteToAddress(PointerPte))]; - //(*UsedPageTableEntries)++; - //ASSERT((*UsedPageTableEntries) <= PTE_COUNT); - DPRINT1("HACK: Not increasing UsedPageTableEntries count!\n"); - } - else if (PteContents.u.Hard.Valid == 1) + MiIncrementPageTableReferences(MiPteToAddress(PointerPte)); + } + + /* Check what kind of PTE we are dealing with */ + if (PteContents.u.Hard.Valid == 1) { /* Get the PFN entry */ Pfn1 = MiGetPfnEntry(PFN_FROM_PTE(&PteContents)); @@ -2080,8 +2072,11 @@ (NewAccessProtection & PAGE_GUARD)) { /* The page should be in the WS and we should make it transition now */ - UNIMPLEMENTED; - //continue; + DPRINT1("Making valid page invalid is not yet supported!\n"); + Status = STATUS_NOT_IMPLEMENTED; + /* Unlock the working set */ + MiUnlockProcessWorkingSetUnsafe(Process, Thread); + goto FailPath; }
/* Write the protection mask and write it with a TLB flush */ @@ -2270,7 +2265,6 @@ ULONG PteCount = 0; PMMPFN Pfn1; MMPTE PteContents; - PUSHORT UsedPageTableEntries; PETHREAD CurrentThread = PsGetCurrentThread();
// @@ -2292,7 +2286,7 @@ // // Check if we've crossed a PDE boundary // - if ((((ULONG_PTR)PointerPte) & (SYSTEM_PD_SIZE - 1)) == 0) + if (MiIsPteOnPdeBoundary(PointerPte)) { // // Get the new PDE and flush the valid PTEs we had built up until @@ -2383,9 +2377,7 @@ // This used to be a zero PTE and it no longer is, so we must add a // reference to the pagetable. // - UsedPageTableEntries = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(StartingAddress)]; - (*UsedPageTableEntries)++; - ASSERT((*UsedPageTableEntries) <= PTE_COUNT); + MiIncrementPageTableReferences(StartingAddress);
// // Next, we account for decommitted PTEs and make the PTE as such @@ -3648,7 +3640,6 @@ PMEMORY_AREA MemoryArea; PFN_NUMBER PageCount; PMMVAD Vad, FoundVad; - PUSHORT UsedPageTableEntries; NTSTATUS Status; PMMSUPPORT AddressSpace; PVOID PBaseAddress; @@ -4268,7 +4259,7 @@ // // Have we crossed into a new page table? // - if (!(((ULONG_PTR)PointerPte) & (SYSTEM_PD_SIZE - 1))) + if (MiIsPteOnPdeBoundary(PointerPte)) { // // Get the PDE and now make it valid too @@ -4286,9 +4277,7 @@ // First increment the count of pages in the page table for this // process // - UsedPageTableEntries = &MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(MiPteToAddress(PointerPte))]; - (*UsedPageTableEntries)++; - ASSERT((*UsedPageTableEntries) <= PTE_COUNT); + MiIncrementPageTableReferences(MiPteToAddress(PointerPte));
// // And now write the invalid demand-zero PTE as requested
-
ion＠svn.reactos.org