[ros-diffs] [arty] 28451: Bug found by arty, thanks to alex for helping fix it: Process termination fixes part 1: don't suicide our process when we're killing a sibling. - Clarify the use of KillByHandle. It does *not* mean "suicide". - In fact, we had considered the use of KillByHandle backward, that is, our process would commit suicide if it was killing a sibling process. - Make suicide contingent on killing the same process. - Properly handle DBG_TERMINATE_PROCESS semidocumented debugger hack.

[arty at svn.reactos.org]

Author: arty
Date: Wed Aug 22 11:28:45 2007
New Revision: 28451

URL: http://svn.reactos.org/svn/reactos?rev=28451&view=rev
Log:
Bug found by arty, thanks to alex for helping fix it:
Process termination fixes part 1: don't suicide our process when we're killing
a sibling.

 - Clarify the use of KillByHandle.  It does *not* mean "suicide".
 - In fact, we had considered the use of KillByHandle backward, that is,
   our process would commit suicide if it was killing a sibling process.
 - Make suicide contingent on killing the same process.
 - Properly handle DBG_TERMINATE_PROCESS semidocumented debugger hack.

Modified:
    trunk/reactos/ntoskrnl/include/internal/ps.h
    trunk/reactos/ntoskrnl/ps/kill.c

Modified: trunk/reactos/ntoskrnl/include/internal/ps.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/include/internal/ps.h?rev=28451&r1=28450&r2=28451&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/include/internal/ps.h (original)
+++ trunk/reactos/ntoskrnl/include/internal/ps.h Wed Aug 22 11:28:45 2007
@@ -57,6 +57,10 @@
 #define PSTRACE(x, ...) DPRINT(__VA_ARGS__);
 #define PSREFTRACE(x)
 #endif
+
+#define PspSetProcessFlag(Process, Flag) \
+ InterlockedOr((PLONG)&Process->Flags, Flag)
+
 
 //
 // Maximum Count of Notification Routines

Modified: trunk/reactos/ntoskrnl/ps/kill.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/kill.c?rev=28451&r1=28450&r2=28451&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/ps/kill.c (original)
+++ trunk/reactos/ntoskrnl/ps/kill.c Wed Aug 22 11:28:45 2007
@@ -1108,12 +1108,21 @@
     PSTRACE(PS_KILL_DEBUG,
             "ProcessHandle: %p ExitStatus: %p\n", ProcessHandle, ExitStatus);
 
-    /* Remember how we will kill it */
-    KillByHandle = (ProcessHandle != NULL);
+    /* Were we passed a process handle? */
+    if (ProcessHandle)
+    {
+        /* Yes we were, use it */
+        KillByHandle = TRUE;
+    }
+    else
+    {
+        /* We weren't... we assume this is suicide */
+        KillByHandle = FALSE;
+        ProcessHandle = NtCurrentProcess();
+    }
 
     /* Get the Process Object */
-    Status = ObReferenceObjectByHandle((KillByHandle) ?
-                                       ProcessHandle : NtCurrentProcess(),
+    Status = ObReferenceObjectByHandle(ProcessHandle,
                                        PROCESS_TERMINATE,
                                        PsProcessType,
                                        KeGetPreviousMode(),
@@ -1138,9 +1147,8 @@
         return STATUS_PROCESS_IS_TERMINATING;
     }
 
-    /* Set the delete flag */
-    if (!KillByHandle) InterlockedOr((PLONG)&Process->Flags,
-                                     PSF_PROCESS_DELETE_BIT);
+    /* Set the delete flag, unless the process is comitting suicide */
+    if (KillByHandle) PspSetProcessFlag(Process, PSF_PROCESS_DELETE_BIT);
 
     /* Get the first thread */
     Status = STATUS_NOTHING_TO_TERMINATE;
@@ -1169,23 +1177,22 @@
     ExReleaseRundownProtection(&Process->RundownProtect);
 
     /* Check if we are killing ourselves */
-    if (Process != CurrentProcess)
-    {
-        /* Check for the DBG_TERMINATE_PROCESS exit code */
-        if (ExitStatus == DBG_TERMINATE_PROCESS)
-        {
-            /* Disable debugging on this process */
-            DbgkClearProcessDebugObject(Process, NULL);
-        }
-    }
-    /* Make sure that we got a handle */
-    else if (KillByHandle)
-    {
-        /* Dereference the project */
-        ObDereferenceObject(Process);
-
-        /* Terminate ourselves */
-        PspTerminateThreadByPointer(CurrentThread, ExitStatus, TRUE);
+    if (Process == CurrentProcess)
+    {
+        /* Also make sure the caller gave us our handle */
+        if (KillByHandle)
+        {
+            /* Dereference the project */
+            ObDereferenceObject(Process);
+
+            /* Terminate ourselves */
+            PspTerminateThreadByPointer(CurrentThread, ExitStatus, TRUE);
+        }
+    }
+    else if (ExitStatus == DBG_TERMINATE_PROCESS)
+    {
+        /* Disable debugging on this process */
+        DbgkClearProcessDebugObject(Process, NULL);
     }
 
     /* Check if there was nothing to terminate, or if we have a Debug Port */