[fireball] 29244: - Fix multiple bugs in RtlWalkFrameChain and secure it against any possibility of a bugcheck while walking the stack. - Fix bugs in RtlUnwind and RtlExceptionDispatch which assumed the DPC stack size was 4KB instead of 12KB. - Fix multiple bugs in RtlpGetStackLimits and seure it against bugchecks. Properly detect DPC or invalid stacks. - PsConvertToGuiThread should acquire a guarded region, not a critical section, to stop all APCs. - Fix bug in bugzilla reporting which was maki

Author: fireball
Date: Thu Sep 27 17:07:43 2007
New Revision: 29244

URL: http://svn.reactos.org/svn/reactos?rev=29244&view=rev
Log:
- Fix multiple bugs in RtlWalkFrameChain and secure it against any possibility of a
bugcheck while walking the stack.
- Fix bugs in RtlUnwind and RtlExceptionDispatch which assumed the DPC stack size was 4KB
instead of 12KB.
- Fix multiple bugs in RtlpGetStackLimits and seure it against bugchecks. Properly detect
DPC or invalid stacks.
- PsConvertToGuiThread should acquire a guarded region, not a critical section, to stop
all APCs.
- Fix bug in bugzilla reporting which was making things crash.
- Unlock address space before raising to HIGH_LEVEL in KeBugCheck.
- Display blue screen at APC_LEVEL, to avoid the assertion in procobj.c when trying to
attach to csrss. This should fix the recursive bugchecking when the GUI is up, and also
take down the GUI properly. The fix is a hack.
- Fix bogus implementation of IoGetStackLimits and make it work properly.
- Make MmCreateKernelStack return the base of the stack, not the limit, and fix all
callers appropriately.
- Remove svn:needs-lock properties of various files, whose contents either changes too
often or whose contents is definately clean.

Bugreports and information - by Alex.

Modified:
    trunk/reactos/lib/rtl/exception.c
    trunk/reactos/ntoskrnl/io/iomgr/util.c
    trunk/reactos/ntoskrnl/ke/bug.c   (contents, props changed)
    trunk/reactos/ntoskrnl/ke/i386/abios.c   (props changed)
    trunk/reactos/ntoskrnl/ke/i386/exp.c   (props changed)
    trunk/reactos/ntoskrnl/ke/i386/kiinit.c   (contents, props changed)
    trunk/reactos/ntoskrnl/ke/i386/ldt.c   (props changed)
    trunk/reactos/ntoskrnl/ke/i386/thrdini.c   (props changed)
    trunk/reactos/ntoskrnl/ke/thrdobj.c
    trunk/reactos/ntoskrnl/mm/procsup.c
    trunk/reactos/ntoskrnl/ps/win32.c
    trunk/reactos/ntoskrnl/rtl/libsupp.c   (contents, props changed)
    trunk/reactos/ntoskrnl/rtl/misc.c   (props changed)
    trunk/reactos/ntoskrnl/rtl/strtok.c   (props changed)

Modified: trunk/reactos/lib/rtl/exception.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/rtl/exception.c?rev=29…
==============================================================================
--- trunk/reactos/lib/rtl/exception.c (original)
+++ trunk/reactos/lib/rtl/exception.c Thu Sep 27 17:07:43 2007
@@ -122,12 +122,11 @@
                   IN ULONG Count,
                   IN ULONG Flags)
 {
-    PULONG Stack, NewStack;
+    ULONG_PTR Stack, NewStack, StackBegin, StackEnd;
     ULONG Eip;
-    ULONG_PTR StackBegin, StackEnd;
     BOOLEAN Result, StopSearch = FALSE;
     ULONG i = 0;
-
+    
     /* Get current EBP */
 #if defined(_M_IX86)
 #if defined __GNUC__
@@ -136,16 +135,16 @@
     __asm mov Stack, ebp
 #endif
 #elif defined(_M_MIPS)
-    __asm__("move $sp, %0" : "=r" (Stack) : );
+        __asm__("move $sp, %0" : "=r" (Stack) : );
 #elif defined(_M_PPC)
     __asm__("mr %0,1" : "=r" (Stack) : );
 #else
 #error Unknown architecture
 #endif
-
+    
     /* Set it as the stack begin limit as well */
     StackBegin = (ULONG_PTR)Stack;
-
+    
     /* Check if we're called for non-logging mode */
     if (!Flags)
     {
@@ -155,52 +154,65 @@
                                         &StackEnd);
         if (!Result) return 0;
     }
-
-    /* Loop the frames */
-    for (i = 0; i < Count; i++)
-    {
-        /* Check if we're past the stack */
-        if ((ULONG_PTR)Stack >= StackEnd) break;
-
-        /* Check if this is the first entry */
-#if 0
-        if (!i)
+    
+    /* Use a SEH block for maximum protection */
+    _SEH_TRY
+    {
+        /* Loop the frames */
+        for (i = 0; i < Count; i++)
         {
-            if ((ULONG_PTR)Stack != StackBegin) break;
+            /*
+             * Leave if we're past the stack,
+             * if we're before the stack,
+             * or if we've reached ourselves.
+             */
+            if ((Stack >= StackEnd) ||
+                (!i ? (Stack < StackBegin) : (Stack <= StackBegin)) ||
+                ((StackEnd - Stack) < (2 * sizeof(ULONG_PTR))))
+            {
+                /* We're done or hit a bad address */
+                break;
+            }
+            
+            /* Get new stack and EIP */
+            NewStack = *(PULONG_PTR)Stack;
+            Eip = *(PULONG_PTR)(Stack + sizeof(ULONG_PTR));
+            
+            /* Check if the new pointer is above the oldone and past the end */
+            if (!((Stack < NewStack) && (NewStack < StackEnd)))
+            {
+                /* Stop searching after this entry */
+                StopSearch = TRUE;
+            }
+            
+            /* Also make sure that the EIP isn't a stack address */
+            if ((StackBegin < Eip) && (Eip < StackEnd)) break;
+            
+            /* Check if we reached a user-mode address */
+            if (!(Flags) && !(Eip & 0x80000000)) break;
+            
+            /* Save this frame */
+            Callers[i] = (PVOID)Eip;
+            
+            /* Check if we should continue */
+            if (StopSearch)
+            {
+                /* Return the next index */
+                i++;
+                break;
+            }
+            
+            /* Move to the next stack */
+            Stack = NewStack;
         }
-        else
-        {
-            if ((ULONG_PTR)Stack == StackBegin) break;
-        }
-#endif
-
-        /* Make sure there's enough frames */
-        if ((StackEnd - (ULONG_PTR)Stack) < (2 * sizeof(ULONG_PTR))) break;
-
-        /* Get new stack and EIP */
-        NewStack = (PULONG)Stack[0];
-        Eip = Stack[1];
-
-        /* Check if the new pointer is above the oldone and past the end */
-        if (!((Stack < NewStack) && ((ULONG_PTR)NewStack < StackEnd)))
-        {
-            /* Stop searching after this entry */
-            StopSearch = TRUE;
-        }
-
-        /* Also make sure that the EIP isn't a stack address */
-        if ((StackBegin < Eip) && (Eip < StackEnd)) break;
-
-        /* Save this frame */
-        Callers[i] = (PVOID)Eip;
-
-        /* Check if we should continue */
-        if (StopSearch) break;
-
-        /* Move to the next stack */
-        Stack = NewStack;
-    }
-
+    }
+    _SEH_HANDLE
+    {
+        /* No index */
+        i = 0;
+    }
+    _SEH_END;
+        
     /* Return frames parsed */
     return i;
 }

Modified: trunk/reactos/ntoskrnl/io/iomgr/util.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/util.c?r…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/util.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/util.c Thu Sep 27 17:07:43 2007
@@ -12,6 +12,11 @@
 #define NDEBUG
 #include <internal/debug.h>
 
+VOID
+NTAPI
+RtlpGetStackLimits(PULONG_PTR StackBase,
+                   PULONG_PTR StackLimit);
+
 /* DATA **********************************************************************/
 
 KSPIN_LOCK CancelSpinLock;
@@ -48,10 +53,33 @@
 IoGetStackLimits(OUT PULONG LowLimit,
                  OUT PULONG HighLimit)
 {
-    /* Return the limits from the TEB... this is wrong! */
-    DPRINT1("FIXME: IoGetStackLimits returning B*LLSHIT!\n");
-    *LowLimit = (ULONG)NtCurrentTeb()->Tib.StackLimit;
-    *HighLimit = (ULONG)NtCurrentTeb()->Tib.StackBase;
+    PKPRCB Prcb = KeGetCurrentPrcb();
+    ULONG_PTR DpcStack = (ULONG_PTR)(Prcb->DpcStack);
+    volatile ULONG_PTR StackAddress;
+
+    /* Save our stack address so we always know it's valid */
+    StackAddress = (ULONG_PTR)(&StackAddress);
+    
+    /* Get stack values */
+    RtlpGetStackLimits(LowLimit, HighLimit);
+
+    /* Check if we're outside the stack */
+    if ((StackAddress < *LowLimit) || (StackAddress > *HighLimit))
+    {
+        /* Check if we may be in a DPC */
+        if (KeGetCurrentIrql() >= DISPATCH_LEVEL)
+        {
+            /* Check if we really are in a DPC */
+            if ((Prcb->DpcRoutineActive) &&
+                (StackAddress <= DpcStack) &&
+                (StackAddress >= DpcStack - KERNEL_STACK_SIZE))
+            {
+                /* Use the DPC stack limits */
+                *HighLimit = DpcStack;
+                *LowLimit = DpcStack - KERNEL_STACK_SIZE;
+            }
+        }
+    }
 }
 
 /*

Modified: trunk/reactos/ntoskrnl/ke/bug.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/bug.c?rev=2924…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/bug.c (original)
+++ trunk/reactos/ntoskrnl/ke/bug.c Thu Sep 27 17:07:43 2007
@@ -222,9 +222,7 @@
              "BIOS Version: %wZ\n"
              "Video BIOS Date: %wZ\n"
              "Video BIOS Version: %wZ\n"
-             "Memory: %d\n"
-             "NT Build Number: %lx\n"
-             "NT Build Lab: %s\n",
+             "Memory: %d\n",
              KeProcessorArchitecture,
              KeFeatureBits,
              KiFastSystemCallDisable,
@@ -251,9 +249,7 @@
              &KeRosBiosVersion,
              &KeRosVideoBiosDate,
              &KeRosVideoBiosVersion,
-             MmStats.NrTotalPages * PAGE_SIZE,
-             NtBuildNumber,
-             NtBuildLab);
+             MmStats.NrTotalPages * PAGE_SIZE);
 #endif
 }
 
@@ -640,7 +636,6 @@
     PVOID DriverBase;
     PLDR_DATA_TABLE_ENTRY LdrEntry;
     PULONG_PTR HardErrorParameters;
-    KIRQL OldIrql;
 #ifdef CONFIG_SMP
     LONG i = 0;
 #endif
@@ -976,15 +971,15 @@
         }
     }
 
-    /* Raise IRQL to HIGH_LEVEL */
-    _disable();
-    KeRaiseIrql(HIGH_LEVEL, &OldIrql);
-
     /* ROS HACK: Unlock the Kernel Address Space if we own it */
     if (KernelAddressSpaceLock.Owner == KeGetCurrentThread())
     {
         MmUnlockAddressSpace(MmGetKernelAddressSpace());
     }
+    
+    /* Raise IRQL to HIGH_LEVEL */    
+    _disable();
+    KfRaiseIrql(HIGH_LEVEL);
 
     /* Avoid recursion */
     if (!InterlockedDecrement((PLONG)&KeBugCheckCount))
@@ -1006,11 +1001,13 @@
 #endif
 
         /* Display the BSOD */
+        KfLowerIrql(APC_LEVEL); // This is a nastier hack than any ever before
         KiDisplayBlueScreen(MessageId,
                             IsHardError,
                             HardErrCaption,
                             HardErrMessage,
                             AnsiName);
+        KfRaiseIrql(HIGH_LEVEL);
 
         /* Check if the debugger is disabled but we can enable it */
         if (!(KdDebuggerEnabled) && !(KdPitchDebugger))

Propchange: trunk/reactos/ntoskrnl/ke/bug.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/ke/i386/abios.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/ke/i386/exp.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Modified: trunk/reactos/ntoskrnl/ke/i386/kiinit.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/kiinit.c?…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/kiinit.c (original)
+++ trunk/reactos/ntoskrnl/ke/i386/kiinit.c Thu Sep 27 17:07:43 2007
@@ -582,7 +582,7 @@
         /* Allocate the DPC Stack */
         DpcStack = MmCreateKernelStack(FALSE, 0);
         if (!DpcStack) KeBugCheckEx(NO_PAGES_AVAILABLE, 1, 0, 0, 0);
-        Prcb->DpcStack = (PVOID)((ULONG_PTR)DpcStack + KERNEL_STACK_SIZE);
+        Prcb->DpcStack = DpcStack;
 
         /* Allocate the IOPM save area. */
         Ki386IopmSaveArea = ExAllocatePoolWithTag(PagedPool,

Propchange: trunk/reactos/ntoskrnl/ke/i386/kiinit.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/ke/i386/ldt.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/ke/i386/thrdini.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Modified: trunk/reactos/ntoskrnl/ke/thrdobj.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/thrdobj.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/thrdobj.c (original)
+++ trunk/reactos/ntoskrnl/ke/thrdobj.c Thu Sep 27 17:07:43 2007
@@ -797,8 +797,7 @@
     if (!KernelStack)
     {
         /* We don't, allocate one */
-        KernelStack = (PVOID)((ULONG_PTR)MmCreateKernelStack(FALSE, 0) +
-                              KERNEL_STACK_SIZE);
+        KernelStack = MmCreateKernelStack(FALSE, 0);
         if (!KernelStack) return STATUS_INSUFFICIENT_RESOURCES;
 
         /* Remember for later */
@@ -806,8 +805,8 @@
     }
 
     /* Set the Thread Stacks */
-    Thread->InitialStack = (PCHAR)KernelStack;
-    Thread->StackBase = (PCHAR)KernelStack;
+    Thread->InitialStack = KernelStack;
+    Thread->StackBase = KernelStack;
     Thread->StackLimit = (ULONG_PTR)KernelStack - KERNEL_STACK_SIZE;
     Thread->KernelStackResident = TRUE;
 

Modified: trunk/reactos/ntoskrnl/mm/procsup.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/procsup.c?rev=…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/procsup.c (original)
+++ trunk/reactos/ntoskrnl/mm/procsup.c Thu Sep 27 17:07:43 2007
@@ -242,8 +242,9 @@
         KEBUGCHECK(0);
     }
 
-    /* Return the stack */
-    return KernelStack;
+    /* Return the stack base */
+    return (PVOID)((ULONG_PTR)KernelStack +
+                   (GuiStack ? KERNEL_LARGE_STACK_SIZE : KERNEL_STACK_SIZE));
 }
 
 /*

Modified: trunk/reactos/ntoskrnl/ps/win32.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/win32.c?rev=29…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/win32.c (original)
+++ trunk/reactos/ntoskrnl/ps/win32.c Thu Sep 27 17:07:43 2007
@@ -52,8 +52,7 @@
     if (!Thread->Tcb.LargeStack)
     {
         /* We don't create one */
-        NewStack = (ULONG_PTR)MmCreateKernelStack(TRUE, 0) +
-                    KERNEL_LARGE_STACK_SIZE;
+        NewStack = (ULONG_PTR)MmCreateKernelStack(TRUE, 0);
         if (!NewStack)
         {
             /* Panic in user-mode */
@@ -61,15 +60,15 @@
             return STATUS_NO_MEMORY;
         }
 
-        /* We're about to switch stacks. Enter a critical region */
-        KeEnterCriticalRegion();
+        /* We're about to switch stacks. Enter a guarded region */
+        KeEnterGuardedRegion();
 
         /* Switch stacks */
         OldStack = KeSwitchKernelStack((PVOID)NewStack,
                                        (PVOID)(NewStack - KERNEL_STACK_SIZE));
 
-        /* Leave the critical region */
-        KeLeaveCriticalRegion();
+        /* Leave the guarded region */
+        KeLeaveGuardedRegion();
 
         /* Delete the old stack */
         MmDeleteKernelStack(OldStack, FALSE);

Modified: trunk/reactos/ntoskrnl/rtl/libsupp.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/rtl/libsupp.c?rev…
==============================================================================
--- trunk/reactos/ntoskrnl/rtl/libsupp.c (original)
+++ trunk/reactos/ntoskrnl/rtl/libsupp.c Thu Sep 27 17:07:43 2007
@@ -197,11 +197,11 @@
         /* Check if we are in a DPC and the stack matches */
         if ((Prcb->DpcRoutineActive) &&
             (RegistrationFrameEnd <= DpcStack) &&
-            ((ULONG_PTR)RegistrationFrame >= DpcStack - 4096))
+            ((ULONG_PTR)RegistrationFrame >= DpcStack - KERNEL_STACK_SIZE))
         {
             /* Update the limits to the DPC Stack's */
             *StackHigh = DpcStack;
-            *StackLow = DpcStack - 4096;
+            *StackLow = DpcStack - KERNEL_STACK_SIZE;
             return TRUE;
         }
     }
@@ -218,18 +218,37 @@
 {
     PKTHREAD Thread = KeGetCurrentThread();
 
-    /* FIXME: Super native implementation */
-
+    /* Don't even try at ISR level or later */
+    if (KeGetCurrentIrql() > DISPATCH_LEVEL) return FALSE;
+    
     /* Start with defaults */
     *StackBegin = Thread->StackLimit;
     *StackEnd = (ULONG_PTR)Thread->StackBase;
-
-    /* Check if we seem to be on the DPC stack */
-    if ((*StackBegin > Ebp) || (Ebp > *StackEnd))
+    
+    /* Check if EBP is inside the stack */
+    if ((*StackBegin <= Ebp) && (Ebp <= *StackEnd))
     {
-        /* FIXME: TODO */
-        //ASSERT(FALSE);
-        DPRINT1("Stacks: %p %p %p\n", Ebp, *StackBegin, *StackEnd);
+        /* Then make the stack start at EBP */
+        *StackBegin = Ebp;
+    }
+    else
+    {
+        /* Now we're going to assume we're on the DPC stack */
+        *StackEnd = (ULONG_PTR)(KeGetPcr()->Prcb->DpcStack);
+        *StackBegin = *StackEnd - KERNEL_STACK_SIZE;
+        
+        /* Check if we seem to be on the DPC stack */
+        if ((*StackEnd) && (*StackBegin < Ebp) && (Ebp <=
*StackEnd))
+        {
+            /* We're on the DPC stack */
+            *StackBegin = Ebp;
+        }
+        else
+        {
+            /* We're somewhere else entirely... use EBP for safety */
+            *StackBegin = Ebp;
+            *StackEnd = (ULONG_PTR)PAGE_ALIGN(*StackBegin);
+        }
     }
 
     /* Return success */

Propchange: trunk/reactos/ntoskrnl/rtl/libsupp.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/rtl/misc.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*

Propchange: trunk/reactos/ntoskrnl/rtl/strtok.c
------------------------------------------------------------------------------
--- svn:needs-lock (original)
+++ svn:needs-lock (removed)
@@ -1,1 +1,0 @@
-*