29 Aug
2013
29 Aug
'13
7:33 a.m.
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=…
==============================================================================
--- 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.…
==============================================================================
--- 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/window…
==============================================================================
--- 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;