[ros-diffs] [tkreuzer] 63420: [NTOSKRNL] When a user mode debugger has single stepping enabled and steps over a sysenter instruction, you are obviously not supposed to enter the kernel debugger on the syscall ...

Author: tkreuzer
Date: Thu May 22 22:28:57 2014
New Revision: 63420

URL: http://svn.reactos.org/svn/reactos?rev=63420&view=rev
Log:
[NTOSKRNL]
When a user mode debugger has single stepping enabled and steps over a sysenter
instruction, you are obviously not supposed to enter the kernel debugger on the syscall
entry handler. But exactly this happened on reactos. This was because the sysenter
instruction doesn't disable single stepping, so we need to handle this special case
manually in the single stepping handler (which we didn't). We now check if the single
step comes from KiFastCallEntry and when it does, disable single stepping in the current
(nested) trap frame and return back to a secondary fast call entry. The 2nd entrypoint
will make sure to re-enable the single step flag in EFLAGS before returning to usermode.
To make this actually work, the asm entry stub itself needs to handle saving of eflags, so
some trap frame modification from KiFastCallEntryHandler was moved into the asm stub.
Since the amount of asm instructions is rather small (10 instructions, pretty straight
forward) I moved everything from KiSystemServiceHandler to the asm stub and killed
KiFastCallEntryHandler entirely, calling KiSystemServiceHandler instead.
Now stepping over a sysenter instruction works with OllyDbg without breaking into the
kernel debugger. CORE-8057 #resolve

Modified:
    trunk/reactos/include/asm/ks386.template.h
    trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
    trunk/reactos/ntoskrnl/ke/i386/trap.s
    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c

Modified: trunk/reactos/include/asm/ks386.template.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/include/asm/ks386.template…
==============================================================================
--- trunk/reactos/include/asm/ks386.template.h	[iso-8859-1] (original)
+++ trunk/reactos/include/asm/ks386.template.h	[iso-8859-1] Thu May 22 22:28:57 2014
@@ -347,7 +347,7 @@
 //OFFSET(CONTEXT_FLOAT_SAVE_STATUS_WORD          CONTEXT_FLOAT_SAVE + FP_STATUS_WORD
 //OFFSET(CONTEXT_FLOAT_SAVE_TAG_WORD             CONTEXT_FLOAT_SAVE + FP_TAG_WORD
 //OFFSET(CONTEXT_FRAME_LENGTH                    0x2D0
-SIZE(CONTEXT_FRAME_LENGTH, CONTEXT), 
+SIZE(CONTEXT_FRAME_LENGTH, CONTEXT),
 
 HEADER("FIBER"),
 OFFSET(FIBER_PARAMETER, FIBER, Parameter),
@@ -387,7 +387,7 @@
 OFFSET(EXCEPTION_RECORD_EXCEPTION_ADDRESS, EXCEPTION_RECORD, ExceptionAddress),
 SIZE(SIZEOF_EXCEPTION_RECORD, EXCEPTION_RECORD),
 CONSTANT(EXCEPTION_RECORD_LENGTH),
- 
+
 //#define EXCEPTION_RECORD_LENGTH                 0x50
 
 HEADER("KTHREAD"),
@@ -463,6 +463,5 @@
 CONSTANT(STATUS_CALLBACK_POP_STACK),
 CONSTANT(CONTEXT_ALIGNED_SIZE),
 CONSTANT(PROCESSOR_FEATURE_FXSR),
-
-
-
+CONSTANT(KUSER_SHARED_SYSCALL_RET),
+

Modified: trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/…
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/include/internal/i386/asmmacro.S	[iso-8859-1] Thu May 22
22:28:57 2014
@@ -83,7 +83,7 @@
     if (Flags AND KI_FAST_SYSTEM_CALL)
 
         /* SYSENTER requires us to build a complete ring transition trap frame */
-        FrameSize = KTRAP_FRAME_V86_ES
+        FrameSize = KTRAP_FRAME_EIP
 
         /* Fixup fs. cx is free to clobber */
         mov cx, KGDT_R0_PCR
@@ -94,6 +94,13 @@
 
         /* Get a stack pointer */
         mov esp, [ecx + KTSS_ESP0]
+
+        /* Set up a fake hardware trap frame */
+        push KGDT_R3_DATA or RPL_MASK
+        push edx
+        pushfd
+        push KGDT_R3_CODE or RPL_MASK
+        push dword ptr ds:[KUSER_SHARED_SYSCALL_RET]
 
     elseif (Flags AND KI_SOFTWARE_TRAP)
 
@@ -183,7 +190,19 @@
     mov es, ax
 
     /* Fast system calls have fs already fixed */
-    if (NOT (Flags AND KI_FAST_SYSTEM_CALL))
+    if (Flags AND KI_FAST_SYSTEM_CALL)
+
+        /* Enable interrupts and set a sane FS value */
+        or dword ptr [esp + KTRAP_FRAME_EFLAGS], EFLAGS_INTERRUPT_MASK
+        mov dword ptr [esp + KTRAP_FRAME_FS], KGDT_R3_TEB or RPL_MASK
+
+        /* Set sane active EFLAGS */
+        push 2
+        popfd
+
+        /* Point edx to the usermode parameters */
+        add edx, 8
+    else
         /* Otherwise fix fs now */
         mov ax, KGDT_R0_PCR
         mov fs, ax

Modified: trunk/reactos/ntoskrnl/ke/i386/trap.s
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/trap.s?re…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/trap.s	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/trap.s	[iso-8859-1] Thu May 22 22:28:57 2014
@@ -142,14 +142,20 @@
     KiCallHandler @KiSystemServiceHandler@8
 .ENDP
 
-EXTERN @KiFastCallEntryHandler@8:PROC
 PUBLIC _KiFastCallEntry
 .PROC _KiFastCallEntry
     FPO 0, 0, 0, 0, 1, FRAME_TRAP
     KiEnterTrap (KI_FAST_SYSTEM_CALL OR KI_NONVOLATILES_ONLY OR KI_DONT_SAVE_SEGS)
-    KiCallHandler @KiFastCallEntryHandler@8
+    KiCallHandler @KiSystemServiceHandler@8
 .ENDP
 
+PUBLIC _KiFastCallEntryWithSingleStep
+.PROC _KiFastCallEntryWithSingleStep
+    FPO 0, 0, 0, 0, 1, FRAME_TRAP
+    KiEnterTrap (KI_FAST_SYSTEM_CALL OR KI_NONVOLATILES_ONLY OR KI_DONT_SAVE_SEGS)
+    or dword ptr [ecx + KTRAP_FRAME_EFLAGS], EFLAGS_TF
+    KiCallHandler @KiSystemServiceHandler@8
+.ENDP
 
 PUBLIC _KiEndUnexpectedRange@0
 _KiEndUnexpectedRange@0:

Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.…
==============================================================================
--- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c	[iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c	[iso-8859-1] Thu May 22 22:28:57 2014
@@ -11,6 +11,9 @@
 #include <ntoskrnl.h>
 #define NDEBUG
 #include <debug.h>
+
+VOID KiFastCallEntry(VOID);
+VOID KiFastCallEntryWithSingleStep(VOID);
 
 /* GLOBALS ********************************************************************/
 
@@ -417,13 +420,26 @@
 {
     /* Save trap frame */
     KiEnterTrap(TrapFrame);
-    
+
     /* Check for VDM trap */
     ASSERT((KiVdmTrap(TrapFrame)) == FALSE);
 
+    /* Check if this was a single step after sysenter */
+    if (TrapFrame->Eip == (ULONG)KiFastCallEntry)
+    {
+        /* Disable single stepping */
+        TrapFrame->EFlags &= ~EFLAGS_TF;
+
+        /* Re-enter at the alternative sysenter entry point */
+        TrapFrame->Eip = (ULONG)KiFastCallEntryWithSingleStep;
+
+        /* End this trap */
+        KiEoiHelper(TrapFrame);
+    }
+
     /* Enable interrupts if the trap came from user-mode */
     if (KiUserTrap(TrapFrame)) _enable();
-    
+
     /*  Mask out trap flag and dispatch the exception */
     TrapFrame->EFlags &= ~EFLAGS_TF;
     KiDispatchException0Args(STATUS_SINGLE_STEP,
@@ -1521,11 +1537,11 @@
     return Result;
 }
 
-FORCEINLINE
-DECLSPEC_NORETURN
-VOID
-KiSystemCall(IN PKTRAP_FRAME TrapFrame,
-             IN PVOID Arguments)
+DECLSPEC_NORETURN
+VOID
+FASTCALL
+KiSystemServiceHandler(IN PKTRAP_FRAME TrapFrame,
+                       IN PVOID Arguments)
 {
     PKTHREAD Thread;
     PKSERVICE_TABLE_DESCRIPTOR DescriptorTable;
@@ -1658,38 +1674,6 @@
     KiServiceExit(TrapFrame, Result);
 }
 
-DECLSPEC_NORETURN
-VOID
-FASTCALL
-KiSystemServiceHandler(IN PKTRAP_FRAME TrapFrame,
-                       IN PVOID Arguments)
-{
-    /* Call the shared handler (inline) */
-    KiSystemCall(TrapFrame, Arguments);
-}
-
-DECLSPEC_NORETURN
-VOID
-FASTCALL
-KiFastCallEntryHandler(IN PKTRAP_FRAME TrapFrame,
-                       IN PVOID Arguments)
-{
-    /* Set up a fake INT Stack and enable interrupts */
-    TrapFrame->HardwareSegSs = KGDT_R3_DATA | RPL_MASK;
-    TrapFrame->HardwareEsp = (ULONG_PTR)Arguments;
-    TrapFrame->EFlags = __readeflags() | EFLAGS_INTERRUPT_MASK;
-    TrapFrame->SegCs = KGDT_R3_CODE | RPL_MASK;
-    TrapFrame->Eip = SharedUserData->SystemCallReturn;
-    TrapFrame->SegFs = KGDT_R3_TEB | RPL_MASK;
-    __writeeflags(0x2);
-    
-    /* Arguments are actually 2 frames down (because of the double indirection) */
-    Arguments = (PVOID)(TrapFrame->HardwareEsp + 8);
-
-    /* Call the shared handler (inline) */
-    KiSystemCall(TrapFrame, Arguments);
-}
-
 /*
  * @implemented
  */