[ros-diffs] [reactos] 26/29: [KDGDB] Avoid looping for ever when something unexpected happens

[Jérôme Gardou]

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

commit 3ee1eb42a5ec447df568d8dd7d9ba69d96282e47
Author:     Jérôme Gardou <jerome.gardou(a)reactos.org>
AuthorDate: Fri Apr 23 15:26:33 2021 +0200
Commit:     Jérôme Gardou <zefklop(a)users.noreply.github.com>
CommitDate: Wed Apr 28 13:10:23 2021 +0200

    [KDGDB] Avoid looping for ever when something unexpected happens
---
 drivers/base/kdgdb/gdb_input.c | 25 +++++++++++++++++--------
 drivers/base/kdgdb/kdgdb.h     |  2 +-
 drivers/base/kdgdb/kdpacket.c  | 36 +++++++++++++++---------------------
 3 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/base/kdgdb/gdb_input.c b/drivers/base/kdgdb/gdb_input.c
index 23f030686d4..c10d7750c84 100644
--- a/drivers/base/kdgdb/gdb_input.c
+++ b/drivers/base/kdgdb/gdb_input.c
@@ -430,7 +430,7 @@ handle_gdb_registers(
 #endif
 
 static
-void
+BOOLEAN
 ReadMemorySendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -442,12 +442,13 @@ ReadMemorySendHandler(
     {
         // KdAssert
         KDDBGPRINT("Wrong packet type (%lu) received after DbgKdReadVirtualMemoryApi
request.\n", PacketType);
-        while (1);
+        return FALSE;
     }
 
     if (State->ApiNumber != DbgKdReadVirtualMemoryApi)
     {
         KDDBGPRINT("Wrong API number (%lu) after DbgKdReadVirtualMemoryApi
request.\n", State->ApiNumber);
+        return FALSE;
     }
 
     /* Check status. Allow to send partial data. */
@@ -469,6 +470,8 @@ ReadMemorySendHandler(
         if (ProcessListHead->Flink)
             __writecr3(PsGetCurrentProcess()->Pcb.DirectoryTableBase[0]);
     }
+
+    return TRUE;
 }
 
 static
@@ -531,7 +534,7 @@ handle_gdb_read_mem(
 }
 
 static
-void
+BOOLEAN
 WriteMemorySendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -543,12 +546,13 @@ WriteMemorySendHandler(
     {
         // KdAssert
         KDDBGPRINT("Wrong packet type (%lu) received after
DbgKdWriteVirtualMemoryApi request.\n", PacketType);
-        while (1);
+        return FALSE;
     }
 
     if (State->ApiNumber != DbgKdWriteVirtualMemoryApi)
     {
         KDDBGPRINT("Wrong API number (%lu) after DbgKdWriteVirtualMemoryApi
request.\n", State->ApiNumber);
+        return FALSE;
     }
 
     /* Check status */
@@ -570,6 +574,7 @@ WriteMemorySendHandler(
         if (ProcessListHead->Flink)
             __writecr3(PsGetCurrentProcess()->Pcb.DirectoryTableBase[0]);
     }
+    return TRUE;
 }
 
 static
@@ -669,7 +674,7 @@ handle_gdb_write_mem(
 }
 
 static
-void
+BOOLEAN
 WriteBreakPointSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -681,12 +686,13 @@ WriteBreakPointSendHandler(
     {
         // KdAssert
         KDDBGPRINT("Wrong packet type (%lu) received after DbgKdWriteBreakPointApi
request.\n", PacketType);
-        while (1);
+        return FALSE;
     }
 
     if (State->ApiNumber != DbgKdWriteBreakPointApi)
     {
         KDDBGPRINT("Wrong API number (%lu) after DbgKdWriteBreakPointApi
request.\n", State->ApiNumber);
+        return FALSE;
     }
 
     /* Check status */
@@ -712,6 +718,7 @@ WriteBreakPointSendHandler(
     }
     KdpSendPacketHandler = NULL;
     KdpManipulateStateHandler = NULL;
+    return TRUE;
 }
 
 static
@@ -767,7 +774,7 @@ handle_gdb_insert_breakpoint(
 }
 
 static
-void
+BOOLEAN
 RestoreBreakPointSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -780,12 +787,13 @@ RestoreBreakPointSendHandler(
     {
         // KdAssert
         KDDBGPRINT("Wrong packet type (%lu) received after DbgKdRestoreBreakPointApi
request.\n", PacketType);
-        while (1);
+        return FALSE;
     }
 
     if (State->ApiNumber != DbgKdRestoreBreakPointApi)
     {
         KDDBGPRINT("Wrong API number (%lu) after DbgKdRestoreBreakPointApi
request.\n", State->ApiNumber);
+        return FALSE;
     }
 
     /* We ignore failure here. If DbgKdRestoreBreakPointApi fails,
@@ -804,6 +812,7 @@ RestoreBreakPointSendHandler(
 
     KdpSendPacketHandler = NULL;
     KdpManipulateStateHandler = NULL;
+    return TRUE;
 }
 
 static
diff --git a/drivers/base/kdgdb/kdgdb.h b/drivers/base/kdgdb/kdgdb.h
index a260e570319..4e365b77e27 100644
--- a/drivers/base/kdgdb/kdgdb.h
+++ b/drivers/base/kdgdb/kdgdb.h
@@ -57,7 +57,7 @@ InitManipulateFromStateChange(
 }
 
 /* Callbacks to simulate a KdReceive <-> KdSend loop without GDB being aware of it
*/
-typedef VOID (*KDP_SEND_HANDLER)(
+typedef BOOLEAN (*KDP_SEND_HANDLER)(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
     _In_ PSTRING MessageData
diff --git a/drivers/base/kdgdb/kdpacket.c b/drivers/base/kdgdb/kdpacket.c
index dfe4ee7f261..ba5cd44dd25 100644
--- a/drivers/base/kdgdb/kdpacket.c
+++ b/drivers/base/kdgdb/kdpacket.c
@@ -9,7 +9,7 @@
 
 /* LOCALS *********************************************************************/
 static
-VOID
+BOOLEAN
 FirstSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -33,7 +33,7 @@ PETHREAD TheIdleThread;
 /* PRIVATE FUNCTIONS **********************************************************/
 
 static
-VOID
+BOOLEAN
 GetContextSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -47,14 +47,14 @@ GetContextSendHandler(
             || (State->ApiNumber != DbgKdGetContextApi)
             || (MessageData->Length < sizeof(*Context)))
     {
-        /* Should we bugcheck ? */
         KDDBGPRINT("ERROR: Received wrong packet from KD.\n");
-        while (1);
+        return FALSE;
     }
 
     /* Just copy it */
     RtlCopyMemory(&CurrentContext, Context, sizeof(*Context));
     KdpSendPacketHandler = NULL;
+    return TRUE;
 }
 
 static
@@ -80,7 +80,7 @@ GetContextManipulateHandler(
 }
 
 static
-VOID
+BOOLEAN
 SetContextSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -96,10 +96,11 @@ SetContextSendHandler(
     {
         /* Should we bugcheck ? */
         KDDBGPRINT("BAD BAD BAD not manipulating state for sending
context.\n");
-        while (1);
+        return FALSE;
     }
 
     KdpSendPacketHandler = NULL;
+    return TRUE;
 }
 
 KDSTATUS
@@ -236,7 +237,7 @@ ContinueManipulateStateHandler(
 }
 
 static
-VOID
+BOOLEAN
 GetVersionSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -250,9 +251,8 @@ GetVersionSendHandler(
             || (State->ApiNumber != DbgKdGetVersionApi)
             || !NT_SUCCESS(State->ReturnStatus))
     {
-        /* FIXME: should detach from KD and go along without debugging */
         KDDBGPRINT("Wrong packet received after asking for data.\n");
-        while(1);
+        return FALSE;
     }
 
     /* Copy the relevant data */
@@ -265,6 +265,7 @@ GetVersionSendHandler(
     /* Now we can get the context for the current state */
     KdpSendPacketHandler = NULL;
     KdpManipulateStateHandler = GetContextManipulateHandler;
+    return TRUE;
 }
 
 static
@@ -288,7 +289,7 @@ GetVersionManipulateStateHandler(
 }
 
 static
-VOID
+BOOLEAN
 FirstSendHandler(
     _In_ ULONG PacketType,
     _In_ PSTRING MessageHeader,
@@ -297,18 +298,10 @@ FirstSendHandler(
     DBGKD_ANY_WAIT_STATE_CHANGE* StateChange =
(DBGKD_ANY_WAIT_STATE_CHANGE*)MessageHeader->Buffer;
     PETHREAD Thread;
 
-    if (PacketType == PACKET_TYPE_KD_DEBUG_IO)
-    {
-        /* This is not the packet we are waiting for */
-        send_kd_debug_io((DBGKD_DEBUG_IO*)MessageHeader->Buffer, MessageData);
-        return;
-    }
-
     if (PacketType != PACKET_TYPE_KD_STATE_CHANGE64)
     {
         KDDBGPRINT("First KD packet is not a state change!\n");
-        /* FIXME: What should we send back to KD ? */
-        while(1);
+        return FALSE;
     }
 
     KDDBGPRINT("KDGDB: START!\n");
@@ -332,6 +325,7 @@ FirstSendHandler(
     /* The next receive call will be asking for the version data */
     KdpSendPacketHandler = NULL;
     KdpManipulateStateHandler = GetVersionManipulateStateHandler;
+    return TRUE;
 }
 
 /* PUBLIC FUNCTIONS ***********************************************************/
@@ -426,9 +420,9 @@ KdSendPacket(
     }
 
     /* Maybe we are in a send <-> receive loop that GDB doesn't need to know
about */
-    if (KdpSendPacketHandler)
+    if (KdpSendPacketHandler
+        && KdpSendPacketHandler(PacketType, MessageHeader, MessageData))
     {
-        KdpSendPacketHandler(PacketType, MessageHeader, MessageData);
         return;
     }