[ros-diffs] [reactos] 08/13: [NTOS:KD64][NTOS:KDBG] Fix spinlocks use

[Jérôme Gardou]

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

commit 608032bd0833a4c6e769a68e57bfc4a45eed79b9
Author:     Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Tue Jun 22 11:41:25 2021 +0200
Commit:     Jérôme Gardou <zefklop(a)users.noreply.github.com>
CommitDate: Tue Jun 29 11:49:20 2021 +0200

    [NTOS:KD64][NTOS:KDBG] Fix spinlocks use
    
    Raise IRQL before entering debugger, so that KeAcquireSpinLockAtDpcLevel works as
expected.
     - HIGH_LEVEL since we don't know where we are coming from.
    
    Do not try to read debug symbol from files in KDBG.
     - There is no reason that this works if Mm didn't map it in the first place.
---
 ntoskrnl/kd64/kdlock.c      |   3 ++
 ntoskrnl/kd64/kdtrap.c      |   6 +++
 ntoskrnl/kdbg/kdb_symbols.c | 108 ++++++--------------------------------------
 3 files changed, 24 insertions(+), 93 deletions(-)

diff --git a/ntoskrnl/kd64/kdlock.c b/ntoskrnl/kd64/kdlock.c
index 25e4cde3dc9..6b1512fa91b 100644
--- a/ntoskrnl/kd64/kdlock.c
+++ b/ntoskrnl/kd64/kdlock.c
@@ -92,7 +92,9 @@ KdPollBreakIn(VOID)
         }
         else
         {
+            KIRQL OldIrql;
             /* Try to acquire the lock */
+            KeRaiseIrql(HIGH_LEVEL, &OldIrql);
             if (KeTryToAcquireSpinLockAtDpcLevel(&KdpDebuggerLock))
             {
                 /* Now get a packet */
@@ -110,6 +112,7 @@ KdPollBreakIn(VOID)
                 /* Let go of the port */
                 KdpPortUnlock();
             }
+            KeLowerIrql(OldIrql);
         }
 
         /* Re-enable interrupts if they were enabled previously */
diff --git a/ntoskrnl/kd64/kdtrap.c b/ntoskrnl/kd64/kdtrap.c
index dc78b11891e..b8314a94685 100644
--- a/ntoskrnl/kd64/kdtrap.c
+++ b/ntoskrnl/kd64/kdtrap.c
@@ -144,6 +144,10 @@ KdpTrap(IN PKTRAP_FRAME TrapFrame,
     BOOLEAN Handled;
     NTSTATUS ReturnStatus;
     USHORT ReturnLength;
+    KIRQL OldIrql;
+
+    /* Raise as high as we can. */
+    KeRaiseIrql(HIGH_LEVEL, &OldIrql);
 
     /*
      * Check if we got a STATUS_BREAKPOINT with a SubID for Print, Prompt or
@@ -257,6 +261,8 @@ KdpTrap(IN PKTRAP_FRAME TrapFrame,
                             SecondChanceException);
     }
 
+    KeLowerIrql(OldIrql);
+
     /* Return TRUE or FALSE to caller */
     return Handled;
 }
diff --git a/ntoskrnl/kdbg/kdb_symbols.c b/ntoskrnl/kdbg/kdb_symbols.c
index e3673e8db6d..a066e39d8b7 100644
--- a/ntoskrnl/kdbg/kdb_symbols.c
+++ b/ntoskrnl/kdbg/kdb_symbols.c
@@ -244,11 +244,10 @@ KdbpSymFindCachedFile(
 {
     PIMAGE_SYMBOL_INFO_CACHE Current;
     PLIST_ENTRY CurrentEntry;
-    KIRQL Irql;
 
     DPRINT("Looking for cached symbol file %wZ\n", FileName);
 
-    KeAcquireSpinLock(&SymbolFileListLock, &Irql);
+    KeAcquireSpinLockAtDpcLevel(&SymbolFileListLock);
 
     CurrentEntry = SymbolFileListHead.Flink;
     while (CurrentEntry != (&SymbolFileListHead))
@@ -259,7 +258,7 @@ KdbpSymFindCachedFile(
         if (RtlEqualUnicodeString(&Current->FileName, FileName, TRUE))
         {
             Current->RefCount++;
-            KeReleaseSpinLock(&SymbolFileListLock, Irql);
+            KeReleaseSpinLockFromDpcLevel(&SymbolFileListLock);
             DPRINT("Found cached file!\n");
             return Current->RosSymInfo;
         }
@@ -267,7 +266,7 @@ KdbpSymFindCachedFile(
         CurrentEntry = CurrentEntry->Flink;
     }
 
-    KeReleaseSpinLock(&SymbolFileListLock, Irql);
+    KeReleaseSpinLockFromDpcLevel(&SymbolFileListLock);
 
     DPRINT("Cached file not found!\n");
     return NULL;
@@ -355,81 +354,6 @@ KdbpSymRemoveCachedFile(
     DPRINT1("Warning: Removing unknown symbol file: RosSymInfo = %p\n",
RosSymInfo);
 }
 
-/*! \brief Loads a symbol file.
- *
- * \param FileName    Filename of the symbol file to load.
- * \param RosSymInfo  Pointer to a ROSSYM_INFO which gets filled.
- *
- * \sa KdbpSymUnloadModuleSymbols
- */
-static VOID
-KdbpSymLoadModuleSymbols(
-    IN PUNICODE_STRING FileName,
-    OUT PROSSYM_INFO *RosSymInfo)
-{
-    OBJECT_ATTRIBUTES ObjectAttributes;
-    HANDLE FileHandle;
-    NTSTATUS Status;
-    IO_STATUS_BLOCK IoStatusBlock;
-    BOOLEAN Result;
-
-    /* Allow KDB to break on module load */
-    KdbModuleLoaded(FileName);
-
-    if (!LoadSymbols)
-    {
-        *RosSymInfo = NULL;
-        return;
-    }
-
-    /*  Try to find cached (already loaded) symbol file  */
-    *RosSymInfo = KdbpSymFindCachedFile(FileName);
-    if (*RosSymInfo)
-    {
-        DPRINT("Found cached symbol file %wZ\n", FileName);
-        return;
-    }
-
-    /*  Open the file  */
-    InitializeObjectAttributes(&ObjectAttributes,
-                               FileName,
-                               OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE,
-                               NULL,
-                               NULL);
-
-    DPRINT("Attempting to open image: %wZ\n", FileName);
-
-    Status = ZwOpenFile(&FileHandle,
-                        FILE_READ_ACCESS | SYNCHRONIZE,
-                        &ObjectAttributes,
-                        &IoStatusBlock,
-                        FILE_SHARE_READ | FILE_SHARE_WRITE,
-                        FILE_SYNCHRONOUS_IO_NONALERT);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT("Could not open image file: %wZ\n", FileName);
-        return;
-    }
-
-    DPRINT("Loading symbols from %wZ...\n", FileName);
-
-    Result = RosSymCreateFromFile(&FileHandle, RosSymInfo);
-    ZwClose(FileHandle);
-
-    if (!Result)
-    {
-        DPRINT("Failed to load symbols from %wZ\n", FileName);
-        return;
-    }
-
-    DPRINT("Symbols loaded.\n");
-
-    /* add file to cache */
-    KdbpSymAddCachedFile(FileName, *RosSymInfo);
-
-    DPRINT("Installed symbols: %wZ %p\n", FileName, *RosSymInfo);
-}
-
 VOID
 KdbSymProcessSymbols(
     IN PLDR_DATA_TABLE_ENTRY LdrEntry)
@@ -444,21 +368,17 @@ KdbSymProcessSymbols(
     if (LdrEntry->PatchInformation)
         KdbpSymRemoveCachedFile(LdrEntry->PatchInformation);
 
-    /* Load new symbol information */
-    if (! RosSymCreateFromMem(LdrEntry->DllBase,
-        LdrEntry->SizeOfImage,
-        (PROSSYM_INFO*)&LdrEntry->PatchInformation))
-    {
-        /* Error loading symbol info, try to load it from file */
-        KdbpSymLoadModuleSymbols(&LdrEntry->FullDllName,
-            (PROSSYM_INFO*)&LdrEntry->PatchInformation);
+    /* Check cache */
+    LdrEntry->PatchInformation =
KdbpSymFindCachedFile(&LdrEntry->FullDllName);
 
-        /* It already added symbols to cache */
-    }
-    else
+    if (!LdrEntry->PatchInformation)
     {
-        /* Add file to cache */
-        KdbpSymAddCachedFile(&LdrEntry->FullDllName,
LdrEntry->PatchInformation);
+        /* Load new symbol information */
+        if (RosSymCreateFromMem(LdrEntry->DllBase, LdrEntry->SizeOfImage,
(PROSSYM_INFO*)&LdrEntry->PatchInformation))
+        {
+            /* Add file to cache */
+            KdbpSymAddCachedFile(&LdrEntry->FullDllName,
LdrEntry->PatchInformation);
+        }
     }
 
     DPRINT("Installed symbols: %wZ@%p-%p %p\n",
@@ -466,7 +386,6 @@ KdbSymProcessSymbols(
            LdrEntry->DllBase,
            (PVOID)(LdrEntry->SizeOfImage + (ULONG_PTR)LdrEntry->DllBase),
            LdrEntry->PatchInformation);
-
 }
 
 VOID
@@ -567,14 +486,17 @@ KdbInitialize(
     }
     else if (BootPhase == 1)
     {
+        KIRQL OldIrql;
         /* Load symbols for NTOSKRNL.EXE.
            It is always the first module in PsLoadedModuleList. KeLoaderBlock can't
be used here as its content is just temporary. */
+        OldIrql = KeRaiseIrqlToDpcLevel();
         LdrEntry = CONTAINING_RECORD(PsLoadedModuleList.Flink, LDR_DATA_TABLE_ENTRY,
InLoadOrderLinks);
         KdbSymProcessSymbols(LdrEntry);
 
         /* Also load them for HAL.DLL. */
         LdrEntry = CONTAINING_RECORD(PsLoadedModuleList.Flink->Flink,
LDR_DATA_TABLE_ENTRY, InLoadOrderLinks);
         KdbSymProcessSymbols(LdrEntry);
+        KeLowerIrql(OldIrql);
 
         KdbpSymbolsInitialized = TRUE;
     }