[ros-diffs] [fireball] 33870: - Fix a major bug while closing a process that was debugging another process, which would cause a bugcheck because it tried to kill itself while it was being killed -- instead, it should try killing the debuggee! fixes the bug that arty and I fought about when he said "process killing seems broken" and tried to add delayed-object-closing. Patch by Alex Ionescu. - Remove a leftover from kernel32 which assumed that if "BeingDebugged" was TRUE, the library was being tested on an XP machine.

fireball at svn.reactos.org fireball at svn.reactos.org
Fri Jun 6 23:43:38 CEST 2008


Author: fireball
Date: Fri Jun  6 16:43:37 2008
New Revision: 33870

URL: http://svn.reactos.org/svn/reactos?rev=33870&view=rev
Log:
- Fix a major bug while closing a process that was debugging another process, which would cause a bugcheck because it tried to kill itself while it was being killed -- instead, it should try killing the debuggee! fixes the bug that arty and I fought about when he said "process killing seems broken" and tried to add delayed-object-closing. Patch by Alex Ionescu.
- Remove a leftover from kernel32 which assumed that if "BeingDebugged" was TRUE, the library was being tested on an XP machine.

Modified:
    trunk/reactos/dll/win32/kernel32/misc/dllmain.c
    trunk/reactos/ntoskrnl/dbgk/dbgkobj.c

Modified: trunk/reactos/dll/win32/kernel32/misc/dllmain.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/misc/dllmain.c?rev=33870&r1=33869&r2=33870&view=diff
==============================================================================
--- trunk/reactos/dll/win32/kernel32/misc/dllmain.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/misc/dllmain.c [iso-8859-1] Fri Jun  6 16:43:37 2008
@@ -257,42 +257,6 @@
     {
         case DLL_PROCESS_ATTACH:
 
-#ifdef _M_IX86
-        /* OK, yes, this is really retarded but it works for now */
-        InWindows = NtCurrentPeb()->BeingDebugged;
-
-        /*
-         * CreateProcess will run in the real kernel32 and it will write
-         * its own BaseProcessStartThunk EIP in the CONTEXT that ZwContinue
-         * will get. We'll be first called by Ldr while initializing, and we'll
-         * be wrapped in 3 layers of SEH, followed by two frames, finally 
-         * followed by our CONTEXT on the stack. We'll modify the EIP in it
-         * to match the correct one (our own) and then everything works.
-         * Tested on XP and 2K3, probably doesn't work in 2K.
-         */
-        if (InWindows)
-        {
-            /* 
-             * Due to yet another bug in how Windows handles .local, LDR will
-             * actually end up loading us twice. The second time will be the
-             * "official" load, at a totally different address. It will be,
-             * it will be at -that- address that all the APIs will be called.
-             * However, that address is dynamic while this one will be static,
-             * so we'll do initilization with this one. Plus, at this one,
-             * we know exactly that we're within 3 SEH layers.
-             */
-            if (hDll == (HANDLE)0x7c800000)
-            {
-                PULONG Eip;
-                __debugbreak();
-                Eip = (PULONG)*(PULONG)*(PULONG)NtCurrentTeb()->Tib.ExceptionList +
-                    0x9 +
-                    FIELD_OFFSET(CONTEXT, Eip) / sizeof(ULONG);
-                *Eip = (ULONG)BaseProcessStartThunk;
-            }
-        }
-#endif
-
         /* Don't bother us for each thread */
         LdrDisableThreadCalloutsForDll((PVOID)hDll);
 

Modified: trunk/reactos/ntoskrnl/dbgk/dbgkobj.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/dbgk/dbgkobj.c?rev=33870&r1=33869&r2=33870&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/dbgk/dbgkobj.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/dbgk/dbgkobj.c [iso-8859-1] Fri Jun  6 16:43:37 2008
@@ -115,7 +115,7 @@
     KeInitializeEvent(&DebugEvent->ContinueEvent, SynchronizationEvent, FALSE);
     DebugEvent->Process = Process;
     DebugEvent->Thread = Thread;
-    RtlCopyMemory(&DebugEvent->ApiMsg, Message, sizeof(DBGKM_MSG));
+    DebugEvent->ApiMsg = *Message;
     DebugEvent->ClientId = Thread->Cid;
 
     /* Check if we have a port object */
@@ -176,7 +176,7 @@
                                   NULL);
 
             /* Copy API Message back */
-            RtlCopyMemory(Message, &DebugEvent->ApiMsg, sizeof(DBGKM_MSG));
+            *Message = DebugEvent->ApiMsg;
 
             /* Set return status */
             Status = DebugEvent->Status;
@@ -1079,8 +1079,6 @@
     DBGKTRACE(DBGK_OBJECT_DEBUG, "OwnerProcess: %p DebugObject: %p\n",
               OwnerProcess, DebugObject);
 
-    DPRINT("APC DISABLE: %d\n", ((PETHREAD)KeGetCurrentThread())->Tcb.CombinedApcDisable);
-
     /* If this isn't the last handle, do nothing */
     if (SystemHandleCount > 1) return;
 
@@ -1124,13 +1122,13 @@
             if (DebugPortCleared)
             {
                 /* Mark this in the PEB */
-                DbgkpMarkProcessPeb(OwnerProcess);
+                DbgkpMarkProcessPeb(Process);
 
                 /* Check if we terminate on exit */
                 if (DebugObject->KillProcessOnExit)
                 {
                     /* Terminate the process */
-                    PsTerminateProcess(OwnerProcess, STATUS_DEBUGGER_INACTIVE);
+                    PsTerminateProcess(Process, STATUS_DEBUGGER_INACTIVE);
                 }
 
                 /* Dereference the debug object */
@@ -1192,6 +1190,7 @@
     if (NT_SUCCESS(Status))
     {
         /* Acquire the global lock */
+ThreadScan:
         GlobalHeld = TRUE;
         ExAcquireFastMutex(&DbgkpProcessDebugPortMutex);
 
@@ -1203,7 +1202,6 @@
         }
         else
         {
-ThreadScan:
             /* Otherwise, set the port and reference the thread */
             Process->DebugPort = DebugObject;
             ObReferenceObject(LastThread);
@@ -1235,14 +1233,7 @@
                 {
                     /* Dereference the first thread and re-acquire the lock */
                     ObDereferenceObject(FirstThread);
-                    GlobalHeld = TRUE;
-                    ExAcquireFastMutex(&DbgkpProcessDebugPortMutex);
-
-                    /* Check if we should loop again */
-                    if (!Process->DebugPort) goto ThreadScan;
-
-                    /* Otherwise, we already have a port */
-                    Status = STATUS_PORT_ALREADY_SET;
+                    goto ThreadScan;
                 }
             }
         }
@@ -1265,8 +1256,7 @@
         {
             /* Set the process flags */
             InterlockedOr((PLONG)&Process->Flags,
-                          PSF_NO_DEBUG_INHERIT_BIT |
-                          PSF_CREATE_REPORTED_BIT);
+                          PSF_NO_DEBUG_INHERIT_BIT | PSF_CREATE_REPORTED_BIT);
 
             /* Reference the debug object */
             ObDereferenceObject(DebugObject);
@@ -2012,7 +2002,7 @@
                 StartTime = NewTime;
 
                 /* Check if we've timed out */
-                if (SafeTimeOut.QuadPart > 0)
+                if (SafeTimeOut.QuadPart >= 0)
                 {
                     /* We have, break out of the loop */
                     Status = STATUS_TIMEOUT;
@@ -2037,9 +2027,7 @@
     _SEH_TRY
     {
         /* Return our wait state change structure */
-        RtlCopyMemory(StateChange,
-                      &WaitStateChange,
-                      sizeof(DBGUI_WAIT_STATE_CHANGE));
+        *StateChange = WaitStateChange;
     }
     _SEH_EXCEPT(_SEH_ExSystemExceptionFilter)
     {



More information about the Ros-diffs mailing list