[ros-diffs] [blight] 12973: Add possibility to make KDB break on module-loads. Fix handling of breakpoints in usermode with KDB. Set ExceptionRecord->ExceptionFlags to 0 for breakpoints/singlesteps and noncontinuable for everything else. Fix WriteProcessMemory.

Add possibility to make KDB break on module-loads. Fix handling of
breakpoints in usermode with KDB. Set ExceptionRecord->ExceptionFlags to
0 for breakpoints/singlesteps and noncontinuable for everything else.
Fix WriteProcessMemory.
Modified: trunk/reactos/ntoskrnl/dbg/kdb.c
Modified: trunk/reactos/ntoskrnl/dbg/kdb.h
Modified: trunk/reactos/ntoskrnl/dbg/kdb_symbols.c
Modified: trunk/reactos/ntoskrnl/ke/catch.c
Modified: trunk/reactos/ntoskrnl/ke/i386/exp.c
Modified: trunk/reactos/ntoskrnl/ke/i386/usertrap.c
Modified: trunk/reactos/ntoskrnl/mm/virtual.c
  _____  

Modified: trunk/reactos/ntoskrnl/dbg/kdb.c
--- trunk/reactos/ntoskrnl/dbg/kdb.c	2005-01-12 16:09:12 UTC (rev
12972)
+++ trunk/reactos/ntoskrnl/dbg/kdb.c	2005-01-12 19:04:06 UTC (rev
12973)
@@ -60,8 +60,9 @@

 
 static BOOLEAN KdbHandleUmode = FALSE;
 static BOOLEAN KdbHandleHandled = FALSE;
+static BOOLEAN KdbBreakOnModuleLoad = FALSE;
+
 static BOOLEAN KdbIgnoreNextSingleStep = FALSE;
-
 static ULONG KdbLastSingleStepFrom = 0xFFFFFFFF;
 static BOOLEAN KdbEnteredOnSingleStep = FALSE;
 
@@ -75,6 +76,8 @@
 ULONG 
 DbgStopCondition(ULONG Aargc, PCH Argv[], PKTRAP_FRAME Tf);
 ULONG
+DbgModuleLoadedAction(ULONG Aargc, PCH Argv[], PKTRAP_FRAME Tf);
+ULONG
 DbgEchoToggle(ULONG Argc, PCH Argv[], PKTRAP_FRAME Tf);
 ULONG 
 DbgRegsCommand(ULONG Argc, PCH Argv[], PKTRAP_FRAME Tf);
@@ -129,7 +132,8 @@
   {"cont", "cont", "Exit the debugger", DbgContCommand},
   {"echo", "echo", "Toggle serial echo", DbgEchoToggle},
   {"condition", "condition [all|umode|kmode]", "Kdbg enter
condition",
DbgStopCondition},
-   
+  {"module-loaded", "module-loaded [break|continue]",
"Module-loaded
action", DbgModuleLoadedAction},
+
   {"regs", "regs", "Display general purpose registers",
DbgRegsCommand},
   {"dregs", "dregs", "Display debug registers",
DbgDRegsCommand},
   {"cregs", "cregs", "Display control registers",
DbgCRegsCommand},
@@ -909,9 +913,8 @@
 {
   PVOID Address;
 
-  DbgPrint("Frames:  ");
-  while (Frame != NULL && (ULONG_PTR)Frame >= StackLimit && 
-	 (ULONG_PTR)Frame < StackBase) /* FIXME: why limit this to
StackBase/StackLimit? */
+  DbgPrint("Frames:\n");
+  while (Frame != NULL)
     {
       if (!NT_SUCCESS(KdbpSafeReadMemory(&Address, Frame + 1, sizeof
(Address))))
         {
@@ -1353,6 +1356,32 @@
 }
 
 ULONG
+DbgModuleLoadedAction(ULONG Argc, PCH Argv[], PKTRAP_FRAME Tf)
+{
+    if (Argc == 1)
+      {
+	if (KdbBreakOnModuleLoad)
+          DbgPrint("Current setting: break\n");
+	else
+          DbgPrint("Current setting: continue\n");
+      }
+    else if (!strcmp(Argv[1], "break"))
+      {
+        KdbBreakOnModuleLoad = TRUE;
+      }
+    else if (!strcmp(Argv[1], "continue"))
+      {
+        KdbBreakOnModuleLoad = FALSE;
+      }
+    else
+      {
+        DbgPrint("Unknown setting: %s\n", Argv[1]);
+      }
+
+    return(TRUE);
+}
+
+ULONG
 DbgEchoToggle(ULONG Argc, PCH Argv[], PKTRAP_FRAME Tf)
 {
   KbdEchoOn = !KbdEchoOn;
@@ -1656,23 +1685,24 @@
   LONG BreakPointNr;
   ULONG ExpNr = (ULONG)TrapFrame->DebugArgMark;
 
+  /* Always handle beakpoints */
   if (ExpNr != 1 && ExpNr != 3)
     {
       DbgPrint(":KDBG:Entered:%s:%s\n",
                PreviousMode==KernelMode ? "kmode" : "umode",
                AlwaysHandle ? "always" : "if-unhandled");
-    }
-  
-  /* If we aren't handling umode exceptions then return */
-  if (PreviousMode == UserMode && !KdbHandleUmode && !AlwaysHandle)
-    {
-      return kdContinue;
-    }
 
-  /* If the exception would be unhandled (and we care) then handle it
*/
-  if (PreviousMode == KernelMode && !KdbHandleHandled && !AlwaysHandle)
-    {
-      return kdContinue;
+      /* If we aren't handling umode exceptions then return */
+      if (PreviousMode == UserMode && !KdbHandleUmode && !AlwaysHandle)
+        {
+          return kdHandleException;
+        }
+
+      /* If the exception would be unhandled (and we care) then handle
it */
+      if (PreviousMode == KernelMode && !KdbHandleHandled &&
!AlwaysHandle)
+        {
+          return kdHandleException;
+        }
     }
 
   /* Exception inside the debugger? Game over. */
@@ -1772,3 +1802,14 @@
       return(kdContinue);
     }
 }
+
+VOID
+KdbModuleLoaded(IN PUNICODE_STRING Name)
+{
+  if (!KdbBreakOnModuleLoad)
+    return;
+    
+  DbgPrint("Module %wZ loaded.\n", Name);
+  DbgBreakPointWithStatus(DBG_STATUS_CONTROL_C);
+}
+
  _____  

Modified: trunk/reactos/ntoskrnl/dbg/kdb.h
--- trunk/reactos/ntoskrnl/dbg/kdb.h	2005-01-12 16:09:12 UTC (rev
12972)
+++ trunk/reactos/ntoskrnl/dbg/kdb.h	2005-01-12 19:04:06 UTC (rev
12973)
@@ -72,6 +72,10 @@

 VOID
 KdbProfileInterrupt(ULONG_PTR Eip);
 
+VOID
+KdbModuleLoaded(IN PUNICODE_STRING Name);
+
+
 struct KDB_BPINFO {
     DWORD Addr;
     DWORD Type;
  _____  

Modified: trunk/reactos/ntoskrnl/dbg/kdb_symbols.c
--- trunk/reactos/ntoskrnl/dbg/kdb_symbols.c	2005-01-12 16:09:12 UTC
(rev 12972)
+++ trunk/reactos/ntoskrnl/dbg/kdb_symbols.c	2005-01-12 19:04:06 UTC
(rev 12973)
@@ -816,6 +816,9 @@

   PSYMBOLFILE_HEADER SymbolFileHeader;
   PIMAGE_SYMBOL_INFO_CACHE CachedSymbolFile;
 
+  /* Allow KDB to break on module load */
+  KdbModuleLoaded(FileName);
+
   /*  Get the path to the symbol store  */
   wcscpy(TmpFileName, L"\\SystemRoot\\symbols\\");
 
  _____  

Modified: trunk/reactos/ntoskrnl/ke/catch.c
--- trunk/reactos/ntoskrnl/ke/catch.c	2005-01-12 16:09:12 UTC (rev
12972)
+++ trunk/reactos/ntoskrnl/ke/catch.c	2005-01-12 19:04:06 UTC (rev
12973)
@@ -50,6 +50,7 @@

 
   DPRINT("KiDispatchException() called\n");
 
+
   /* PCR->KeExceptionDispatchCount++; */
 
   if (Context == NULL)
@@ -95,8 +96,12 @@
 	      NTSTATUS StatusOfCopy;
 
 #ifdef KDBG
-	      KdbEnterDebuggerException (ExceptionRecord, PreviousMode, 
-					 Context, Tf, FALSE);
+	      Action = KdbEnterDebuggerException (ExceptionRecord,
PreviousMode,
+	                                          Context, Tf, FALSE);
+              if (Action == kdContinue)
+                {
+                  return;
+                }
 #endif
 
 	      /* FIXME: Forward exception to user mode debugger */
@@ -141,8 +146,12 @@
 	  /* FIXME: Forward the exception to the process exception port
*/
 
 #ifdef KDBG
-	  KdbEnterDebuggerException (ExceptionRecord, PreviousMode, 
-				     Context, Tf, TRUE);
+	  Action = KdbEnterDebuggerException (ExceptionRecord,
PreviousMode,
+	                                      Context, Tf, TRUE);
+          if (Action == kdContinue)
+            {
+              return;
+            }
 #endif
 
 	  /* Terminate the offending thread */
@@ -153,8 +162,12 @@
 	{
 	  /* PreviousMode == KernelMode */
 #ifdef KDBG
-	  KdbEnterDebuggerException (ExceptionRecord, PreviousMode, 
-				     Context, Tf, FALSE);
+	  Action = KdbEnterDebuggerException (ExceptionRecord,
PreviousMode,
+	                                      Context, Tf, FALSE);
+          if (Action == kdContinue)
+            {
+              return;
+            }
 #endif
 
 	  Value = RtlpDispatchException (ExceptionRecord, Context);
  _____  

Modified: trunk/reactos/ntoskrnl/ke/i386/exp.c
--- trunk/reactos/ntoskrnl/ke/i386/exp.c	2005-01-12 16:09:12 UTC
(rev 12972)
+++ trunk/reactos/ntoskrnl/ke/i386/exp.c	2005-01-12 19:04:06 UTC
(rev 12973)
@@ -190,9 +190,9 @@

       Er.NumberParameters = 0;
     }
 
-  Er.ExceptionFlags = ((NTSTATUS) STATUS_SINGLE_STEP == (NTSTATUS)
Er.ExceptionCode
-    || (NTSTATUS) STATUS_BREAKPOINT == (NTSTATUS) Er.ExceptionCode) ?
-    EXCEPTION_NONCONTINUABLE : 0;
+  Er.ExceptionFlags = (STATUS_SINGLE_STEP == Er.ExceptionCode ||
+                       STATUS_BREAKPOINT  == Er.ExceptionCode) ?
+                       0 : EXCEPTION_NONCONTINUABLE;
 
   KiDispatchException(&Er, 0, Tf, KernelMode, TRUE);
 
  _____  

Modified: trunk/reactos/ntoskrnl/ke/i386/usertrap.c
--- trunk/reactos/ntoskrnl/ke/i386/usertrap.c	2005-01-12 16:09:12 UTC
(rev 12972)
+++ trunk/reactos/ntoskrnl/ke/i386/usertrap.c	2005-01-12 19:04:06 UTC
(rev 12973)
@@ -132,9 +132,9 @@

     }
   
 
-  Er.ExceptionFlags = ((NTSTATUS) STATUS_SINGLE_STEP == (NTSTATUS)
Er.ExceptionCode ||
-    (NTSTATUS) STATUS_BREAKPOINT == (NTSTATUS) Er.ExceptionCode) ?
-    EXCEPTION_NONCONTINUABLE : 0;
+  Er.ExceptionFlags = (STATUS_SINGLE_STEP == Er.ExceptionCode ||
+                       STATUS_BREAKPOINT  == Er.ExceptionCode) ?
+                       0 : EXCEPTION_NONCONTINUABLE;
 
   KiDispatchException(&Er, 0, Tf, UserMode, TRUE);
   return(0);
  _____  

Modified: trunk/reactos/ntoskrnl/mm/virtual.c
--- trunk/reactos/ntoskrnl/mm/virtual.c	2005-01-12 16:09:12 UTC (rev
12972)
+++ trunk/reactos/ntoskrnl/mm/virtual.c	2005-01-12 19:04:06 UTC (rev
12973)
@@ -350,6 +350,56 @@

 }
 
 
+NTSTATUS STDCALL
+MiProtectVirtualMemory(IN PEPROCESS Process,
+                       IN OUT PVOID *BaseAddress,
+                       IN OUT PULONG NumberOfBytesToProtect,
+                       IN ULONG NewAccessProtection,
+                       OUT PULONG OldAccessProtection  OPTIONAL)
+{
+   PMEMORY_AREA MemoryArea;
+   PMADDRESS_SPACE AddressSpace;
+   ULONG OldAccessProtection_;
+   NTSTATUS Status;
+
+   *NumberOfBytesToProtect =
+      PAGE_ROUND_UP((*BaseAddress) + (*NumberOfBytesToProtect)) -
+      PAGE_ROUND_DOWN(*BaseAddress);
+   *BaseAddress = (PVOID)PAGE_ROUND_DOWN(*BaseAddress);
+
+   AddressSpace = &Process->AddressSpace;
+
+   MmLockAddressSpace(AddressSpace);
+   MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace,
*BaseAddress);
+   if (MemoryArea == NULL)
+   {
+      MmUnlockAddressSpace(AddressSpace);
+      return STATUS_UNSUCCESSFUL;
+   }
+
+   if (OldAccessProtection == NULL)
+      OldAccessProtection = &OldAccessProtection_;
+
+   if (MemoryArea->Type == MEMORY_AREA_VIRTUAL_MEMORY)
+   {
+      Status = MmProtectAnonMem(AddressSpace, MemoryArea, *BaseAddress,
+                                *NumberOfBytesToProtect,
NewAccessProtection,
+                                OldAccessProtection);
+   }
+   else if (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW)
+   {
+      Status = MmProtectSectionView(AddressSpace, MemoryArea,
*BaseAddress,
+                                    *NumberOfBytesToProtect,
+                                    NewAccessProtection,
+                                    OldAccessProtection);
+   }
+
+   MmUnlockAddressSpace(AddressSpace);
+
+   return Status;
+}
+
+
 /* (tMk 2004.II.5)
  * FUNCTION:
  * Called from VirtualProtectEx (lib\kernel32\mem\virtual.c)
@@ -357,15 +407,13 @@
  */
 NTSTATUS STDCALL
 NtProtectVirtualMemory(IN HANDLE ProcessHandle,
-                       IN PVOID *UnsafeBaseAddress,
-                       IN ULONG *UnsafeNumberOfBytesToProtect,
+                       IN OUT PVOID *UnsafeBaseAddress,
+                       IN OUT ULONG *UnsafeNumberOfBytesToProtect,
                        IN ULONG NewAccessProtection,
                        OUT PULONG UnsafeOldAccessProtection)
 {
-   PMEMORY_AREA MemoryArea;
    PEPROCESS Process;
    NTSTATUS Status;
-   PMADDRESS_SPACE AddressSpace;
    ULONG OldAccessProtection;
    PVOID BaseAddress;
    ULONG NumberOfBytesToProtect;
@@ -377,18 +425,14 @@
    if (!NT_SUCCESS(Status))
       return Status;
 
-   // (tMk 2004.II.5) in Microsoft SDK I read:
-   // 'if this parameter is NULL or does not point to a valid variable,
the function fails'
+   /* (tMk 2004.II.5) in Microsoft SDK I read:
+    * 'if this parameter is NULL or does not point to a valid variable,
the function fails'
+    */
    if(UnsafeOldAccessProtection == NULL)
    {
       return(STATUS_INVALID_PARAMETER);
    }
 
-   NumberOfBytesToProtect =
-      PAGE_ROUND_UP(BaseAddress + NumberOfBytesToProtect) -
-      PAGE_ROUND_DOWN(BaseAddress);
-   BaseAddress = (PVOID)PAGE_ROUND_DOWN(BaseAddress);
-
    Status = ObReferenceObjectByHandle(ProcessHandle,
                                       PROCESS_VM_OPERATION,
                                       PsProcessType,
@@ -401,32 +445,12 @@
       return(Status);
    }
 
-   AddressSpace = &Process->AddressSpace;
+   Status = MiProtectVirtualMemory(Process,
+                                   &BaseAddress,
+                                   &NumberOfBytesToProtect,
+                                   NewAccessProtection,
+                                   &OldAccessProtection);
 
-   MmLockAddressSpace(AddressSpace);
-   MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace, BaseAddress);
-   if (MemoryArea == NULL)
-   {
-      MmUnlockAddressSpace(AddressSpace);
-      ObDereferenceObject(Process);
-      return(STATUS_UNSUCCESSFUL);
-   }
-
-   if (MemoryArea->Type == MEMORY_AREA_VIRTUAL_MEMORY)
-   {
-      Status = MmProtectAnonMem(AddressSpace, MemoryArea, BaseAddress,
-                                NumberOfBytesToProtect,
NewAccessProtection,
-                                &OldAccessProtection);
-   }
-   else if (MemoryArea->Type == MEMORY_AREA_SECTION_VIEW)
-   {
-      Status = MmProtectSectionView(AddressSpace, MemoryArea,
BaseAddress,
-                                    NumberOfBytesToProtect,
-                                    NewAccessProtection,
-                                    &OldAccessProtection);
-   }
-
-   MmUnlockAddressSpace(AddressSpace);
    ObDereferenceObject(Process);
 
    MmCopyToCaller(UnsafeOldAccessProtection, &OldAccessProtection,
sizeof(ULONG));
@@ -590,12 +614,15 @@
                      IN PVOID BaseAddress,
                      IN PVOID Buffer,
                      IN ULONG NumberOfBytesToWrite,
-                     OUT PULONG NumberOfBytesWritten)
+                     OUT PULONG NumberOfBytesWritten  OPTIONAL)
 {
    NTSTATUS Status;
    PMDL Mdl;
    PVOID SystemAddress;
    PEPROCESS Process;
+   ULONG OldProtection = 0;
+   PVOID ProtectBaseAddress;
+   ULONG ProtectNumberOfBytes;
 
    DPRINT("NtWriteVirtualMemory(ProcessHandle %x, BaseAddress %x, "
           "Buffer %x, NumberOfBytesToWrite
%d)\n",ProcessHandle,BaseAddress,
@@ -612,23 +639,62 @@
       return(Status);
    }
 
+   /* We have to make sure the target memory is writable.
+    *
+    * I am not sure if it is correct to do this in any case, but it has
to be
+    * done at least in some cases because you can use
WriteProcessMemory to
+    * write into the .text section of a module where memcpy() would
crash.
+    *  -blight (2005/01/09)
+    */
+   ProtectBaseAddress = BaseAddress;
+   ProtectNumberOfBytes = NumberOfBytesToWrite;
+
+   /* Write memory */
    if (Process == PsGetCurrentProcess())
    {
+      Status = MiProtectVirtualMemory(Process,
+                                      &ProtectBaseAddress,
+                                      &ProtectNumberOfBytes,
+                                      PAGE_READWRITE,
+                                      &OldProtection);
+      if (!NT_SUCCESS(Status))
+      {
+         ObDereferenceObject(Process);
+         return Status;
+      }
       memcpy(BaseAddress, Buffer, NumberOfBytesToWrite);
    }
    else
    {
+      /* Create MDL describing the source buffer. */
       Mdl = MmCreateMdl(NULL,
                         Buffer,
                         NumberOfBytesToWrite);
-      MmProbeAndLockPages(Mdl,
-                          UserMode,
-                          IoReadAccess);
       if(Mdl == NULL)
       {
          ObDereferenceObject(Process);
          return(STATUS_NO_MEMORY);
       }
+
+      /* Make the target area writable. */
+      Status = MiProtectVirtualMemory(Process,
+                                      &ProtectBaseAddress,
+                                      &ProtectNumberOfBytes,
+                                      PAGE_READWRITE,
+                                      &OldProtection);
+      if (!NT_SUCCESS(Status))
+      {
+         ObDereferenceObject(Process);
+         ExFreePool(Mdl);
+         return Status;
+      }
+
+      /* Map the MDL. */
+      MmProbeAndLockPages(Mdl,
+                          UserMode,
+                          IoReadAccess);
+
+      /* Copy memory from the mapped MDL into the target buffer. */
       KeAttachProcess(&Process->Pcb);
 
       SystemAddress = MmGetSystemAddressForMdl(Mdl);
@@ -636,6 +702,7 @@
 
       KeDetachProcess();
 
+      /* Free the MDL. */
       if (Mdl->MappedSystemVa != NULL)
       {
          MmUnmapLockedPages(Mdl->MappedSystemVa, Mdl);
@@ -644,9 +711,22 @@
       ExFreePool(Mdl);
    }
 
+   /* Reset the protection of the target memory. */
+   Status = MiProtectVirtualMemory(Process,
+                                   &ProtectBaseAddress,
+                                   &ProtectNumberOfBytes,
+                                   OldProtection,
+                                   &OldProtection);
+   if (!NT_SUCCESS(Status))
+   {
+      DPRINT1("Failed to reset protection of the target memory! (Status
0x%x)\n", Status);
+      /* FIXME: Should we bugcheck here? */
+   }
+
    ObDereferenceObject(Process);
 
-   *NumberOfBytesWritten = NumberOfBytesToWrite;
+   if (NumberOfBytesWritten != NULL)
+      MmCopyToCaller(NumberOfBytesWritten, &NumberOfBytesToWrite,
sizeof(ULONG));
 
    return(STATUS_SUCCESS);
 }