- Fix a bug in KeRaiseUserExceptionDispatcher which was causing us not to set the Exception Address (EIP) in the EH record.

- Fix a terrible stack corruption bug in KeRaiseUserExceptionDispatcher which was causing us to eventually fuck up the stack in user mode (0x14 bytes instead of 0x50 bytes were reserved).
- Protect User-mode Callbacks with SEH, and use STATUS_POP_CALLBACK_STACK.
- Fix another nasty stack corruption bug in user-mode APC delivery.
- Protect User-mode APC delivery with SEH.
- Fix SEH handlers to return EXCEPTION_EXECUTE_HANDLER isntead of ExceptionContinueSearch.

svn path=/trunk/; revision=24173

reactos/dll/ntdll/main/i386/dispatch.S

@@ -11,17 +11,9 @@
 #include <ndk/asm.h>
 .intel_syntax noprefix
 
-#define EXCEPTION_NONCONTINUABLE    1
-#define EXCEPTION_UNWINDING         2
-#define EXCEPTION_EXIT_UNWIND       4
-#define EXCEPTION_UNWIND            (EXCEPTION_UNWINDING + EXCEPTION_EXIT_UNWIND)
-
-#define STATUS_CALLBACK_POP_STACK   0xC0000423
-
-#define ExceptionContinueSearch     1
-
 /* FUNCTIONS ****************************************************************/
 
+.func LdrInitializeThunk@16
 .globl _LdrInitializeThunk@16
 _LdrInitializeThunk@16:
 
@@ -36,44 +28,72 @@ _LdrInitializeThunk@16:
 
     /* Jump into the C initialization routine */
     jmp _LdrpInit@12
+.endfunc
 
-.globl _KiUserExceptionApcHandler@16
-_KiUserApcExceptionHandler@16:
+.func KiUserApcExceptionHandler
+_KiUserApcExceptionHandler:
 
     /* Put the exception record in ECX and check the Flags */
     mov ecx, [esp+4]
-    test dword ptr [ecx+EXCEPTION_RECORD_EXCEPTION_FLAGS], EXCEPTION_UNWIND
+    test dword ptr [ecx+EXCEPTION_RECORD_EXCEPTION_FLAGS], EXCEPTION_EXIT_UNWIND + EXCEPTION_UNWINDING
     jz .return
 
     /* Test alert the thread */
     call _NtTestAlert@0
 
 .return:
-    /* We'll continue */
-    mov eax, ExceptionContinueSearch
+    /* We'll execute handler */
+    mov eax, EXCEPTION_EXECUTE_HANDLER
     ret 16
-    
+.endfunc
+
+.func KiUserApcDispatcher@16
 .globl _KiUserApcDispatcher@16
 _KiUserApcDispatcher@16:
 
-    /* Put the Context in EDI */
-    lea edi, [esp+16]
+    /* Setup SEH stack */
+    lea eax, [esp+CONTEXT_ALIGNED_SIZE+16]
+    mov ecx, fs:[TEB_EXCEPTION_LIST]
+    mov edx, offset _KiUserApcExceptionHandler
+    mov [eax], ecx
+    mov [eax+4], edx
 
-    /* Get the ApcRoutine and call it */
+    /* Enable SEH */
+    mov fs:[TEB_EXCEPTION_LIST], eax
+
+    /* Put the Context in EDI */
     pop eax
+    lea edi, [esp+12]
+
+    /* Call the APC Routine */
     call eax
 
+    /* Restore exception list */
+    mov ecx, [edi+CONTEXT_ALIGNED_SIZE]
+    mov fs:[TEB_EXCEPTION_LIST], ecx
+
     /* Switch back to the context */
     push 1
     push edi
     call _ZwContinue@8
 
-.globl _KiUserCallbackExceptionHandler@16
-_KiUserCallbackExceptionHandler@16:
+    /* Save callback return value */
+    mov esi, eax
+
+    /* Raise status */
+StatusRaiseApc:
+    push esi
+    call _RtlRaiseStatus@4
+    jmp StatusRaiseApc
+    ret 16
+.endfunc
+
+.func KiUserCallbackExceptionHandler
+_KiUserCallbackExceptionHandler:
 
     /* Put the exception record in ECX and check the Flags */
     mov ecx, [esp+4]
-    test dword ptr [ecx+EXCEPTION_RECORD_EXCEPTION_FLAGS], EXCEPTION_UNWIND
+    test dword ptr [ecx+EXCEPTION_RECORD_EXCEPTION_FLAGS], EXCEPTION_EXIT_UNWIND + EXCEPTION_UNWINDING
     jz return
 
     /* Tell the kernel to invalidate the stack */
@@ -83,13 +103,25 @@ _KiUserCallbackExceptionHandler@16:
     call _ZwCallbackReturn@12
 
 return:
-    /* We'll continue */
-    mov eax, ExceptionContinueSearch
+    /* We'll execute the handler */
+    mov eax, EXCEPTION_EXECUTE_HANDLER
     ret 16
+.endfunc
 
+.func KiUserCallbackDispatcher@12
 .globl _KiUserCallbackDispatcher@12
 _KiUserCallbackDispatcher@12:
-    
+
+    /* Setup SEH stack */
+    mov ecx, fs:[TEB_EXCEPTION_LIST]
+    mov edx, offset _KiUserCallbackExceptionHandler
+    lea eax, [esp+16]
+    mov [esp+16], ecx
+    mov [esp+20], edx
+
+    /* Enable SEH */
+    mov fs:[TEB_EXCEPTION_LIST], eax
+
     /* Get the callback Index */
     add esp, 4
     pop edx
@@ -107,15 +139,28 @@ _KiUserCallbackDispatcher@12:
     push 0
     call _ZwCallbackReturn@12
 
+    /* Save callback return value */
+    mov esi, eax
+
+    /* Raise status */
+StatusRaise:
+    push esi
+    call _RtlRaiseStatus@4
+    jmp StatusRaise
+    ret 12
+.endfunc
+
+.func KiRaiseUserExceptionDispatcher@0
 .globl _KiRaiseUserExceptionDispatcher@0
 _KiRaiseUserExceptionDispatcher@0:
 
     /* Setup stack for EXCEPTION_RECORD */
     push ebp
     mov ebp, esp
-    sub esp, SIZEOF_EXCEPTION_RECORD
+    sub esp, EXCEPTION_RECORD_LENGTH
 
     /* Fill out the record */
+    mov [esp+EXCEPTION_RECORD_EXCEPTION_ADDRESS], eax
     mov eax, [fs:KPCR_TEB]
     mov eax, [eax+TEB_EXCEPTION_CODE]
     mov [esp+EXCEPTION_RECORD_EXCEPTION_CODE], eax
@@ -132,7 +177,9 @@ _KiRaiseUserExceptionDispatcher@0:
     mov esp, ebp
     pop ebp
     ret
+.endfunc
 
+.func KiUserExceptionDispatcher@8
 .globl _KiUserExceptionDispatcher@8
 _KiUserExceptionDispatcher@8:
 
@@ -140,17 +187,9 @@ _KiUserExceptionDispatcher@8:
     mov ecx, [esp+4]
     mov ebx, [esp]
 
-    /* Call the vectored exception handler */
+    /* Dispatch the exception */
     push ecx
     push ebx
-    call _RtlpExecuteVectoredExceptionHandlers@8
-
-    /* Check for success */
-    or al, al
-    jnz ContinueExecution
-
-    /* Dispatch the exception */
-    sub esp, 8
     call _RtlDispatchException@8
 
     /* Check for success */
@@ -195,7 +234,9 @@ Exit:
     push esp
     call _RtlRaiseException@4
     ret 8
+.endfunc
 
+.func RtlpGetStackLimits@8
 .globl _RtlpGetStackLimits@8
 _RtlpGetStackLimits@8:
 
@@ -211,4 +252,4 @@ _RtlpGetStackLimits@8:
 
     /* return */
     ret 8
-
+.endfunc

reactos/include/ndk/asm.h

@@ -310,6 +310,7 @@ Author:
 #define CONTEXT_FLOAT_SAVE_CONTROL_WORD         CONTEXT_FLOAT_SAVE + FN_CONTROL_WORD
 #define CONTEXT_FLOAT_SAVE_STATUS_WORD          CONTEXT_FLOAT_SAVE + FN_STATUS_WORD
 #define CONTEXT_FLOAT_SAVE_TAG_WORD             CONTEXT_FLOAT_SAVE + FN_TAG_WORD
+#define CONTEXT_ALIGNED_SIZE                    0x2CC
 
 //
 // EXCEPTION_RECORD Offsets
@@ -322,6 +323,24 @@ Author:
 #define SIZEOF_EXCEPTION_RECORD                 0x14
 #define EXCEPTION_RECORD_LENGTH                 0x50
 
+//
+// Exception types
+//
+#ifdef __ASM__
+#define EXCEPTION_NONCONTINUABLE                0x0001
+#define EXCEPTION_UNWINDING                     0x0002
+#define EXCEPTION_EXIT_UNWIND                   0x0004
+#define EXCEPTION_STACK_INVALID                 0x0008
+#define EXCEPTION_NESTED_CALL                   0x00010
+#define EXCEPTION_TARGET_UNWIND                 0x00020
+#define EXCEPTION_COLLIDED_UNWIND               0x00040
+#define EXCEPTION_UNWIND                        0x00066
+#define EXCEPTION_EXECUTE_HANDLER               0x00001
+#define EXCEPTION_CONTINUE_SEARCH               0x00000
+#define EXCEPTION_CONTINUE_EXECUTION            0xFFFFFFFF
+#define EXCEPTION_CHAIN_END                     0xFFFFFFFF
+#endif
+
 //
 // TEB Offsets
 //

reactos/lib/rtl/i386/exception.c

@@ -56,19 +56,17 @@ RtlDispatchException(IN PEXCEPTION_RECORD ExceptionRecord,
     EXCEPTION_DISPOSITION ReturnValue;
     ULONG_PTR StackLow, StackHigh;
     ULONG_PTR RegistrationFrameEnd;
-    DPRINT("RtlDispatchException(): %p, %p \n", ExceptionRecord, Context);
 
     /* Get the current stack limits and registration frame */
     RtlpGetStackLimits(&StackLow, &StackHigh);
     RegistrationFrame = RtlpGetExceptionList();
-    DPRINT("RegistrationFrame is 0x%p\n", RegistrationFrame);
 
     /* Now loop every frame */
     while (RegistrationFrame != EXCEPTION_CHAIN_END)
     {
         /* Find out where it ends */
         RegistrationFrameEnd = (ULONG_PTR)RegistrationFrame +
-                                sizeof(*RegistrationFrame);
+                                sizeof(EXCEPTION_REGISTRATION_RECORD);
 
         /* Make sure the registration frame is located within the stack */
         if ((RegistrationFrameEnd > StackHigh) ||
@@ -87,25 +85,24 @@ RtlDispatchException(IN PEXCEPTION_RECORD ExceptionRecord,
 
             /* Set invalid stack and return false */
             ExceptionRecord->ExceptionFlags |= EXCEPTION_STACK_INVALID;
-            DPRINT1("Invalid exception frame\n");
+            DPRINT1("Invalid exception frame: %p %p %p %p\n",
+                    RegistrationFrame, RegistrationFrameEnd,
+                    StackHigh, StackLow);
             return FALSE;
         }
 
         /* Check if logging is enabled */
-        DPRINT("Checking for logging\n");
         RtlpCheckLogException(ExceptionRecord,
                               Context,
                               RegistrationFrame,
                               sizeof(*RegistrationFrame));
 
         /* Call the handler */
-        DPRINT("Executing handler: %p\n", RegistrationFrame->Handler);
         ReturnValue = RtlpExecuteHandlerForException(ExceptionRecord,
                                                      RegistrationFrame,
                                                      Context,
                                                      &DispatcherContext,
                                                      RegistrationFrame->Handler);
-        DPRINT("Handler returned: %p\n", (PVOID)ReturnValue);
 
         /* Check if this is a nested frame */
         if (RegistrationFrame == NestedFrame)
@@ -128,7 +125,6 @@ RtlDispatchException(IN PEXCEPTION_RECORD ExceptionRecord,
                 ExceptionRecord2.NumberParameters = 0;
 
                 /* Raise the exception */
-                DPRINT("Non-continuable\n");
                 RtlRaiseException(&ExceptionRecord2);
             }
             else
@@ -166,7 +162,6 @@ RtlDispatchException(IN PEXCEPTION_RECORD ExceptionRecord,
     }
 
     /* Unhandled, return false */
-    DPRINT("FALSE\n");
     return FALSE;
 }
 
@@ -188,7 +183,6 @@ RtlUnwind(PVOID RegistrationFrame OPTIONAL,
     ULONG_PTR RegistrationFrameEnd;
     CONTEXT LocalContext;
     PCONTEXT Context;
-    DPRINT("RtlUnwind(). RegistrationFrame 0x%p\n", RegistrationFrame);
 
     /* Get the current stack limits */
     RtlpGetStackLimits(&StackLow, &StackHigh);
@@ -242,8 +236,6 @@ RtlUnwind(PVOID RegistrationFrame OPTIONAL,
     /* Now loop every frame */
     while (RegistrationFrame2 != EXCEPTION_CHAIN_END)
     {
-        DPRINT("RegistrationFrame is 0x%p\n", RegistrationFrame2);
-
         /* If this is the target */
         if (RegistrationFrame2 == RegistrationFrame)
         {
@@ -262,7 +254,6 @@ RtlUnwind(PVOID RegistrationFrame OPTIONAL,
             ExceptionRecord2.NumberParameters = 0;
 
             /* Raise the exception */
-            DPRINT1("Frame is invalid\n");
             RtlRaiseException(&ExceptionRecord2);
         }
 
@@ -298,13 +289,11 @@ RtlUnwind(PVOID RegistrationFrame OPTIONAL,
         else
         {
             /* Call the handler */
-            DPRINT("Executing unwind handler: %p\n", RegistrationFrame2->Handler);
             ReturnValue = RtlpExecuteHandlerForUnwind(ExceptionRecord,
                                                       RegistrationFrame2,
                                                       Context,
                                                       &DispatcherContext,
                                                       RegistrationFrame2->Handler);
-            DPRINT("Handler returned: %p\n", (PVOID)ReturnValue);
 
             /* Handle the dispositions */
             if (ReturnValue == ExceptionContinueSearch)

reactos/ntoskrnl/ke/i386/usercall.c

@@ -65,8 +65,8 @@ KiInitializeUserApc(IN PKEXCEPTION_FRAME ExceptionFrame,
                     IN PVOID SystemArgument2)
 {
     CONTEXT Context;
-    ULONG_PTR Stack;
-    ULONG Size;
+    ULONG_PTR Stack, AlignedEsp;
+    ULONG ContextLength;
     EXCEPTION_RECORD SehExceptRecord;
     _SEH_DECLARE_LOCALS(KiCopyInfo);
 
@@ -84,11 +84,15 @@ KiInitializeUserApc(IN PKEXCEPTION_FRAME ExceptionFrame,
         ASSERT((TrapFrame->SegCs & MODE_MASK) != KernelMode);
 
         /* Get the aligned size */
-        Size = ((sizeof(CONTEXT) + 3) & ~3) + 4 * sizeof(ULONG_PTR);
-        Stack = (Context.Esp & ~3) - Size;
+        AlignedEsp = Context.Esp & ~3;
+        ContextLength = CONTEXT_ALIGNED_SIZE + (4 * sizeof(ULONG_PTR));
+        Stack = ((AlignedEsp - 8) & ~3) - ContextLength;
 
-        /* Probe and copy */
-        ProbeForWrite((PVOID)Stack, Size, 4);
+        /* Probe the stack */
+        ProbeForWrite((PVOID)Stack, AlignedEsp - Stack, 1);
+        ASSERT(!(Stack & 3));
+
+        /* Copy data into it */
         RtlMoveMemory((PVOID)(Stack + (4 * sizeof(ULONG_PTR))),
                       &Context,
                       sizeof(CONTEXT));

reactos/ntoskrnl/rtl/i386/seh.s

@@ -16,20 +16,6 @@
 #define ExceptionNestedException	2
 #define ExceptionCollidedUnwind		3
 
-#define EXCEPTION_NONCONTINUABLE	0x01
-#define EXCEPTION_UNWINDING			0x02
-#define EXCEPTION_EXIT_UNWIND		0x04
-#define EXCEPTION_STACK_INVALID		0x08
-#define EXCEPTION_NESTED_CALL		0x10
-#define EXCEPTION_TARGET_UNWIND		0x20
-#define EXCEPTION_COLLIDED_UNWIND	0x40
-
-#define EXCEPTION_UNWIND_MODE \
-(  EXCEPTION_UNWINDING \
- | EXCEPTION_EXIT_UNWIND \
- | EXCEPTION_TARGET_UNWIND \
- | EXCEPTION_COLLIDED_UNWIND)
-
 #define EREC_CODE		0x00
 #define EREC_FLAGS		0x04
 #define EREC_RECORD		0x08
@@ -178,7 +164,7 @@ __except_handler3:
 
     // Either we're called to handle an exception or we're called to unwind    
     movl	EH3_ERECORD(%ebp), %eax
-    testl	$EXCEPTION_UNWIND_MODE, EREC_FLAGS(%eax)
+    testl	$EXCEPTION_UNWIND, EREC_FLAGS(%eax)
     jnz		.eh3_unwind
 
     // Keep a pointer to the exception registration in EBX