Author: ion Date: Thu Aug 29 07:33:10 2013 New Revision: 59868
URL: http://svn.reactos.org/svn/reactos?rev=59868&view=rev Log: CORE-6639 #resolve #time 1d #comment Guard pages now work ;-) [NDK]: Fix definition of a global flag. [RTL]: RtlpCreateUserStack: 1) If the image is invalid, bail out. 2) If the stack commit is higher than reserve, adjust reserve. 3) After allocating a guard page, the stack limit is now ABOVE the guard page, not BELOW it (stack grows backward!). [RTL]: Remove the hack which always Commited the "StackReserve" value. Threads now have a 4-64KB stack, instead of a 1MB stack. [NTOSKRNL]: Implement MiAccessCheck and MiCheckForUserStackOverflow, which handle guard page + stack expansion. [USER32]: Because threads now correctly run with a smaller stack than usual (and expand as needed), remove some checks in user-mode callbacks which forced larger stacks.
Modified: trunk/reactos/include/ndk/pstypes.h trunk/reactos/lib/rtl/thread.c trunk/reactos/ntoskrnl/mm/ARM3/pagfault.c trunk/reactos/win32ss/user/user32/windows/message.c
Modified: trunk/reactos/include/ndk/pstypes.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/pstypes.h?rev=5... ============================================================================== --- trunk/reactos/include/ndk/pstypes.h [iso-8859-1] (original) +++ trunk/reactos/include/ndk/pstypes.h [iso-8859-1] Thu Aug 29 07:33:10 2013 @@ -69,7 +69,7 @@ #define FLG_KERNEL_STACK_TRACE_DB 0x00002000 #define FLG_MAINTAIN_OBJECT_TYPELIST 0x00004000 #define FLG_HEAP_ENABLE_TAG_BY_DLL 0x00008000 -#define FLG_IGNORE_DEBUG_PRIV 0x00010000 +#define FLG_DISABLE_STACK_EXTENSION 0x00010000 #define FLG_ENABLE_CSRDEBUG 0x00020000 #define FLG_ENABLE_KDEBUG_SYMBOL_LOAD 0x00040000 #define FLG_DISABLE_PAGE_KERNEL_STACKS 0x00080000
Modified: trunk/reactos/lib/rtl/thread.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/thread.c?rev=59868&... ============================================================================== --- trunk/reactos/lib/rtl/thread.c [iso-8859-1] (original) +++ trunk/reactos/lib/rtl/thread.c [iso-8859-1] Thu Aug 29 07:33:10 2013 @@ -46,6 +46,7 @@ { /* Get the Image Headers */ Headers = RtlImageNtHeader(NtCurrentPeb()->ImageBaseAddress); + if (!Headers) return STATUS_INVALID_IMAGE_FORMAT;
/* If we didn't get the parameters, find them ourselves */ if (!StackReserve) StackReserve = Headers->OptionalHeader. @@ -60,14 +61,16 @@ if (!StackCommit) StackCommit = SystemBasicInfo.PageSize; }
+ /* Check if the commit is higher than the reserve*/ + if (StackCommit >= StackReserve) + { + /* Grow the reserve beyond the commit, up to 1MB alignment */ + StackReserve = ROUND_UP(StackCommit, 1024 * 1024); + } + /* Align everything to Page Size */ StackReserve = ROUND_UP(StackReserve, SystemBasicInfo.AllocationGranularity); StackCommit = ROUND_UP(StackCommit, SystemBasicInfo.PageSize); - - // FIXME: Remove once Guard Page support is here - #if 1 - StackCommit = StackReserve; - #endif
/* Reserve memory for the stack */ Status = ZwAllocateVirtualMemory(hProcess, @@ -121,7 +124,7 @@ if (!NT_SUCCESS(Status)) return Status;
/* Update the Stack Limit keeping in mind the Guard Page */ - InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit - + InitialTeb->StackLimit = (PVOID)((ULONG_PTR)InitialTeb->StackLimit + GuardPageSize); }
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] Thu Aug 29 07:33:10 2013 @@ -23,6 +23,162 @@ #endif
/* PRIVATE FUNCTIONS **********************************************************/ + +NTSTATUS +NTAPI +MiCheckForUserStackOverflow(IN PVOID Address, + IN PVOID TrapInformation) +{ + PETHREAD CurrentThread = PsGetCurrentThread(); + PTEB Teb = CurrentThread->Tcb.Teb; + PVOID StackBase, DeallocationStack, NextStackAddress; + ULONG GuranteedSize; + NTSTATUS Status; + + /* Do we own the address space lock? */ + if (CurrentThread->AddressSpaceOwner == 1) + { + /* This isn't valid */ + DPRINT1("Process owns address space lock\n"); + ASSERT(KeAreAllApcsDisabled() == TRUE); + return STATUS_GUARD_PAGE_VIOLATION; + } + + /* Are we attached? */ + if (KeIsAttachedProcess()) + { + /* This isn't valid */ + DPRINT1("Process is attached\n"); + return STATUS_GUARD_PAGE_VIOLATION; + } + + /* Read the current settings */ + StackBase = Teb->NtTib.StackBase; + DeallocationStack = Teb->DeallocationStack; + GuranteedSize = Teb->GuaranteedStackBytes; + DPRINT1("Handling guard page fault with Stacks Addresses 0x%p and 0x%p, guarantee: %lx\n", + StackBase, DeallocationStack, GuranteedSize); + + /* Guarantees make this code harder, for now, assume there aren't any */ + ASSERT(GuranteedSize == 0); + + /* So allocate only the minimum guard page size */ + GuranteedSize = PAGE_SIZE; + + /* Does this faulting stack address actually exist in the stack? */ + if ((Address >= StackBase) || (Address < DeallocationStack)) + { + /* That's odd... */ + DPRINT1("Faulting address outside of stack bounds\n"); + return STATUS_GUARD_PAGE_VIOLATION; + } + + /* This is where the stack will start now */ + NextStackAddress = (PVOID)((ULONG_PTR)PAGE_ALIGN(Address) - GuranteedSize); + + /* Do we have at least one page between here and the end of the stack? */ + if (((ULONG_PTR)NextStackAddress - PAGE_SIZE) <= (ULONG_PTR)DeallocationStack) + { + /* We don't -- Windows would try to make this guard page valid now */ + DPRINT1("Close to our death...\n"); + ASSERT(FALSE); + return STATUS_STACK_OVERFLOW; + } + + /* Don't handle this flag yet */ + ASSERT((PsGetCurrentProcess()->Peb->NtGlobalFlag & FLG_DISABLE_STACK_EXTENSION) == 0); + + /* Update the stack limit */ + Teb->NtTib.StackLimit = (PVOID)((ULONG_PTR)NextStackAddress + GuranteedSize); + + /* Now move the guard page to the next page */ + Status = ZwAllocateVirtualMemory(NtCurrentProcess(), + &NextStackAddress, + 0, + &GuranteedSize, + MEM_COMMIT, + PAGE_READWRITE | PAGE_GUARD); + if ((NT_SUCCESS(Status) || (Status == STATUS_ALREADY_COMMITTED))) + { + /* We did it! */ + DPRINT1("Guard page handled successfully for %p\n", Address); + return STATUS_PAGE_FAULT_GUARD_PAGE; + } + + /* Fail, we couldn't move the guard page */ + DPRINT1("Guard page failure: %lx\n", Status); + ASSERT(FALSE); + return STATUS_STACK_OVERFLOW; +} + +NTSTATUS +NTAPI +MiAccessCheck(IN PMMPTE PointerPte, + IN BOOLEAN StoreInstruction, + IN KPROCESSOR_MODE PreviousMode, + IN ULONG ProtectionCode, + IN PVOID TrapFrame, + IN BOOLEAN LockHeld) +{ + MMPTE TempPte; + + /* Check for invalid user-mode access */ + if ((PreviousMode == UserMode) && (PointerPte > MiHighestUserPte)) + { + return STATUS_ACCESS_VIOLATION; + } + + /* Capture the PTE -- is it valid? */ + TempPte = *PointerPte; + if (TempPte.u.Hard.Valid) + { + /* Was someone trying to write to it? */ + if (StoreInstruction) + { + /* Is it writable?*/ + if ((TempPte.u.Hard.Write) || (TempPte.u.Hard.CopyOnWrite)) + { + /* Then there's nothing to worry about */ + return STATUS_SUCCESS; + } + + /* Oops! This isn't allowed */ + return STATUS_ACCESS_VIOLATION; + } + + /* Someone was trying to read from a valid PTE, that's fine too */ + return STATUS_SUCCESS; + } + + /* Convert any fault flag to 1 only */ + if (StoreInstruction) StoreInstruction = 1; + +#if 0 + /* Check if the protection on the page allows what is being attempted */ + if ((MmReadWrite[Protection] - StoreInstruction) < 10) + { + return STATUS_ACCESS_VIOLATION; + } +#endif + + /* Check if this is a guard page */ + if (ProtectionCode & MM_DECOMMIT) + { + /* Attached processes can't expand their stack */ + if (KeIsAttachedProcess()) return STATUS_ACCESS_VIOLATION; + + /* No support for transition PTEs yet */ + ASSERT(((TempPte.u.Soft.Transition == 1) && + (TempPte.u.Soft.Prototype == 0)) == FALSE); + + /* Remove the guard page bit, and return a guard page violation */ + PointerPte->u.Soft.Protection = ProtectionCode & ~MM_DECOMMIT; + return STATUS_GUARD_PAGE_VIOLATION; + } + + /* Nothing to do */ + return STATUS_SUCCESS; +}
PMMPTE NTAPI @@ -800,7 +956,14 @@ { if (!PteContents.u.Proto.ReadOnly) { - /* FIXME: CHECK FOR ACCESS */ + /* Check for page acess in software */ + Status = MiAccessCheck(PointerProtoPte, + StoreInstruction, + KernelMode, + TempPte.u.Soft.Protection, + TrapInformation, + TRUE); + ASSERT(Status == STATUS_SUCCESS);
/* Check for copy on write page */ if ((TempPte.u.Soft.Protection & MM_WRITECOPY) == MM_WRITECOPY) @@ -1682,9 +1845,6 @@ return Status; }
- /* No guard page support yet */ - ASSERT((ProtectionCode & MM_DECOMMIT) == 0); - /* * Check if this is a real user-mode address or actually a kernel-mode * page table for a user mode address @@ -1693,6 +1853,24 @@ { /* Add an additional page table reference */ MiIncrementPageTableReferences(Address); + } + + /* Is this a guard page? */ + if (ProtectionCode & MM_DECOMMIT) + { + /* Remove the bit */ + PointerPte->u.Soft.Protection = ProtectionCode & ~MM_DECOMMIT; + + /* Not supported */ + ASSERT(ProtoPte == NULL); + ASSERT(CurrentThread->ApcNeeded == 0); + + /* Drop the working set lock */ + MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread); + ASSERT(KeGetCurrentIrql() == OldIrql); + + /* Handle stack expansion */ + return MiCheckForUserStackOverflow(Address, TrapInformation); }
/* Did we get a prototype PTE back? */ @@ -1781,8 +1959,7 @@ return STATUS_PAGE_FAULT_DEMAND_ZERO; }
- /* No guard page support yet */ - ASSERT((ProtectionCode & MM_DECOMMIT) == 0); + /* We should have a valid protection here */ ASSERT(ProtectionCode != 0x100);
/* Write the prototype PTE */ @@ -1831,7 +2008,36 @@ } }
- /* FIXME: Run MiAccessCheck */ + /* Do we have a valid protection code? */ + if (ProtectionCode != 0x100) + { + /* Run a software access check first, including to detect guard pages */ + Status = MiAccessCheck(PointerPte, + StoreInstruction, + Mode, + ProtectionCode, + TrapInformation, + FALSE); + if (Status != STATUS_SUCCESS) + { + /* Not supported */ + ASSERT(CurrentThread->ApcNeeded == 0); + + /* Drop the working set lock */ + MiUnlockProcessWorkingSet(CurrentProcess, CurrentThread); + ASSERT(KeGetCurrentIrql() == OldIrql); + + /* Did we hit a guard page? */ + if (Status == STATUS_GUARD_PAGE_VIOLATION) + { + /* Handle stack expansion */ + return MiCheckForUserStackOverflow(Address, TrapInformation); + } + + /* Otherwise, fail back to the caller directly */ + return Status; + } + }
/* Dispatch the fault */ Status = MiDispatchFault(StoreInstruction,
Modified: trunk/reactos/win32ss/user/user32/windows/message.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/user32/windows... ============================================================================== --- trunk/reactos/win32ss/user/user32/windows/message.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/user32/windows/message.c [iso-8859-1] Thu Aug 29 07:33:10 2013 @@ -1288,7 +1288,6 @@ { MSG AnsiMsg; MSG UnicodeMsg; - ULONG_PTR LowLimit; BOOL Hook = FALSE, MsgOverride = FALSE, Dialog; LRESULT Result = 0, PreResult = 0; DWORD Data = 0; @@ -1297,14 +1296,6 @@ { WARN("IntCallWindowsProcW() called with WndProc = NULL!\n"); return FALSE; - } - - // Safeguard against excessive recursions. - LowLimit = (ULONG_PTR)NtCurrentTeb()->NtTib.StackLimit; - if (((ULONG_PTR)&lParam - LowLimit) < PAGE_SIZE ) - { - ERR("IntCallWindowsProcW() Exceeded Stack!\n"); - return FALSE; }
if (pWnd) @@ -2766,7 +2757,6 @@ PWINDOWPROC_CALLBACK_ARGUMENTS CallbackArgs; MSG KMMsg, UMMsg; PWND pWnd = NULL; - ULONG_PTR LowLimit; PCLIENTINFO pci = GetWin32ClientInfo();
/* Make sure we don't try to access mem beyond what we were given */ @@ -2774,13 +2764,6 @@ { return STATUS_INFO_LENGTH_MISMATCH; } - - LowLimit = (ULONG_PTR)NtCurrentTeb()->NtTib.StackLimit; - if (((ULONG_PTR)&ArgumentLength - LowLimit) < PAGE_SIZE ) - { - ERR("Callback from Win32k Exceeded Stack!\n"); - return STATUS_BAD_STACK; - }
CallbackArgs = (PWINDOWPROC_CALLBACK_ARGUMENTS) Arguments; KMMsg.hwnd = CallbackArgs->Wnd;