From 00d190a404ec6ea10e5e7f2d62a98082fada4bc9 Mon Sep 17 00:00:00 2001 From: Aleksey Bragin Date: Fri, 6 Jun 2008 21:51:21 +0000 Subject: [PATCH] - Fix KDBG's EIP munging. - Don't make single-step break into KDBG if it comes from user-mode. - Don't make KDBG return "continue" for breakpoint/singlestep, it should return "do not handle exception". - Add 20 new invalid instructions detected instead of crashing/"UNHANDLED CODE". - Fix DR registry handling (set DebugActive = TRUE when needed, and set Context->Dr7). - Fix set/get context: These two functions didn't work at all. Get actually performed a Set, and Vice-versa. Also, Set would incorrectly set the frame of the caller, not the target thread. Also, the trap frame pointer wasn't being grabbed correctly for kernel-mode callers. - Move the code to a non-portable i386 directory, since the code is architecture-specific. - Move GET_SET_CTX_CONTENT out to ps.h. svn path=/trunk/; revision=33871 --- reactos/ntoskrnl/include/internal/ps.h | 21 ++++ reactos/ntoskrnl/kd/kdmain.c | 15 +-- reactos/ntoskrnl/kdbg/kdb.c | 6 +- reactos/ntoskrnl/ke/i386/exp.c | 34 +++--- reactos/ntoskrnl/ke/i386/trap.s | 141 ++++++++++++++++++----- reactos/ntoskrnl/ntoskrnl-generic.rbuild | 5 + reactos/ntoskrnl/ps/debug.c | 69 +---------- reactos/ntoskrnl/ps/i386/psctx.c | 93 +++++++++++++++ 8 files changed, 252 insertions(+), 132 deletions(-) create mode 100644 reactos/ntoskrnl/ps/i386/psctx.c diff --git a/reactos/ntoskrnl/include/internal/ps.h b/reactos/ntoskrnl/include/internal/ps.h index 6abd4461d37..9fb2e54ae13 100644 --- a/reactos/ntoskrnl/include/internal/ps.h +++ b/reactos/ntoskrnl/include/internal/ps.h @@ -70,6 +70,17 @@ // #define PSP_JOB_SCHEDULING_CLASSES 10 +// +// Thread "Set/Get Context" Context Structure +// +typedef struct _GET_SET_CTX_CONTEXT +{ + KAPC Apc; + KEVENT Event; + KPROCESSOR_MODE Mode; + CONTEXT Context; +} GET_SET_CTX_CONTEXT, *PGET_SET_CTX_CONTEXT; + // // Initialization Functions // @@ -350,6 +361,16 @@ PsSuspendThread( OUT PULONG PreviousCount OPTIONAL ); +VOID +NTAPI +PspGetOrSetContextKernelRoutine( + IN PKAPC Apc, + IN OUT PKNORMAL_ROUTINE* NormalRoutine, + IN OUT PVOID* NormalContext, + IN OUT PVOID* SystemArgument1, + IN OUT PVOID* SystemArgument2 +); + // // Process Quotas // diff --git a/reactos/ntoskrnl/kd/kdmain.c b/reactos/ntoskrnl/kd/kdmain.c index b0d48f69988..282a46f981b 100644 --- a/reactos/ntoskrnl/kd/kdmain.c +++ b/reactos/ntoskrnl/kd/kdmain.c @@ -162,20 +162,11 @@ KdpEnterDebuggerException(IN PKTRAP_FRAME TrapFrame, /* Bump EIP over int 3 if debugger did not already change it */ if (ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) { -#ifdef KDBG - if (Context->Eip == EipOld) - Context->Eip++; -#else - /* We simulate the original behaviour when KDBG is turned off. - Return var is set to kdHandleException, thus we always return FALSE */ -#ifdef _M_IX86 - Context->Eip = EipOld; -#endif -#endif + //DPRINT1("Address: %p. Return: %d\n", EipOld, Return); } /* Convert return to BOOLEAN */ - if (Return == kdContinue) return TRUE; + if (Return == kdDoNotHandleException) return FALSE; return FALSE; } @@ -204,7 +195,7 @@ KdpCallGdb(IN PKTRAP_FRAME TrapFrame, } /* Convert return to BOOLEAN */ - if (Return == kdContinue) return TRUE; + if (Return == kdDoNotHandleException) return FALSE; return FALSE; } diff --git a/reactos/ntoskrnl/kdbg/kdb.c b/reactos/ntoskrnl/kdbg/kdb.c index cbd04c2c09d..83431420c9f 100644 --- a/reactos/ntoskrnl/kdbg/kdb.c +++ b/reactos/ntoskrnl/kdbg/kdb.c @@ -57,7 +57,7 @@ STATIC KDB_ENTER_CONDITION KdbEnterConditions[][2] = { /* First chance Last chance */ { KdbDoNotEnter, KdbEnterFromKmode }, /* Zero devide */ - { KdbEnterAlways, KdbDoNotEnter }, /* Debug trap */ + { KdbEnterFromKmode, KdbDoNotEnter }, /* Debug trap */ { KdbDoNotEnter, KdbEnterAlways }, /* NMI */ { KdbEnterFromKmode, KdbDoNotEnter }, /* INT3 */ { KdbDoNotEnter, KdbEnterFromKmode }, /* Overflow */ @@ -1485,7 +1485,7 @@ KdbEnterDebuggerException( { if (!EnterConditionMet) { - return ContinueType; + return kdDoNotHandleException; } DbgPrint("Entered debugger on unexpected debug trap!\n"); } @@ -1499,7 +1499,7 @@ KdbEnterDebuggerException( } if (!EnterConditionMet) { - return ContinueType; + return kdDoNotHandleException; } DbgPrint("Entered debugger on embedded INT3 at 0x%04x:0x%08x.\n", diff --git a/reactos/ntoskrnl/ke/i386/exp.c b/reactos/ntoskrnl/ke/i386/exp.c index 7d9ae71fa09..0af453bffb0 100644 --- a/reactos/ntoskrnl/ke/i386/exp.c +++ b/reactos/ntoskrnl/ke/i386/exp.c @@ -593,8 +593,8 @@ KeContextToTrapFrame(IN PCONTEXT Context, /* If we're in user-mode */ if (PreviousMode != KernelMode) { - /* FIXME: Save the mask */ - //KeGetCurrentThread()->DispatcherHeader.DebugActive = DrMask; + /* Save the mask */ + KeGetCurrentThread()->DispatcherHeader.DebugActive = DrMask; } } @@ -782,7 +782,7 @@ KeTrapFrameToContext(IN PKTRAP_FRAME TrapFrame, Context->Dr6 = TrapFrame->Dr6; /* Update DR7 */ - //Context->Dr7 = KiUpdateDr7(TrapFrame->Dr7); + Context->Dr7 = KiUpdateDr7(TrapFrame->Dr7); } else { @@ -947,21 +947,21 @@ KiDispatchException(IN PEXCEPTION_RECORD ExceptionRecord, /* User mode exception, was it first-chance? */ if (FirstChance) { - /* Enter Debugger if available */ - if (PsGetCurrentProcess()->DebugPort) + /* Make sure a debugger is present, and ignore user-mode if requested */ + if ((KiDebugRoutine) && + (!(PsGetCurrentProcess()->DebugPort))) { - /* FIXME : TODO */ - //KEBUGCHECK(0); - } - else if (KiDebugRoutine(TrapFrame, - ExceptionFrame, - ExceptionRecord, - &Context, - PreviousMode, - FALSE)) - { - /* Exception was handled */ - goto Handled; + /* Call the debugger */ + if (KiDebugRoutine(TrapFrame, + ExceptionFrame, + ExceptionRecord, + &Context, + PreviousMode, + FALSE)) + { + /* Exception was handled */ + goto Handled; + } } /* Forward exception to user mode debugger */ diff --git a/reactos/ntoskrnl/ke/i386/trap.s b/reactos/ntoskrnl/ke/i386/trap.s index 379d3245330..65ecc781906 100644 --- a/reactos/ntoskrnl/ke/i386/trap.s +++ b/reactos/ntoskrnl/ke/i386/trap.s @@ -646,14 +646,22 @@ _DispatchNoParam: call _CommonDispatchException .endfunc -.func DispatchOneParam -_DispatchOneParam: +.func DispatchOneParamZero +_DispatchOneParamZero: /* Call the common dispatcher */ xor edx, edx mov ecx, 1 call _CommonDispatchException .endfunc +.func DispatchTwoParamZero +_DispatchTwoParamZero: + /* Call the common dispatcher */ + xor edx, edx + mov ecx, 2 + call _CommonDispatchException +.endfunc + .func DispatchTwoParam _DispatchTwoParam: /* Call the common dispatcher */ @@ -1223,14 +1231,13 @@ CheckError: /* Raise exception */ mov eax, STATUS_FLOAT_INVALID_OPERATION - jmp _DispatchOneParam + jmp _DispatchOneParamZero InvalidStack: /* Raise exception */ mov eax, STATUS_FLOAT_STACK_CHECK - xor edx, edx - jmp _DispatchTwoParam + jmp _DispatchTwoParamZero ValidNpxOpcode: @@ -1240,7 +1247,7 @@ ValidNpxOpcode: /* Raise exception */ mov eax, STATUS_FLOAT_DIVIDE_BY_ZERO - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for denormal */ @@ -1249,7 +1256,7 @@ ValidNpxOpcode: /* Raise exception */ mov eax, STATUS_FLOAT_INVALID_OPERATION - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for overflow */ @@ -1258,7 +1265,7 @@ ValidNpxOpcode: /* Raise exception */ mov eax, STATUS_FLOAT_OVERFLOW - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for underflow */ @@ -1267,7 +1274,7 @@ ValidNpxOpcode: /* Raise exception */ mov eax, STATUS_FLOAT_UNDERFLOW - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for precision fault */ @@ -1276,7 +1283,7 @@ ValidNpxOpcode: /* Raise exception */ mov eax, STATUS_FLOAT_INEXACT_RESULT - jmp _DispatchOneParam + jmp _DispatchOneParamZero UnexpectedNpx: @@ -1446,11 +1453,7 @@ _KiTrap13: /* Otherwise, something is very wrong, raise an exception */ sti - mov ebx, [ebp+KTRAP_FRAME_EIP] - mov esi, -1 - mov eax, STATUS_ACCESS_VIOLATION - xor edx, edx - jmp _DispatchTwoParam + jmp SetException RaiseIrql: @@ -1494,7 +1497,7 @@ NotV86Trap: NotV86: /* Enter trap */ TRAP_PROLOG kitd_a, kitd_t - + /* Check if this was from kernel-mode */ test dword ptr [ebp+KTRAP_FRAME_CS], MODE_MASK jnz UserModeGpf @@ -1594,8 +1597,7 @@ TrapCopy: mov esi, [ebp+KTRAP_FRAME_ERROR_CODE] and esi, 0xFFFF mov eax, STATUS_ACCESS_VIOLATION - xor edx, edx - jmp _DispatchTwoParam + jmp _DispatchTwoParamZero MsrCheck: @@ -1741,19 +1743,95 @@ InstLoop: /* If it's not a prefix byte, check other instructions */ jnz NotPrefixByte + + /* Keep looping */ + loop InstLoop + + /* Fixup the stack */ + pop PCR[KPCR_EXCEPTION_LIST] + add esp, 8 - /* FIXME */ - UNHANDLED_PATH + /* Illegal instruction */ + jmp KmodeOpcode NotPrefixByte: - /* FIXME: Check if it's a HLT */ + /* Check if it's a HLT */ + cmp al, 0x0F4 + je IsPrivInstruction /* Check if the instruction has two bytes */ cmp al, 0xF jne CheckRing3Io - - /* FIXME */ - UNHANDLED_PATH + + /* Check if this is a LLDT or LTR */ + lods byte ptr [esi] + cmp al, 0 + jne NotLldt + + /* Check if this is an LLDT */ + lods byte ptr [esi] + and al, 0x38 + cmp al, 0x10 + je IsPrivInstruction + + /* Check if this is an LTR */ + cmp al, 0x18 + je IsPrivInstruction + + /* Otherwise, access violation */ + jmp NotIoViolation + +NotLldt: + /* Check if this is LGDT or LIDT or LMSW */ + cmp al, 0x01 + jne NotGdt + + /* Check if this is an LGDT */ + lods byte ptr [esi] + and al, 0x38 + cmp al, 0x10 + je IsPrivInstruction + + /* Check if this is an LIDT */ + cmp al, 0x18 + je IsPrivInstruction + + /* Check if this is an LMSW */ + cmp al, 0x30 + je IsPrivInstruction + + /* Otherwise, access violation */ + jmp NotIoViolation + +NotGdt: + /* Check if it's INVD or WBINVD */ + cmp al, 0x8 + je IsPrivInstruction + cmp al, 0x9 + je IsPrivInstruction + + /* Check if it's sysexit */ + cmp al, 0x35 + je IsPrivInstruction + + /* Check if it's a DR move */ + cmp al, 0x26 + je IsPrivInstruction + + /* Check if it's a CLTS */ + cmp al, 0x6 + je IsPrivInstruction + + /* Check if it's a CR move */ + cmp al, 0x20 + jb NotIoViolation + + /* Check if it's a DR move */ + cmp al, 0x24 + jbe IsPrivInstruction + + /* Everything else is an access violation */ + jmp NotIoViolation CheckRing3Io: /* Get EFLAGS and IOPL */ @@ -1804,8 +1882,7 @@ SetException: mov ebx, [ebp+KTRAP_FRAME_EIP] mov esi, -1 mov eax, STATUS_ACCESS_VIOLATION - xor edx, edx - jmp _DispatchTwoParam + jmp _DispatchTwoParamZero DispatchV86Gpf: /* FIXME */ @@ -2123,7 +2200,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_TRAPS - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for zero divide */ @@ -2132,7 +2209,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_TRAPS - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for denormal */ @@ -2141,7 +2218,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_TRAPS - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for overflow*/ @@ -2150,7 +2227,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_FAULTS - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for denormal */ @@ -2159,7 +2236,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_FAULTS - jmp _DispatchOneParam + jmp _DispatchOneParamZero 1: /* Check for Precision */ @@ -2168,7 +2245,7 @@ XmmiMakeCr0Dirty: /* Raise exception */ mov eax, STATUS_FLOAT_MULTIPLE_FAULTS - jmp _DispatchOneParam + jmp _DispatchOneParamZero UnexpectedXmmi: /* Strange result, bugcheck the OS */ diff --git a/reactos/ntoskrnl/ntoskrnl-generic.rbuild b/reactos/ntoskrnl/ntoskrnl-generic.rbuild index 58f080fc353..4bd37e0325d 100644 --- a/reactos/ntoskrnl/ntoskrnl-generic.rbuild +++ b/reactos/ntoskrnl/ntoskrnl-generic.rbuild @@ -398,6 +398,11 @@ events.c + + + psctx.c + + debug.c job.c kill.c diff --git a/reactos/ntoskrnl/ps/debug.c b/reactos/ntoskrnl/ps/debug.c index 6af913d9c0e..42db185d12d 100644 --- a/reactos/ntoskrnl/ps/debug.c +++ b/reactos/ntoskrnl/ps/debug.c @@ -11,18 +11,7 @@ #include #define NDEBUG -#include - -/* GLOBALS *****************************************************************/ - -/* Thread "Set/Get Context" Context Structure */ -typedef struct _GET_SET_CTX_CONTEXT -{ - KAPC Apc; - KEVENT Event; - KPROCESSOR_MODE Mode; - CONTEXT Context; -} GET_SET_CTX_CONTEXT, *PGET_SET_CTX_CONTEXT; +#include /* PRIVATE FUNCTIONS *********************************************************/ @@ -101,62 +90,6 @@ PspDumpThreads(BOOLEAN IncludeSystem) } #endif -VOID -NTAPI -PspGetOrSetContextKernelRoutine(IN PKAPC Apc, - IN OUT PKNORMAL_ROUTINE* NormalRoutine, - IN OUT PVOID* NormalContext, - IN OUT PVOID* SystemArgument1, - IN OUT PVOID* SystemArgument2) -{ -#if defined(_M_IX86) - PGET_SET_CTX_CONTEXT GetSetContext; - PKEVENT Event; - PCONTEXT Context; - PKTHREAD Thread; - KPROCESSOR_MODE Mode; - PKTRAP_FRAME TrapFrame; - PAGED_CODE(); - - /* Get the Context Structure */ - GetSetContext = CONTAINING_RECORD(Apc, GET_SET_CTX_CONTEXT, Apc); - Context = &GetSetContext->Context; - Event = &GetSetContext->Event; - Mode = GetSetContext->Mode; - Thread = Apc->SystemArgument2; - - /* Get the trap frame */ - TrapFrame = (PKTRAP_FRAME)((ULONG_PTR)KeGetCurrentThread()->InitialStack - - sizeof (FX_SAVE_AREA) - sizeof (KTRAP_FRAME)); - - /* Sanity check */ - ASSERT(((TrapFrame->SegCs & MODE_MASK) != KernelMode) || - (TrapFrame->EFlags & EFLAGS_V86_MASK)); - - /* Check if it's a set or get */ - if (SystemArgument1) - { - /* Get the Context */ - KeTrapFrameToContext(TrapFrame, NULL, Context); - } - else - { - /* Set the Context */ - KeContextToTrapFrame(Context, - NULL, - TrapFrame, - Context->ContextFlags, - Mode); - } - - /* Notify the Native API that we are done */ - KeSetEvent(Event, IO_NO_INCREMENT, FALSE); -#else - DPRINT1("PspGetOrSetContextKernelRoutine() not implemented!"); - for (;;); -#endif -} - /* PUBLIC FUNCTIONS **********************************************************/ /* diff --git a/reactos/ntoskrnl/ps/i386/psctx.c b/reactos/ntoskrnl/ps/i386/psctx.c new file mode 100644 index 00000000000..b1dd1e5b1cd --- /dev/null +++ b/reactos/ntoskrnl/ps/i386/psctx.c @@ -0,0 +1,93 @@ +/* + * PROJECT: ReactOS Kernel + * LICENSE: GPL - See COPYING in the top level directory + * FILE: ntoskrnl/ps/i386/pxctx.c + * PURPOSE: Process Manager: Set/Get Context for i386 + * PROGRAMMERS: Alex Ionescu (alex.ionescu@reactos.org) + */ + +/* INCLUDES *******************************************************************/ + +#include +#define NDEBUG +#include + +/* FUNCTIONS ******************************************************************/ + +VOID +NTAPI +PspGetContext(IN PKTRAP_FRAME TrapFrame, + IN PVOID NonVolatileContext, + IN OUT PCONTEXT Context) +{ + PAGED_CODE(); + + /* Convert the trap frame to a context */ + KeTrapFrameToContext(TrapFrame, NULL, Context); +} + +VOID +NTAPI +PspSetContext(OUT PKTRAP_FRAME TrapFrame, + OUT PVOID NonVolatileContext, + IN PCONTEXT Context, + IN KPROCESSOR_MODE Mode) +{ + PAGED_CODE(); + + /* Convert the context to a trap frame structure */ + KeContextToTrapFrame(Context, NULL, TrapFrame, Context->ContextFlags, Mode); +} + +VOID +NTAPI +PspGetOrSetContextKernelRoutine(IN PKAPC Apc, + IN OUT PKNORMAL_ROUTINE* NormalRoutine, + IN OUT PVOID* NormalContext, + IN OUT PVOID* SystemArgument1, + IN OUT PVOID* SystemArgument2) +{ + PGET_SET_CTX_CONTEXT GetSetContext; + PKEVENT Event; + PCONTEXT Context; + PKTHREAD Thread; + KPROCESSOR_MODE Mode; + PKTRAP_FRAME TrapFrame = NULL; + PAGED_CODE(); + + /* Get the Context Structure */ + GetSetContext = CONTAINING_RECORD(Apc, GET_SET_CTX_CONTEXT, Apc); + Context = &GetSetContext->Context; + Event = &GetSetContext->Event; + Mode = GetSetContext->Mode; + Thread = Apc->SystemArgument2; + + /* If this is a kernel-mode request, grab the saved trap frame */ + if (Mode == KernelMode) TrapFrame = Thread->TrapFrame; + + /* If we don't have one, grab it from the stack */ + if (!TrapFrame) + { + /* Trap frame is right under our initial stack */ + TrapFrame = (PKTRAP_FRAME)((ULONG_PTR)Thread->InitialStack - + ROUND_UP(sizeof(KTRAP_FRAME), KTRAP_FRAME_ALIGN) - + sizeof(FX_SAVE_AREA)); + } + + /* Check if it's a set or get */ + if (Apc->SystemArgument1) + { + /* Get the Context */ + PspSetContext(TrapFrame, NULL, Context, Mode); + } + else + { + /* Set the Context */ + PspGetContext(TrapFrame, NULL, Context); + } + + /* Notify the Native API that we are done */ + KeSetEvent(Event, IO_NO_INCREMENT, FALSE); +} + +/* EOF */