[reactos] 03/03: [NTOS:KDBG] Reintroduce the capability of KdbpCliInit() to interpret the KDBinit file (#4917)

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

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

commit b15963abb821c4557fc840fa9b1a1b25b4123ade
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Mon Nov 28 03:13:31 2022 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Mon Oct 14 22:51:54 2024 +0200

    [NTOS:KDBG] Reintroduce the capability of KdbpCliInit() to interpret the KDBinit file
(#4917)
    
    Addendum to commit baa47fa5e.
    
    Similarly to what was originally done, have KdbpCliInterpretInitFile()
    parse the KDBinit file by breaking back into the debugger.
    But contrary to before, replace the deprecated call to KdbEnter() by
    a standard DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C) . This allows
    KdbEnterDebuggerException() to do the KdbpCliInterpretInitFile() call.
    
    Additional fixes and improvements:
    
    - Run KdbpCliInterpretInitFile() in full KDBG environment (interrupts
      disabled, modified IRQL, own stack), like the usual interactive loop.
    
    - The KDBinit data buffer must be in non-paged pool.
    
    - Demote the "Could not open KDBinit" error to a DPRINT, so that it
      doesn't pollute the debug log when the KDBG init function is called
      early (before the storage stack is initialized), or if the file
      doesn't exist -- since this is an optional feature.
---
 ntoskrnl/kdbg/kdb.c     | 37 ++++++++++++++++++--------
 ntoskrnl/kdbg/kdb.h     |  2 +-
 ntoskrnl/kdbg/kdb_cli.c | 70 +++++++++++++++++++++++++------------------------
 3 files changed, 63 insertions(+), 46 deletions(-)

diff --git a/ntoskrnl/kdbg/kdb.c b/ntoskrnl/kdbg/kdb.c
index bb5b3afe0af..05b8bc0765f 100644
--- a/ntoskrnl/kdbg/kdb.c
+++ b/ntoskrnl/kdbg/kdb.c
@@ -1141,20 +1141,29 @@ KdbpAttachToProcess(
     return KdbpAttachToThread(Thread->Cid.UniqueThread);
 }
 
-/*!\brief Calls the main loop ...
- */
+/**
+ * @brief   Calls the main interactive debugger loop.
+ **/
 static VOID
 KdbpCallMainLoop(VOID)
 {
     KdbpCliMainLoop(KdbEnteredOnSingleStep);
 }
 
-/*!\brief Internal function to enter KDB.
+/**
+ * @brief
+ * Internal function to enter KDBG and run the specified procedure.
  *
  * Disables interrupts, releases display ownership, ...
- */
+ *
+ * @param[in]   Procedure
+ * The procedure to execute under the KDBG environment.
+ * Either execute the main interactive debugger loop (KdbpCallMainLoop)
+ * or run the KDBinit file (KdbpCliInterpretInitFile).
+ **/
 static VOID
-KdbpInternalEnter(VOID)
+KdbpInternalEnter(
+    _In_ VOID (*Procedure)(VOID))
 {
     PETHREAD Thread;
     PVOID SavedInitialStack, SavedStackBase, SavedKernelStack;
@@ -1166,7 +1175,7 @@ KdbpInternalEnter(VOID)
     if (KdpDebugMode.Screen)
         KdpScreenAcquire();
 
-    /* Call the interface's main loop on a different stack */
+    /* Call the specified debugger procedure on a different stack */
     Thread = PsGetCurrentThread();
     SavedInitialStack = Thread->Tcb.InitialStack;
     SavedStackBase = Thread->Tcb.StackBase;
@@ -1179,7 +1188,7 @@ KdbpInternalEnter(VOID)
     // KdbPrintf("Switching to KDB stack 0x%08x-0x%08x (Current Stack is
0x%08x)\n",
     //           Thread->Tcb.StackLimit, Thread->Tcb.StackBase, Esp);
 
-    KdbpStackSwitchAndCall(KdbStack + KDB_STACK_SIZE - KDB_STACK_RESERVE,
KdbpCallMainLoop);
+    KdbpStackSwitchAndCall(KdbStack + KDB_STACK_SIZE - KDB_STACK_RESERVE, Procedure);
 
     Thread->Tcb.InitialStack = SavedInitialStack;
     Thread->Tcb.StackBase = SavedStackBase;
@@ -1276,6 +1285,7 @@ KdbEnterDebuggerException(
     ULONG OldEflags;
     KIRQL OldIrql;
     NTSTATUS ExceptionCode;
+    VOID (*EntryPoint)(VOID) = KdbpCallMainLoop;
 
     ExceptionCode = (ExceptionRecord ? ExceptionRecord->ExceptionCode :
STATUS_BREAKPOINT);
 
@@ -1481,11 +1491,15 @@ KdbEnterDebuggerException(
     }
     else if (ExceptionCode == STATUS_BREAKPOINT)
     {
+        /* Do the condition check and banner display only if we enter
+         * from a true code breakpoint. We skip those when running the
+         * KDBinit file, because it is done via an artificial breakpoint. */
         if (KdbInitFileBuffer)
         {
-            KdbpCliInterpretInitFile();
-            EnterConditionMet = FALSE;
+            EntryPoint = KdbpCliInterpretInitFile;
+            goto EnterKdbg;
         }
+
         if (!EnterConditionMet)
         {
             return kdHandleException;
@@ -1493,6 +1507,7 @@ KdbEnterDebuggerException(
 
         KdbPrintf("\nEntered debugger on embedded INT3 at 0x%04x:0x%p.\n",
                   Context->SegCs & 0xffff, KeGetContextPc(Context));
+EnterKdbg:;
     }
     else
     {
@@ -1543,8 +1558,8 @@ KdbEnterDebuggerException(
         return kdHandleException;
     }
 
-    /* Call the main loop */
-    KdbpInternalEnter();
+    /* Enter KDBG proper and run either the main loop or the KDBinit file */
+    KdbpInternalEnter(EntryPoint);
 
     /* Check if we should single step */
     if (KdbNumSingleSteps > 0)
diff --git a/ntoskrnl/kdbg/kdb.h b/ntoskrnl/kdbg/kdb.h
index df2945a7d9c..b76c4706bc5 100644
--- a/ntoskrnl/kdbg/kdb.h
+++ b/ntoskrnl/kdbg/kdb.h
@@ -61,7 +61,7 @@ typedef enum _KD_CONTINUE_TYPE
 
 /* GLOBALS *******************************************************************/
 
-extern PCHAR KdbInitFileBuffer;
+extern volatile PCHAR KdbInitFileBuffer;
 
 extern PEPROCESS KdbCurrentProcess;
 extern PETHREAD KdbCurrentThread;
diff --git a/ntoskrnl/kdbg/kdb_cli.c b/ntoskrnl/kdbg/kdb_cli.c
index 08c33d8afb8..852326cff22 100644
--- a/ntoskrnl/kdbg/kdb_cli.c
+++ b/ntoskrnl/kdbg/kdb_cli.c
@@ -134,7 +134,7 @@ static ULONG KdbNumberOfColsPrinted = 0;
 static BOOLEAN KdbOutputAborted = FALSE;
 static BOOLEAN KdbRepeatLastCommand = FALSE;
 
-PCHAR KdbInitFileBuffer = NULL; /* Buffer where KDBinit file is loaded into during
initialization */
+volatile PCHAR KdbInitFileBuffer = NULL; /* Buffer where KDBinit file is loaded into
during initialization */
 BOOLEAN KdbpBugCheckRequested = FALSE;
 
 /* Variables for Dmesg */
@@ -3330,20 +3330,28 @@ KdbpCliMainLoop(
     }
 }
 
-/*!\brief This function is called by KdbEnterDebuggerException...
+/**
+ * @brief
+ * Interprets the KDBinit file from the \SystemRoot\System32\drivers\etc
+ * directory, that has been loaded by KdbpCliInit().
  *
- * Used to interpret the init file in a context with a trapframe setup
- * (KdbpCliInit call KdbEnter which will call KdbEnterDebuggerException which will
- * call this function if KdbInitFileBuffer is not NULL.
- */
+ * This function is used to interpret the init file in the debugger context
+ * with a trap frame set up. KdbpCliInit() enters the debugger by calling
+ * DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C). In turn, this will call
+ * KdbEnterDebuggerException() which will finally call this function if
+ * KdbInitFileBuffer is not NULL.
+ **/
 VOID
 KdbpCliInterpretInitFile(VOID)
 {
     PCHAR p1, p2;
 
+    p1 = InterlockedExchangePointer((PVOID*)&KdbInitFileBuffer, NULL);
+    if (!p1)
+        return;
+
     /* Execute the commands in the init file */
-    DPRINT("KDB: Executing KDBinit file...\n");
-    p1 = KdbInitFileBuffer;
+    KdbPuts("KDB: Executing KDBinit file...\n");
     while (p1[0] != '\0')
     {
         size_t i = strcspn(p1, "\r\n");
@@ -3360,11 +3368,12 @@ KdbpCliInterpretInitFile(VOID)
             if (strncmp(p2, "break", sizeof("break")-1) == 0
&&
                 (p2[sizeof("break")-1] == '\0' ||
isspace(p2[sizeof("break")-1])))
             {
-                /* break into the debugger */
+                /* Run the interactive debugger loop */
                 KdbpCliMainLoop(FALSE);
             }
             else if (p2[0] != '#' && p2[0] != '\0') /* Ignore
empty lines and comments */
             {
+                /* Invoke the command */
                 KdbpDoCommand(p1);
             }
 
@@ -3375,14 +3384,14 @@ KdbpCliInterpretInitFile(VOID)
         while (p1[0] == '\r' || p1[0] == '\n')
             p1++;
     }
-    DPRINT("KDB: KDBinit executed\n");
+    KdbPuts("KDB: KDBinit executed\n");
 }
 
 /**
  * @brief   Called when KDB is initialized.
  *
- * Reads the KDBinit file from the SystemRoot\System32\drivers\etc directory
- * and executes it.
+ * Loads the KDBinit file from the \SystemRoot\System32\drivers\etc
+ * directory and interprets it, by calling back into the debugger.
  **/
 NTSTATUS
 KdbpCliInit(VOID)
@@ -3393,9 +3402,8 @@ KdbpCliInit(VOID)
     IO_STATUS_BLOCK Iosb;
     FILE_STANDARD_INFORMATION FileStdInfo;
     HANDLE hFile = NULL;
-    INT FileSize;
+    ULONG FileSize;
     PCHAR FileBuffer;
-    ULONG OldEflags;
 
     /* Don't load the KDBinit file if its buffer is already lying around */
     if (KdbInitFileBuffer)
@@ -3416,7 +3424,7 @@ KdbpCliInit(VOID)
                         FILE_NO_INTERMEDIATE_BUFFERING);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("Could not open \\SystemRoot\\System32\\drivers\\etc\\KDBinit
(Status 0x%lx)\n", Status);
+        DPRINT("Could not open %wZ (Status 0x%lx)\n", &FileName, Status);
         return Status;
     }
 
@@ -3427,22 +3435,23 @@ KdbpCliInit(VOID)
     if (!NT_SUCCESS(Status))
     {
         ZwClose(hFile);
-        DPRINT1("Could not query size of
\\SystemRoot\\System32\\drivers\\etc\\KDBinit (Status 0x%lx)\n", Status);
+        DPRINT1("Could not query size of %wZ (Status 0x%lx)\n", &FileName,
Status);
         return Status;
     }
     FileSize = FileStdInfo.EndOfFile.u.LowPart;
 
-    /* Allocate memory for the file */
-    FileBuffer = ExAllocatePool(PagedPool, FileSize + 1); /* add 1 byte for terminating
'\0' */
+    /* Allocate memory for the file (add 1 byte for terminating NUL) */
+    FileBuffer = ExAllocatePool(NonPagedPool, FileSize + 1);
     if (!FileBuffer)
     {
         ZwClose(hFile);
-        DPRINT1("Could not allocate %d bytes for KDBinit file\n", FileSize);
+        DPRINT1("Could not allocate %lu bytes for KDBinit file\n", FileSize);
         return Status;
     }
 
     /* Load file into memory */
-    Status = ZwReadFile(hFile, NULL, NULL, NULL, &Iosb, FileBuffer, FileSize, NULL,
NULL);
+    Status = ZwReadFile(hFile, NULL, NULL, NULL, &Iosb,
+                        FileBuffer, FileSize, NULL, NULL);
     ZwClose(hFile);
 
     if (!NT_SUCCESS(Status) && (Status != STATUS_END_OF_FILE))
@@ -3452,20 +3461,13 @@ KdbpCliInit(VOID)
         return Status;
     }
 
-    FileSize = min(FileSize, (INT)Iosb.Information);
-    FileBuffer[FileSize] = '\0';
-
-    /* Enter critical section */
-    OldEflags = __readeflags();
-    _disable();
-
-    /* Interpret the init file... */
-    KdbInitFileBuffer = FileBuffer;
-    //KdbEnter(); // FIXME, see commit baa47fa5e
-    KdbInitFileBuffer = NULL;
+    FileSize = min(FileSize, (ULONG)Iosb.Information);
+    FileBuffer[FileSize] = ANSI_NULL;
 
-    /* Leave critical section */
-    __writeeflags(OldEflags);
+    /* Interpret the KDBinit file by calling back into the debugger */
+    InterlockedExchangePointer((PVOID*)&KdbInitFileBuffer, FileBuffer);
+    DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C);
+    InterlockedExchangePointer((PVOID*)&KdbInitFileBuffer, NULL);
 
     ExFreePool(FileBuffer);
 
@@ -3607,7 +3609,7 @@ KdbInitialize(
 
     if (BootPhase >= 2)
     {
-        /* I/O is now set up for disk access: Read KDB Data */
+        /* I/O is now set up for disk access: load the KDBinit file */
         NTSTATUS Status = KdbpCliInit();
 
         /* Schedule an I/O reinitialization if needed */