From 6086842a76345ff2cdad0713698211d7ea3c9300 Mon Sep 17 00:00:00 2001 From: Sir Richard Date: Mon, 27 Feb 2012 23:42:22 +0000 Subject: [PATCH] [NTOS]: A few key changes to the page fault path: 1) MiCheckVirtualAddress should be called *after* determining if the PTE is a Demand Zero PTE. This is because when memory is allocated with MEM_RESERVE, and then MEM_COMMIT is called later, the VAD does not have the MemCommit flag set to TRUE. As such, MiCheckVirtualAddress returns MM_NOACCESS for the VAD (even though one is found) and the demand zero fault results in an access violation. Double-checked with Windows and this is the right behavior. 2) MiCheckVirtualAddress now supports non-commited reserve VADs (ie: trying to access MEM_RESERVE memory). It used to ASSERT, now it returns MM_NOACCESS so an access violation is raised. Before change #1, this would also happen if MEM_COMMIT was later performed on the ranges, but this is now fixed. 3) When calling MiResolveDemandZeroFault, we should not make the PDE a demand zero PDE. This is senseless. The whole point is that the PDE does exist, and MiInitializePfn needs it to keep track of the page table allocation. Removed the nonsensical line of code which performed cleard the PDE during a demand-zero fault. I am able to boot to 3rd stage with these changes, so I have seen no regressions. Additionally, with these changes, the as-of-yet-uncommitted VAD-based Virtual Memory code completes 1st stage setup successfully, instead of instantly crashing on boot. svn path=/trunk/; revision=55894 --- reactos/ntoskrnl/mm/ARM3/pagfault.c | 36 +++++++++++++++++------------ 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/reactos/ntoskrnl/mm/ARM3/pagfault.c b/reactos/ntoskrnl/mm/ARM3/pagfault.c index 478725b7e4e..a7c06a52538 100644 --- a/reactos/ntoskrnl/mm/ARM3/pagfault.c +++ b/reactos/ntoskrnl/mm/ARM3/pagfault.c @@ -76,11 +76,19 @@ MiCheckVirtualAddress(IN PVOID VirtualAddress, ASSERT(Vad->u.VadFlags.VadType == VadNone); /* Check if it's a section, or just an allocation */ - if (Vad->u.VadFlags.PrivateMemory == TRUE) + if (Vad->u.VadFlags.PrivateMemory) { /* This must be a TEB/PEB VAD */ - ASSERT(Vad->u.VadFlags.MemCommit == TRUE); - *ProtectCode = (ULONG)Vad->u.VadFlags.Protection; + if (Vad->u.VadFlags.MemCommit) + { + /* It's committed, so return the VAD protection */ + *ProtectCode = (ULONG)Vad->u.VadFlags.Protection; + } + else + { + /* It has not yet been committed, so return no access */ + *ProtectCode = MM_NOACCESS; + } return NULL; } else @@ -349,7 +357,7 @@ MiResolveDemandZeroFault(IN PVOID Address, // // It's all good now // - DPRINT("Paged pool page has now been paged in\n"); + DPRINT("Demand zero page has now been paged in\n"); return STATUS_PAGE_FAULT_DEMAND_ZERO; } @@ -679,7 +687,6 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, PMMVAD Vad; PFN_NUMBER PageFrameIndex; ULONG Color; - DPRINT("ARM3 FAULT AT: %p\n", Address); /* Check for page fault on high IRQL */ @@ -887,15 +894,6 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, ASSERT(PointerPde->u.Hard.LargePage == 0); #endif - /* Check if this address range belongs to a valid allocation (VAD) */ - ProtoPte = MiCheckVirtualAddress(Address, &ProtectionCode, &Vad); - if (ProtectionCode == MM_NOACCESS) - { - /* This is a bogus VA */ - MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread); - return STATUS_ACCESS_VIOLATION; - } - #if (_MI_PAGING_LEVELS == 4) /* Check if the PXE is valid */ if (PointerPxe->u.Hard.Valid == 0) @@ -966,7 +964,6 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, if (TempPte.u.Long == (MM_READWRITE << MM_PTE_SOFTWARE_PROTECTION_BITS)) { /* Resolve the fault */ - MI_WRITE_INVALID_PDE(PointerPde, DemandZeroPde); MiResolveDemandZeroFault(Address, (ULONG)PointerPte->u.Soft.Protection, CurrentProcess, @@ -979,6 +976,15 @@ MmArmAccessFault(IN BOOLEAN StoreInstruction, /* Make sure it's not a prototype PTE */ ASSERT(TempPte.u.Soft.Prototype == 0); + + /* Check if this address range belongs to a valid allocation (VAD) */ + ProtoPte = MiCheckVirtualAddress(Address, &ProtectionCode, &Vad); + if (ProtectionCode == MM_NOACCESS) + { + /* This is a bogus VA */ + MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread); + return STATUS_ACCESS_VIOLATION; + } /* Check for non-demand zero PTE */ if (TempPte.u.Long != 0)