[ros-diffs] [reactos] 01/01: [NTOS:KDBG] Rewrite the TSS handling code in the backtrace function, removing limitations (and bugs) of the original code. CORE-16448, PR #2003. Supersedes PR #1997.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=d21ff0ed137d61b7881af…

commit d21ff0ed137d61b7881af96dd1d6e14387ccb89b
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Mon Oct 28 01:01:23 2019 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Wed Nov 13 02:14:48 2019 +0100

    [NTOS:KDBG] Rewrite the TSS handling code in the backtrace function, removing
limitations (and bugs) of the original code.
    CORE-16448, PR #2003. Supersedes PR #1997.
    
    This commit supersedes commit 6c5c7809 (r54503).
    
    The original code was checking for the NMI or Double-Fault TSS by
    comparing the current stack-traced EIP address with their corresponding
    trap handler address ranges. That method was actually buggy because
    nothing was ensuring that the trap handlers were in the "expected" order
    in the kernel binary (and in memory).
    
    Instead, we now can handle completely generic nested TSSes, instead of
    just the NMI or the Double-Fault ones.
    The way we proceed is by performing the full stack backtrace of the
    current TSS, then once finished we check whether this TSS is nested
    (has a parent). If so we change the (cached) current TSS to the latter,
    restarting the backtrace at the parent TSS' latest EIP.
    
    Examples of stack backtraces:
    =============================
    
    - General Protection fault:
    
    <snip>
    
    *** Fatal System Error: 0x0000007f
                           (0x0000000D,0x00000000,0x00000000,0x00000000)
    
    Entered debugger on embedded INT3 at 0x0008:0x80953528.
    kdb:> bt
    Eip:
    <ntoskrnl.exe:153529 (sdk/lib/rtl/i386/debug_asm.S:57
(RtlpBreakWithStatusInstruction))>
    Frames:
    <ntoskrnl.exe:899b0 (ntoskrnl/ke/bug.c:1136 (KeBugCheckWithTf))>
    <ntoskrnl.exe:134826 (ntoskrnl/ke/i386/exp.c:1161 (KeRaiseUserException))>
    <ntoskrnl.exe:19ae67 (ntoskrnl/ke/i386/traphdlr.c:1282 (KiTrap0DHandler))>
    <ntoskrnl.exe:19a840 (:0 (KiTrap0D))>
    <ntoskrnl.exe:1925e6 (ntoskrnl/include/internal/i386/intrin_i.h:45
(KiInitMachineDependent))>
    <ntoskrnl.exe:187688 (ntoskrnl/ke/krnlinit.c:305 (KeInitSystem))>
    <ntoskrnl.exe:17fb2f (ntoskrnl/ex/init.c:1621 (Phase1InitializationDiscard))>
    <ntoskrnl.exe:3247f (ntoskrnl/ex/init.c:2019 (Phase1Initialization))>
    <ntoskrnl.exe:11c079 (ntoskrnl/ps/thread.c:156 (PspSystemThreadStartup))>
    <ntoskrnl.exe:135c8a (ntoskrnl/ke/i386/thrdini.c:78 (KiThreadStartup))>
    <ntoskrnl.exe:11c040 (ntoskrnl/ps/thread.c:141 (PspSystemThreadStartup))>
    <5d8950ec>
    Couldn't access memory at 0x83E58959!
    
    </snip>
    
    - Double-fault (manually triggered by removing the GP handler):
    
    Note how the backtrace explicitly specifies the crossed TSS boundaries,
    and the trace in the parent TSS is indeed consistent with the previous
    example. Note also that log2lines (used here to completely resolve the
    trace) failed to see KiTrap08Handler(), which has been instead mistaken
    for KiTrap09().
    
    <snip>
    
    *** Fatal System Error: 0x0000007f
                           (0x00000008,0x8009C000,0x00000000,0x00000000)
    
    Entered debugger on embedded INT3 at 0x0008:0x80953528.
    kdb:> bt
    [Active TSS 0x0050 @ 0x80A10CA0]
    Eip:
    <ntoskrnl.exe:153529 (sdk/lib/rtl/i386/debug_asm.S:57
(RtlpBreakWithStatusInstruction))>
    Frames:
    <ntoskrnl.exe:899b0 (ntoskrnl/ke/bug.c:1136 (KeBugCheckWithTf))>
    <ntoskrnl.exe:19a1d8 (ntoskrnl/ke/i386/traphdlr.c:917 (KiTrap09))>      //
<-- Here, log2lines fails to see it's actually KiTrap08Handler.
    <ntoskrnl.exe:19a145 (:0 (KiTrap08))>
    [Parent TSS 0x0028 @ 0x8009C000]
    <ntoskrnl.exe:1925e6 (ntoskrnl/include/internal/i386/intrin_i.h:45
(KiInitMachineDependent))>
    <ntoskrnl.exe:187688 (ntoskrnl/ke/krnlinit.c:305 (KeInitSystem))>
    <ntoskrnl.exe:17fb2f (ntoskrnl/ex/init.c:1621 (Phase1InitializationDiscard))>
    <ntoskrnl.exe:3247f (ntoskrnl/ex/init.c:2019 (Phase1Initialization))>
    <ntoskrnl.exe:11c079 (ntoskrnl/ps/thread.c:156 (PspSystemThreadStartup))>
    <ntoskrnl.exe:135c8a (ntoskrnl/ke/i386/thrdini.c:78 (KiThreadStartup))>
    <ntoskrnl.exe:11c040 (ntoskrnl/ps/thread.c:141 (PspSystemThreadStartup))>
    <5d8950ec>
    Couldn't access memory at 0x83E58959!
    
    </snip>
---
 ntoskrnl/kdbg/kdb_cli.c | 186 ++++++++++++++++++++++++++----------------------
 1 file changed, 102 insertions(+), 84 deletions(-)

diff --git a/ntoskrnl/kdbg/kdb_cli.c b/ntoskrnl/kdbg/kdb_cli.c
index fd93246afa7..abf2e6d84aa 100644
--- a/ntoskrnl/kdbg/kdb_cli.c
+++ b/ntoskrnl/kdbg/kdb_cli.c
@@ -1033,86 +1033,73 @@ KdbpRetrieveTss(
     return Tss;
 }
 
-static BOOLEAN
-KdbpTrapFrameFromPrevTss(
-    PKTRAP_FRAME TrapFrame)
+FORCEINLINE BOOLEAN
+KdbpIsNestedTss(
+    IN USHORT TssSelector,
+    IN PKTSS Tss)
 {
-    ULONG_PTR Eip, Ebp;
-    KDESCRIPTOR Gdtr;
-    KGDTENTRY Desc;
-    USHORT Sel;
-    PKTSS Tss;
-
-    Ke386GetGlobalDescriptorTable(&Gdtr.Limit);
-    Sel = Ke386GetTr();
+    USHORT Backlink;
 
-    if ((Sel & (sizeof(KGDTENTRY) - 1)) ||
-        (Sel < sizeof(KGDTENTRY)) ||
-        (Sel + sizeof(KGDTENTRY) - 1 > Gdtr.Limit))
+    if (!Tss)
         return FALSE;
 
-    if (!NT_SUCCESS(KdbpSafeReadMemory(&Desc,
-                                       (PVOID)(Gdtr.Base + Sel),
-                                       sizeof(KGDTENTRY))))
+    /* Retrieve the TSS Backlink */
+    if (!NT_SUCCESS(KdbpSafeReadMemory(&Backlink,
+                                       (PVOID)&Tss->Backlink,
+                                       sizeof(USHORT))))
+    {
         return FALSE;
+    }
 
-    if (Desc.HighWord.Bits.Type != 0xB)
-        return FALSE;
+    return (Backlink != 0 && Backlink != TssSelector);
+}
 
-    Tss = (PKTSS)(ULONG_PTR)(Desc.BaseLow |
-                             Desc.HighWord.Bytes.BaseMid << 16 |
-                             Desc.HighWord.Bytes.BaseHi << 24);
+static BOOLEAN
+KdbpTrapFrameFromPrevTss(
+    IN OUT PKTRAP_FRAME TrapFrame,
+    OUT PUSHORT TssSelector,
+    IN OUT PKTSS* pTss,
+    IN PKDESCRIPTOR pGdtr)
+{
+    ULONG_PTR Eip, Ebp;
+    USHORT Backlink;
+    PKTSS Tss = *pTss;
 
-    if (!NT_SUCCESS(KdbpSafeReadMemory(&Sel,
+    /* Retrieve the TSS Backlink */
+    if (!NT_SUCCESS(KdbpSafeReadMemory(&Backlink,
                                        (PVOID)&Tss->Backlink,
                                        sizeof(USHORT))))
+    {
         return FALSE;
+    }
 
-    if ((Sel & (sizeof(KGDTENTRY) - 1)) ||
-        (Sel < sizeof(KGDTENTRY)) ||
-        (Sel + sizeof(KGDTENTRY) - 1 > Gdtr.Limit))
-        return FALSE;
-
-    if (!NT_SUCCESS(KdbpSafeReadMemory(&Desc,
-                                       (PVOID)(Gdtr.Base + Sel),
-                                       sizeof(KGDTENTRY))))
-        return FALSE;
-
-    if (Desc.HighWord.Bits.Type != 0xB)
+    /* Retrieve the parent TSS */
+    Tss = KdbpRetrieveTss(Backlink, NULL, pGdtr);
+    if (!Tss)
         return FALSE;
 
-    Tss = (PKTSS)(ULONG_PTR)(Desc.BaseLow |
-                             Desc.HighWord.Bytes.BaseMid << 16 |
-                             Desc.HighWord.Bytes.BaseHi << 24);
-
     if (!NT_SUCCESS(KdbpSafeReadMemory(&Eip,
                                        (PVOID)&Tss->Eip,
                                        sizeof(ULONG_PTR))))
+    {
         return FALSE;
+    }
 
     if (!NT_SUCCESS(KdbpSafeReadMemory(&Ebp,
                                        (PVOID)&Tss->Ebp,
                                        sizeof(ULONG_PTR))))
+    {
         return FALSE;
+    }
 
+    /* Return the parent TSS and its trap frame */
+    *TssSelector = Backlink;
+    *pTss = Tss;
     TrapFrame->Eip = Eip;
     TrapFrame->Ebp = Ebp;
     return TRUE;
 }
 
-VOID __cdecl KiTrap02(VOID);
-VOID FASTCALL KiTrap03Handler(IN PKTRAP_FRAME);
-VOID __cdecl KiTrap08(VOID);
-VOID __cdecl KiTrap09(VOID);
-
-static BOOLEAN
-KdbpInNmiOrDoubleFaultHandler(
-    ULONG_PTR Address)
-{
-    return (Address > (ULONG_PTR)KiTrap02 && Address <
(ULONG_PTR)KiTrap03Handler) ||
-           (Address > (ULONG_PTR)KiTrap08 && Address <
(ULONG_PTR)KiTrap09);
-}
-
 /*!\brief Displays a backtrace.
  */
 static BOOLEAN
@@ -1122,15 +1109,17 @@ KdbpCmdBackTrace(
 {
     ULONG ul;
     ULONGLONG Result = 0;
-    ULONG_PTR Frame = KdbCurrentTrapFrame->Tf.Ebp;
+    KTRAP_FRAME TrapFrame = KdbCurrentTrapFrame->Tf;
+    ULONG_PTR Frame = TrapFrame.Ebp;
     ULONG_PTR Address;
-    KTRAP_FRAME TrapFrame;
+    KDESCRIPTOR Gdtr;
+    USHORT TssSelector;
+    PKTSS Tss;
 
     if (Argc >= 2)
     {
         /* Check for [L count] part */
         ul = 0;
-
         if (strcmp(Argv[Argc-2], "L") == 0)
         {
             ul = strtoul(Argv[Argc-1], NULL, 0);
@@ -1157,7 +1146,7 @@ KdbpCmdBackTrace(
         Argc++;
     }
 
-    /* Check if frame addr or thread id is given. */
+    /* Check if a Frame Address or Thread ID is given */
     if (Argc > 1)
     {
         if (Argv[1][0] == '*')
@@ -1169,7 +1158,7 @@ KdbpCmdBackTrace(
                 return TRUE;
 
             if (Result > (ULONGLONG)(~((ULONG_PTR)0)))
-                KdbpPrint("Warning: Address %I64x is beeing
truncated\n",Result);
+                KdbpPrint("Warning: Address %I64x is beeing truncated\n",
Result);
 
             Frame = (ULONG_PTR)Result;
         }
@@ -1179,66 +1168,95 @@ KdbpCmdBackTrace(
             return TRUE;
         }
     }
-    else
+
+    /* Retrieve the Global Descriptor Table */
+    Ke386GetGlobalDescriptorTable(&Gdtr.Limit);
+
+    /* Retrieve the current (active) TSS */
+    TssSelector = Ke386GetTr();
+    Tss = KdbpRetrieveTss(TssSelector, NULL, &Gdtr);
+    if (KdbpIsNestedTss(TssSelector, Tss))
     {
-        KdbpPrint("Eip:\n");
+        /* Display the active TSS if it is nested */
+        KdbpPrint("[Active TSS 0x%04x @ 0x%p]\n", TssSelector, Tss);
+    }
 
-        /* Try printing the function at EIP */
-        if (!KdbSymPrintAddress((PVOID)KdbCurrentTrapFrame->Tf.Eip,
&KdbCurrentTrapFrame->Tf))
-            KdbpPrint("<%08x>\n", KdbCurrentTrapFrame->Tf.Eip);
+    /* If no Frame Address or Thread ID was given, try printing the function at EIP */
+    if (Argc <= 1)
+    {
+        KdbpPrint("Eip:\n");
+        if (!KdbSymPrintAddress((PVOID)TrapFrame.Eip, &TrapFrame))
+            KdbpPrint("<%08x>\n", TrapFrame.Eip);
         else
             KdbpPrint("\n");
     }
 
-    TrapFrame = KdbCurrentTrapFrame->Tf;
+    /* Walk through the frames */
     KdbpPrint("Frames:\n");
-
     for (;;)
     {
         BOOLEAN GotNextFrame;
 
         if (Frame == 0)
-            break;
+            goto CheckForParentTSS;
 
-        if (!NT_SUCCESS(KdbpSafeReadMemory(&Address, (PVOID)(Frame +
sizeof(ULONG_PTR)), sizeof (ULONG_PTR))))
+        Address = 0;
+        if (!NT_SUCCESS(KdbpSafeReadMemory(&Address, (PVOID)(Frame +
sizeof(ULONG_PTR)), sizeof(ULONG_PTR))))
         {
             KdbpPrint("Couldn't access memory at 0x%p!\n", Frame +
sizeof(ULONG_PTR));
-            break;
+            goto CheckForParentTSS;
         }
 
-        if ((GotNextFrame = NT_SUCCESS(KdbpSafeReadMemory(&Frame, (PVOID)Frame,
sizeof (ULONG_PTR)))))
+        if (Address == 0)
+            goto CheckForParentTSS;
+
+        GotNextFrame = NT_SUCCESS(KdbpSafeReadMemory(&Frame, (PVOID)Frame,
sizeof(ULONG_PTR)));
+        if (GotNextFrame)
             TrapFrame.Ebp = Frame;
+        // else
+            // Frame = 0;
 
-        /* Print the location of the call instruction */
+        /* Print the location of the call instruction (assumed 5 bytes length) */
         if (!KdbSymPrintAddress((PVOID)(Address - 5), &TrapFrame))
             KdbpPrint("<%08x>\n", Address);
         else
             KdbpPrint("\n");
 
-        if (KdbOutputAborted) break;
-
-        if (Address == 0)
+        if (KdbOutputAborted)
             break;
 
-        if (KdbpInNmiOrDoubleFaultHandler(Address))
+        if (!GotNextFrame)
         {
-            if ((GotNextFrame = KdbpTrapFrameFromPrevTss(&TrapFrame)))
-            {
-                Address = TrapFrame.Eip;
-                Frame = TrapFrame.Ebp;
-
-                if (!KdbSymPrintAddress((PVOID)Address, &TrapFrame))
-                    KdbpPrint("<%08x>\n", Address);
-                else
-                    KdbpPrint("\n");
-            }
+            KdbpPrint("Couldn't access memory at 0x%p!\n", Frame);
+            goto CheckForParentTSS; // break;
         }
 
+        continue;
+
+CheckForParentTSS:
+        /*
+         * We have ended the stack walking for the current (active) TSS.
+         * Check whether this TSS was nested, and if so switch to its parent
+         * and walk its stack.
+         */
+        if (!KdbpIsNestedTss(TssSelector, Tss))
+            break; // The TSS is not nested, we stop there.
+
+        GotNextFrame = KdbpTrapFrameFromPrevTss(&TrapFrame, &TssSelector,
&Tss, &Gdtr);
         if (!GotNextFrame)
         {
-            KdbpPrint("Couldn't access memory at 0x%p!\n", Frame);
-            break;
+            KdbpPrint("Couldn't access parent TSS 0x%04x\n",
Tss->Backlink);
+            break; // Cannot retrieve the parent TSS, we stop there.
         }
+        Address = TrapFrame.Eip;
+        Frame = TrapFrame.Ebp;
+
+        KdbpPrint("[Parent TSS 0x%04x @ 0x%p]\n", TssSelector, Tss);
+
+        if (!KdbSymPrintAddress((PVOID)Address, &TrapFrame))
+            KdbpPrint("<%08x>\n", Address);
+        else
+            KdbpPrint("\n");
     }
 
     return TRUE;