From 64d3af216f65f9a4b2b7dedb8c761e6d689096ec Mon Sep 17 00:00:00 2001 From: Sir Richard Date: Wed, 29 Feb 2012 23:11:21 +0000 Subject: [PATCH] [NTOS]: A few other fixups to the page fault path: 1) Assert on empty kernel PTE instead of handling it as a bugcheck. Windows ASSERTs too. Also clarify some ASSERTs which Windows also does versus ASSERTs we are only doing due to lack of support for said feature. 2) User page fault path can now distinguish between a user-mode PTE fault, and a kernel-mode fault on a user PDE, both by creating a correct kernel PDE when needed instead of always creating user PTEs, as well as by only touching the UsedPageTableEntry reference counting mechanism when a user-address is in play. 3) Related to #2, also recognize when the faulting PTE is actually a PDE in the self-mapping region -- another scenario when the "user fault" is actually a kernel fault for a user PDE. 4) Add one more path where a Paged Pool PDE fixup can save the day instead of always faulting. 5) Finally, related to #2 and #3, handle the MI_IS_PAGE_TABLE_OR_HYPER_ADDRESS scenario for a User PDE by treating it as a user fault. The code looks deceptively similar but there are slight differences which require the separate codepaths with some duplicated code. The magic is in the ordering. In trunk, these changes should not cause any regressions (let's hope so). On the internal VAD-based Virtual Memory branch, they now allow booting to 3rd stage and a fully usable ReactOS environment. MEMORY_AREA_VIRTUAL_MEMORY is gone on that branch. It's coming. [NTOS]: Use PAGE_READWRITE as hardcoded protection instead of PAGE_EXECUTE_READWRITE -- the difference is meaningless on ReactOS Mm but actually causes issues on ARM3 with VADs. svn path=/trunk/; revision=55938 --- reactos/ntoskrnl/mm/ARM3/pagfault.c | 113 +++++++++++++++++----------- reactos/ntoskrnl/mm/anonmem.c | 4 +- 2 files changed, 70 insertions(+), 47 deletions(-) diff --git a/reactos/ntoskrnl/mm/ARM3/pagfault.c b/reactos/ntoskrnl/mm/ARM3/pagfault.c index a5d76bd6d92..efc520294ce 100644 --- a/reactos/ntoskrnl/mm/ARM3/pagfault.c +++ b/reactos/ntoskrnl/mm/ARM3/pagfault.c @@ -614,25 +614,17 @@ MiDispatchFault(IN BOOLEAN StoreInstruction, } // - // The PTE must be invalid + // The PTE must be invalid but not completely empty. It must also not be a + // prototype PTE as that scenario should've been handled above // ASSERT(TempPte.u.Hard.Valid == 0); - - /* Check if the PTE is completely empty */ - if (TempPte.u.Long == 0) - { - /* The address is not from any pageable area! */ - KeBugCheckEx(PAGE_FAULT_IN_NONPAGED_AREA, - (ULONG_PTR)Address, - StoreInstruction, - (ULONG_PTR)TrapInformation, - 2); - } - - // - // No prototype, transition or page file software PTEs in ARM3 yet - // ASSERT(TempPte.u.Soft.Prototype == 0); + ASSERT(TempPte.u.Long != 0); + + // + // No transition or page file software PTEs in ARM3 yet, so this must be a + // demand zero page + // ASSERT(TempPte.u.Soft.Transition == 0); ASSERT(TempPte.u.Soft.PageFileHigh == 0); @@ -770,22 +762,12 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, /* Get the current thread */ CurrentThread = PsGetCurrentThread(); - // Check for a fault on the page table or hyperspace - if (MI_IS_PAGE_TABLE_OR_HYPER_ADDRESS(Address)) - { - ASSERT(TempPte.u.Long != (MM_READWRITE << MM_PTE_SOFTWARE_PROTECTION_BITS)); - ASSERT(TempPte.u.Soft.Prototype == 0); + /* Check for a fault on the page table or hyperspace */ + if (MI_IS_PAGE_TABLE_OR_HYPER_ADDRESS(Address)) goto UserFault; - /* Use the process working set */ - CurrentProcess = (PEPROCESS)CurrentThread->Tcb.ApcState.Process; - WorkingSet = &CurrentProcess->Vm; - } - else - { - /* Otherwise use the system working set */ - WorkingSet = &MmSystemCacheWs; - CurrentProcess = NULL; - } + /* Use the system working set */ + WorkingSet = &MmSystemCacheWs; + CurrentProcess = NULL; /* Acquire the working set lock */ KeRaiseIrql(APC_LEVEL, &LockIrql); @@ -884,6 +866,7 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, } /* This is a user fault */ +UserFault: CurrentThread = PsGetCurrentThread(); CurrentProcess = (PEPROCESS)CurrentThread->Tcb.ApcState.Process; @@ -936,6 +919,9 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, /* Right now, we only handle scenarios where the PDE is totally empty */ ASSERT(PointerPde->u.Long == 0); + /* Right now, we expect a valid protection mask on the VAD */ + ASSERT(ProtectionCode != MM_NOACCESS); + /* And go dispatch the fault on the PDE. This should handle the demand-zero */ #if MI_TRACE_PFNS UserPdeFault = TRUE; @@ -978,9 +964,16 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, ProtoPte = MiCheckVirtualAddress(Address, &ProtectionCode, &Vad); if (ProtectionCode == MM_NOACCESS) { - /* This is a bogus VA */ +#if (_MI_PAGING_LEVELS == 2) + /* Could be a page table for paged pool */ + MiCheckPdeForPagedPool(Address); +#endif + /* Has the code above changed anything -- is this now a valid PTE? */ + Status = (PointerPte->u.Hard.Valid == 1) ? STATUS_SUCCESS : STATUS_ACCESS_VIOLATION; + + /* Either this was a bogus VA or we've fixed up a paged pool PDE */ MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread); - return STATUS_ACCESS_VIOLATION; + return Status; } /* Check for non-demand zero PTE */ @@ -1007,17 +1000,34 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, return Status; } -#if (_MI_PAGING_LEVELS == 2) - /* Add an additional page table reference */ - MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++; - ASSERT(MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)] <= PTE_COUNT); -#endif + /* + * Check if this is a real user-mode address or actually a kernel-mode + * page table for a user mode address + */ + if (Address <= MM_HIGHEST_USER_ADDRESS) + { + /* Add an additional page table reference */ + MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)]++; + ASSERT(MmWorkingSetList->UsedPageTableEntries[MiGetPdeOffset(Address)] <= PTE_COUNT); + } + + /* No guard page support yet */ + ASSERT((ProtectionCode & MM_DECOMMIT) == 0); /* Did we get a prototype PTE back? */ if (!ProtoPte) { - /* No, create a new PTE. First, write the protection */ - PointerPte->u.Soft.Protection = ProtectionCode; + /* Is this PTE actually part of the PDE-PTE self-mapping directory? */ + if (PointerPde == MiAddressToPde(PTE_BASE)) + { + /* Then it's really a demand-zero PDE (on behalf of user-mode) */ + MI_WRITE_INVALID_PTE(PointerPte, DemandZeroPde); + } + else + { + /* No, create a new PTE. First, write the protection */ + PointerPte->u.Soft.Protection = ProtectionCode; + } /* Lock the PFN database since we're going to grab a page */ OldIrql = KeAcquireQueuedSpinLock(LockQueuePfnLock); @@ -1052,17 +1062,30 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, /* And we're done with the lock */ KeReleaseQueuedSpinLock(LockQueuePfnLock, OldIrql); - /* User fault, build a user PTE */ - MI_MAKE_HARDWARE_PTE_USER(&TempPte, - PointerPte, - PointerPte->u.Soft.Protection, - PageFrameIndex); + /* Fault on user PDE, or fault on user PTE? */ + if (PointerPte <= MiHighestUserPte) + { + /* User fault, build a user PTE */ + MI_MAKE_HARDWARE_PTE_USER(&TempPte, + PointerPte, + PointerPte->u.Soft.Protection, + PageFrameIndex); + } + else + { + /* This is a user-mode PDE, create a kernel PTE for it */ + MI_MAKE_HARDWARE_PTE(&TempPte, + PointerPte, + PointerPte->u.Soft.Protection, + PageFrameIndex); + } /* Write the dirty bit for writeable pages */ if (MI_IS_PAGE_WRITEABLE(&TempPte)) MI_MAKE_DIRTY_PAGE(&TempPte); /* And now write down the PTE, making the address valid */ MI_WRITE_VALID_PTE(PointerPte, TempPte); + ASSERT(MiGetPfnEntry(PageFrameIndex)->u1.Event == NULL); /* Demand zero */ Status = STATUS_PAGE_FAULT_DEMAND_ZERO; diff --git a/reactos/ntoskrnl/mm/anonmem.c b/reactos/ntoskrnl/mm/anonmem.c index 2555ad5b7e5..5f8a7e4a3be 100644 --- a/reactos/ntoskrnl/mm/anonmem.c +++ b/reactos/ntoskrnl/mm/anonmem.c @@ -789,9 +789,9 @@ NtAllocateVirtualMemory(IN HANDLE ProcessHandle, MmLockAddressSpace(AddressSpace); // - // Force PAGE_EXECUTE_READWRITE for everything, for now + // Force PAGE_READWRITE for everything, for now // - Protect = PAGE_EXECUTE_READWRITE; + Protect = PAGE_READWRITE; if (PBaseAddress != 0) {