From a195100319e13bd7bc3470d1959c50d691ca8500 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Sat, 3 Mar 2007 17:24:58 +0000 Subject: [PATCH] - Fix critical bugs in DR_TRAP_FIXUP, TRAP_PROLOG and TRAP_EPILOG which would either cause infinite loops during exceptions or corruption of the correct code path when dealing with debug registers. - Fix a bug in KiRecordDr7 setting the new DR7 mask. - Make KiEspToTrapFrame thread-safe by raising to APC_LEVEL to make sure a thread/set context doesn't corrupt the state. - Fix thread-safe IRQL Code in KeContexToTrapFrame/KeTrapFrameToContext. - Fix KiDispatchException to properly handle KI_EXCEPTION_ACCESS_VIOLATION and convert it back to STATUS_ACCESS_VIOLATION which is what the system expects. - Also fix the way we do bugchecks so the the trapframe gets properly put as a parameter. - Make KiDebugService call into KiTrap3 to share code (merge from kd-branch). - Changes to the KdpEnterDebuggerException hack we have to handle this change. - Temporarily disable DebugPrint functionality (sorry, I'm onto a big bug here!) svn path=/trunk/; revision=25975 --- reactos/lib/rtl/debug.c | 2 +- .../ntoskrnl/include/internal/i386/asmmacro.S | 6 +- reactos/ntoskrnl/kd/kdmain.c | 23 ++++++- reactos/ntoskrnl/ke/i386/exp.c | 67 ++++++++++++++----- reactos/ntoskrnl/ke/i386/trap.s | 39 +---------- 5 files changed, 77 insertions(+), 60 deletions(-) diff --git a/reactos/lib/rtl/debug.c b/reactos/lib/rtl/debug.c index 60cf6b3ff50..f91ad108987 100644 --- a/reactos/lib/rtl/debug.c +++ b/reactos/lib/rtl/debug.c @@ -23,7 +23,7 @@ DebugPrint(IN PANSI_STRING DebugString, IN ULONG Level) { /* Call the INT2D Service */ - //return STATUS_SUCCESS; + return STATUS_SUCCESS; return DebugService(BREAKPOINT_PRINT, DebugString->Buffer, DebugString->Length, diff --git a/reactos/ntoskrnl/include/internal/i386/asmmacro.S b/reactos/ntoskrnl/include/internal/i386/asmmacro.S index 063d438ef24..2972df214ad 100644 --- a/reactos/ntoskrnl/include/internal/i386/asmmacro.S +++ b/reactos/ntoskrnl/include/internal/i386/asmmacro.S @@ -304,7 +304,7 @@ _KiUnexpectedInterrupt&Number: /* Set them */ mov dr6, ebx mov dr7, ecx - jz 3f + jmp 3f .endm // @@ -482,12 +482,12 @@ _KiUnexpectedInterrupt&Number: /* Flush DR7 */ and dword ptr [ebp+KTRAP_FRAME_DR7], 0 -3: /* Check if the thread was being debugged */ test byte ptr [ecx+KTHREAD_DEBUG_ACTIVE], 0xFF jnz Dr_&Label /* Set the Trap Frame Debug Header */ +3: SET_TF_DEBUG_HEADER .endm @@ -1171,7 +1171,7 @@ FastExit: mov dr3, esi mov dr6, edi mov dr7, ebx - jz 4b + jmp 4b 7: /* Restore real CS value */ diff --git a/reactos/ntoskrnl/kd/kdmain.c b/reactos/ntoskrnl/kd/kdmain.c index 13e65a798d6..168bf01909e 100644 --- a/reactos/ntoskrnl/kd/kdmain.c +++ b/reactos/ntoskrnl/kd/kdmain.c @@ -106,9 +106,28 @@ KdpEnterDebuggerException(IN PKTRAP_FRAME TrapFrame, IN BOOLEAN SecondChance) { KD_CONTINUE_TYPE Return; + ULONG ExceptionCommand = ExceptionRecord->ExceptionInformation[0]; - /* HACK (just like all this routine */ - if (ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) Context->Eip++; + /* Check if this was a breakpoint due to DbgPrint or Load/UnloadSymbols */ + if ((ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) && + (ExceptionRecord->NumberParameters > 0) && + ((ExceptionCommand == BREAKPOINT_LOAD_SYMBOLS) || + (ExceptionCommand == BREAKPOINT_UNLOAD_SYMBOLS) || + (ExceptionCommand == BREAKPOINT_COMMAND_STRING) || + (ExceptionCommand == BREAKPOINT_PRINT))) + { + /* Check if this is a debug print */ + if (ExceptionCommand == BREAKPOINT_PRINT) + { + /* Print the string */ + KdpServiceDispatcher(BREAKPOINT_PRINT, + (PVOID)ExceptionRecord->ExceptionInformation[1], + ExceptionRecord->ExceptionInformation[2]); + } + + /* This we can handle: simply bump EIP */ + Context->Eip++; + } /* Get out of here if the Debugger isn't connected */ if (KdDebuggerNotPresent) return FALSE; diff --git a/reactos/ntoskrnl/ke/i386/exp.c b/reactos/ntoskrnl/ke/i386/exp.c index 90a365fd8a4..4893a1eac64 100644 --- a/reactos/ntoskrnl/ke/i386/exp.c +++ b/reactos/ntoskrnl/ke/i386/exp.c @@ -140,7 +140,7 @@ KiRecordDr7(OUT PULONG Dr7Ptr, NewMask |= DR_MASK(DR7_OVERRIDE_V); /* Set DR7 override */ - *DrMask = DR7_OVERRIDE_MASK; + *DrMask |= DR7_OVERRIDE_MASK; } else { @@ -210,10 +210,19 @@ NTAPI KiEspToTrapFrame(IN PKTRAP_FRAME TrapFrame, IN ULONG Esp) { - ULONG Previous = KiEspFromTrapFrame(TrapFrame); + KIRQL OldIrql; + ULONG Previous; + + /* Raise to APC_LEVEL if needed */ + OldIrql = KeGetCurrentIrql(); + if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql); + + /* Get the old ESP */ + Previous = KiEspFromTrapFrame(TrapFrame); /* Check if this is user-mode or V86 */ - if ((TrapFrame->SegCs & MODE_MASK) || (TrapFrame->EFlags & EFLAGS_V86_MASK)) + if ((TrapFrame->SegCs & MODE_MASK) || + (TrapFrame->EFlags & EFLAGS_V86_MASK)) { /* Write it directly */ TrapFrame->HardwareEsp = Esp; @@ -221,7 +230,11 @@ KiEspToTrapFrame(IN PKTRAP_FRAME TrapFrame, else { /* Don't allow ESP to be lowered, this is illegal */ - if (Esp < Previous) KeBugCheck(SET_OF_INVALID_CONTEXT); + if (Esp < Previous) KeBugCheckEx(SET_OF_INVALID_CONTEXT, + Esp, + Previous, + (ULONG_PTR)TrapFrame, + 0); /* Create an edit frame, check if it was alrady */ if (!(TrapFrame->SegCs & FRAME_EDITED)) @@ -243,6 +256,9 @@ KiEspToTrapFrame(IN PKTRAP_FRAME TrapFrame, } } } + + /* Restore IRQL */ + if (OldIrql < APC_LEVEL) KeLowerIrql(OldIrql); } ULONG @@ -316,12 +332,13 @@ KeContextToTrapFrame(IN PCONTEXT Context, PFX_SAVE_AREA FxSaveArea; ULONG i; BOOLEAN V86Switch = FALSE; - KIRQL OldIrql = APC_LEVEL; + KIRQL OldIrql; ULONG DrMask = 0; PVOID SafeDr; /* Do this at APC_LEVEL */ - if (KeGetCurrentIrql() < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql); + OldIrql = KeGetCurrentIrql(); + if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql); /* Start with the basic Registers */ if ((ContextFlags & CONTEXT_CONTROL) == CONTEXT_CONTROL) @@ -544,7 +561,7 @@ KeContextToTrapFrame(IN PCONTEXT Context, else { /* FIXME: Handle FPU Emulation */ - ASSERT(FALSE); + //ASSERT(FALSE); } } @@ -600,11 +617,12 @@ KeTrapFrameToContext(IN PKTRAP_FRAME TrapFrame, FLOATING_SAVE_AREA UnalignedArea; } FloatSaveBuffer; FLOATING_SAVE_AREA *FloatSaveArea; - KIRQL OldIrql = APC_LEVEL; + KIRQL OldIrql; ULONG i; /* Do this at APC_LEVEL */ - if (KeGetCurrentIrql() < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql); + OldIrql = KeGetCurrentIrql(); + if (OldIrql < APC_LEVEL) KeRaiseIrql(APC_LEVEL, &OldIrql); /* Start with the Control flags */ if ((Context->ContextFlags & CONTEXT_CONTROL) == CONTEXT_CONTROL) @@ -817,11 +835,26 @@ KiDispatchException(IN PEXCEPTION_RECORD ExceptionRecord, /* Get a Context */ KeTrapFrameToContext(TrapFrame, ExceptionFrame, &Context); - /* Fix up EIP */ - if (ExceptionRecord->ExceptionCode == STATUS_BREAKPOINT) + /* Look at our exception code */ + switch (ExceptionRecord->ExceptionCode) { - /* Decrement EIP by one */ - Context.Eip--; + /* Breapoint */ + case STATUS_BREAKPOINT: + + /* Decrement EIP by one */ + Context.Eip--; + break; + + /* Internal exception */ + case KI_EXCEPTION_ACCESS_VIOLATION: + + /* Set correct code */ + ExceptionRecord->ExceptionCode = STATUS_ACCESS_VIOLATION; + if (PreviousMode == UserMode) + { + /* FIXME: Handle no execute */ + } + break; } /* Sanity check */ @@ -866,8 +899,8 @@ KiDispatchException(IN PEXCEPTION_RECORD ExceptionRecord, KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED, ExceptionRecord->ExceptionCode, (ULONG_PTR)ExceptionRecord->ExceptionAddress, - ExceptionRecord->ExceptionInformation[0], - ExceptionRecord->ExceptionInformation[1]); + (ULONG_PTR)TrapFrame, + 0); } else { @@ -989,8 +1022,8 @@ DispatchToUser: KeBugCheckEx(KMODE_EXCEPTION_NOT_HANDLED, ExceptionRecord->ExceptionCode, (ULONG_PTR)ExceptionRecord->ExceptionAddress, - ExceptionRecord->ExceptionInformation[0], - ExceptionRecord->ExceptionInformation[1]); + (ULONG_PTR)TrapFrame, + 0); } Handled: diff --git a/reactos/ntoskrnl/ke/i386/trap.s b/reactos/ntoskrnl/ke/i386/trap.s index 7392de685d6..0bab3fcf9a7 100644 --- a/reactos/ntoskrnl/ke/i386/trap.s +++ b/reactos/ntoskrnl/ke/i386/trap.s @@ -473,43 +473,8 @@ _KiDebugService: mov ecx, [ebp+KTRAP_FRAME_ECX] mov edx, [ebp+KTRAP_FRAME_EDX] - /* Check for V86 mode */ - test dword ptr [ebp+KTRAP_FRAME_EFLAGS], EFLAGS_V86_MASK - jnz NotUserMode - - /* Check if this is kernel or user-mode */ - test byte ptr [ebp+KTRAP_FRAME_CS], 1 - jz CallDispatch - cmp word ptr [ebp+KTRAP_FRAME_CS], KGDT_R3_CODE + RPL_MASK - jnz NotUserMode - - /* Re-enable interrupts */ -VdmProc: - sti - - /* Call the debug routine */ -CallDispatch: - mov esi, ecx - mov edi, edx - mov edx, eax - mov ecx, 3 - push edi - push esi - push edx - call _KdpServiceDispatcher@12 - -NotUserMode: - - /* Get the current process */ - mov ebx, [fs:KPCR_CURRENT_THREAD] - mov ebx, [ebx+KTHREAD_APCSTATE_PROCESS] - - /* Check if this is a VDM Process */ - //cmp dword ptr [ebx+EPROCESS_VDM_OBJECTS], 0 - //jz VdmProc - - /* Exit through common routine */ - jmp _Kei386EoiHelper@0 + /* Jump to INT3 handler */ + jmp PrepareInt3 .endfunc .func NtRaiseException@12