[ros-diffs] [ion] 23193: - Fix Win32K thread rundown bug, fixes shutdown crash and other bugchecks where some ASSERT(Class->windows) wasn't 0. - Close the right debug handle instead of some random value. - Some generic formatting cleanup.

Author: ion
Date: Thu Jul 20 19:46:10 2006
New Revision: 23193

URL: http://svn.reactos.org/svn/reactos?rev=23193&view=rev
Log:
- Fix Win32K thread rundown bug, fixes shutdown crash and other bugchecks where some
ASSERT(Class->windows) wasn't 0.
- Close the right debug handle instead of some random value.
- Some generic formatting cleanup.

Modified:
    trunk/reactos/include/ndk/obfuncs.h
    trunk/reactos/ntoskrnl/KrnlFun.c
    trunk/reactos/ntoskrnl/ps/kill.c

Modified: trunk/reactos/include/ndk/obfuncs.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/obfuncs.h?rev=…
==============================================================================
--- trunk/reactos/include/ndk/obfuncs.h (original)
+++ trunk/reactos/include/ndk/obfuncs.h Thu Jul 20 19:46:10 2006
@@ -29,6 +29,14 @@
 //
 // Object Functions
 //
+NTKERNELAPI
+NTSTATUS
+NTAPI
+ObCloseHandle(
+    IN HANDLE Handle,
+    IN KPROCESSOR_MODE AccessMode
+);
+
 NTKERNELAPI
 NTSTATUS
 NTAPI

Modified: trunk/reactos/ntoskrnl/KrnlFun.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/KrnlFun.c?rev=231…
==============================================================================
--- trunk/reactos/ntoskrnl/KrnlFun.c (original)
+++ trunk/reactos/ntoskrnl/KrnlFun.c Thu Jul 20 19:46:10 2006
@@ -25,16 +25,15 @@
 //  - Add tracing to iofunc.c
 //
 // Ps:
-//  - Use Process Rundown.
+//  - Use Process/Thread Rundown.
 //  - Use Process Pushlock Locks.
-//  - Use Safe Referencing in PsGetNextProcess/Thread.
+//  - Use Safe Referencing where needed.
 //  - Use Guarded Mutex instead of Fast Mutex for Active Process Locks.
 //  - Use Security Locks in security.c
-//  - Fix referencing problem.
+//  - Figure out why processes don't die.
 //  - Generate process cookie for user-more thread.
 //  - Add security calls where necessary.
 //  - Add tracing.
-//  - Fix crash on shutdown due to possibly incorrect win32k uninitailization.
 //  - Add failure/race checks for thread creation.
 //
 // Ob:

Modified: trunk/reactos/ntoskrnl/ps/kill.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ps/kill.c?rev=231…
==============================================================================
--- trunk/reactos/ntoskrnl/ps/kill.c (original)
+++ trunk/reactos/ntoskrnl/ps/kill.c Thu Jul 20 19:46:10 2006
@@ -14,11 +14,6 @@
 #define NDEBUG
 #include <internal/debug.h>
 
-#define LockEvent Spare0[0]
-#define LockCount Spare0[1]
-#define LockOwner Spare0[2]
-
-
 /* GLOBALS *******************************************************************/
 
 LIST_ENTRY PspReaperListHead = {0};
@@ -31,7 +26,7 @@
 extern PEPROCESS PsInitialSystemProcess;
 extern PEPROCESS PsIdleProcess;
 
-/* FUNCTIONS *****************************************************************/
+/* PRIVATE FUNCTIONS *********************************************************/
 
 NTSTATUS
 NTAPI
@@ -53,14 +48,16 @@
     InterlockedOr((PLONG)&Process->Flags, 8);
 
     /* Get the first thread */
-    while ((Thread = PsGetNextProcessThread(Process, Thread)))
+    Thread = PsGetNextProcessThread(Process, Thread);
+    while (Thread)
     {
         /* Kill it */
         PspTerminateThreadByPointer(Thread, ExitStatus, FALSE);
-        }
+        Thread = PsGetNextProcessThread(Process, Thread);
+    }
 
     /* Clear the handle table */
-    ObClearProcessHandleTable(Process);
+    if (Process->ObjectTable) ObClearProcessHandleTable(Process);
 
     /* Return success*/
     return STATUS_SUCCESS;
@@ -73,7 +70,8 @@
     PEPROCESS Process = NULL;
 
     /* Loop every process */
-    while ((Process == PsGetNextProcess(Process)))
+    Process == PsGetNextProcess(Process);
+    while (Process)
     {
         /* Make sure this isn't the idle or initial process */
         if ((Process != PsInitialSystemProcess) && (Process != PsIdleProcess))
@@ -81,6 +79,9 @@
             /* Kill it */
             PspTerminateProcess(Process, STATUS_SYSTEM_SHUTDOWN);
         }
+
+        /* Get the next process */
+        Process == PsGetNextProcess(Process);
     }
 }
 
@@ -140,17 +141,15 @@
 {
     PEPROCESS Process = (PEPROCESS)ObjectBody;
     KAPC_STATE ApcState;
-    DPRINT1("PspDeleteProcess(ObjectBody %x)\n", ObjectBody);
     PAGED_CODE();
-    DPRINT1("Name: %.16s\n", &Process->ImageFileName);
 
     /* Check if it has an Active Process Link */
     if (Process->ActiveProcessLinks.Flink)
     {
-    /* Remove it from the Active List */
-    ExAcquireFastMutex(&PspActiveProcessMutex);
-    RemoveEntryList(&Process->ActiveProcessLinks);
-    ExReleaseFastMutex(&PspActiveProcessMutex);
+        /* Remove it from the Active List */
+        ExAcquireFastMutex(&PspActiveProcessMutex);
+        RemoveEntryList(&Process->ActiveProcessLinks);
+        ExReleaseFastMutex(&PspActiveProcessMutex);
     }
 
     /* Check for Auditing information */
@@ -233,7 +232,7 @@
 
         /* Completely delete the Address Space */
         MmDeleteProcessAddressSpace(Process);
-}
+    }
 
     /* See if we have a PID */
     if(Process->UniqueProcessId)
@@ -272,9 +271,8 @@
 {
     PETHREAD Thread = (PETHREAD)ObjectBody;
     PEPROCESS Process = Thread->ThreadsProcess;
-    DPRINT1("PspDeleteThread(Thread %p, Process %p)\n", ObjectBody, Process);
     PAGED_CODE();
-    //ASSERT(Thread->Tcb.Win32Thread == NULL); FIXME
+    ASSERT(Thread->Tcb.Win32Thread == NULL);
 
     /* Check if we have a stack */
     if (Thread->Tcb.InitialStack)
@@ -284,9 +282,10 @@
                             Thread->Tcb.LargeStack);
     }
 
-    /* Delete the CID Handle */
-    if(Thread->Cid.UniqueThread)
-    {
+    /* Check if we have a CID Handle */
+    if (Thread->Cid.UniqueThread)
+    {
+        /* Delete the CID Handle */
         if (!(ExDestroyHandle(PspCidTable, Thread->Cid.UniqueThread)))
         {
             /* Something wrong happened, bugcheck */
@@ -338,7 +337,6 @@
     PKAPC Apc;
     PTOKEN PrimaryToken;
     PAGED_CODE();
-    DPRINT1("PspExitThread(%p, %lx)\n", PsGetCurrentThread(), ExitStatus);
 
     /* Get the Current Thread and Process */
     Thread = PsGetCurrentThread();
@@ -348,11 +346,12 @@
     /* Can't terminate a thread if it attached another process */
     if (KeIsAttachedProcess())
     {
+        /* Bugcheck */
         KEBUGCHECKEX(INVALID_PROCESS_ATTACH_ATTEMPT,
-                     (ULONG)CurrentProcess,
-                     (ULONG)Thread->Tcb.ApcState.Process,
-                     (ULONG)Thread->Tcb.ApcStateIndex,
-                     (ULONG)Thread);
+                     (ULONG_PTR)CurrentProcess,
+                     (ULONG_PTR)Thread->Tcb.ApcState.Process,
+                     (ULONG_PTR)Thread->Tcb.ApcStateIndex,
+                     (ULONG_PTR)Thread);
     }
 
     /* Lower to Passive Level */
@@ -361,8 +360,9 @@
     /* Can't be a worker thread */
     if (Thread->ActiveExWorker)
     {
+        /* Bugcheck */
         KEBUGCHECKEX(ACTIVE_EX_WORKER_THREAD_TERMINATION,
-                     (ULONG)Thread,
+                     (ULONG_PTR)Thread,
                      0,
                      0,
                      0);
@@ -371,6 +371,7 @@
     /* Can't have pending APCs */
     if (Thread->Tcb.CombinedApcDisable != 0)
     {
+        /* Bugcheck */
         KEBUGCHECKEX(KERNEL_APC_PENDING_DURING_EXIT,
                      0,
                      Thread->Tcb.KernelApcDisable,
@@ -378,9 +379,8 @@
                      0);
     }
 
-    /* FIXME: Lock the thread
-     * ExAcquireRundownProtect(&Thread->RundownProtect);
-     */
+    /* Lock the thread */
+    ExWaitForRundownProtectionRelease(&Thread->RundownProtect);
 
     /* Cleanup the power state */
     PopCleanupPowerState((PPOWER_STATE)&Thread->Tcb.PowerState);
@@ -413,7 +413,8 @@
             if (CurrentProcess->ExitStatus == STATUS_PENDING)
             {
                 /* Use the last exit status */
-                CurrentProcess->ExitStatus = CurrentProcess->LastThreadExitStatus;
+                CurrentProcess->ExitStatus = CurrentProcess->
+                                             LastThreadExitStatus;
             }
         }
         else
@@ -427,15 +428,14 @@
     else if (ExitStatus != STATUS_THREAD_IS_TERMINATING)
     {
         /* Write down the exit status of the last thread to get killed */
-    CurrentProcess->LastThreadExitStatus = ExitStatus;
+        CurrentProcess->LastThreadExitStatus = ExitStatus;
     }
 
     /* Unlock the Process */
     PsUnlockProcess(CurrentProcess);
 
     /* Check if the process has a debug port and if this is a user thread */
-    if ((CurrentProcess->DebugPort) && 
-        !(Thread->SystemThread))
+    if ((CurrentProcess->DebugPort) && !(Thread->SystemThread))
     {
         /* Notify the Debug API. */
         Last ? DbgkExitProcess(CurrentProcess->ExitStatus) :
@@ -457,7 +457,7 @@
         {
             /* FIXME: Add critical process support */
             DbgBreakPoint();
-    }
+        }
         else
         {
             /* Bugcheck, we can't allow this */
@@ -560,7 +560,8 @@
     }
 
     /* Rundown Win32 Thread if there is one */
-    if (Thread->Tcb.Win32Thread) PspW32ThreadCallout(Thread, FALSE);
+    if (Thread->Tcb.Win32Thread) PspW32ThreadCallout(Thread,
+                                                     PsW32ThreadCalloutExit);
 
     /* If we are the last thread and have a W32 Process */
     if ((Last) && (CurrentProcess->Win32Process))
@@ -585,14 +586,12 @@
     /* FIXME: Rundown Registry Notifications (NtChangeNotify)
     CmNotifyRunDown(Thread); */
 
-    /* Clear NPX Thread */
-    (void)InterlockedCompareExchangePointer(&KeGetCurrentPrcb()->NpxThread, NULL,
Thread);
-
     /* Rundown Mutexes */
     KeRundownThread();
 
-    /* Free the TEB */
-    if((Teb = Thread->Tcb.Teb))
+    /* Check if we have a TEB */
+    Teb = Thread->Tcb.Teb;
+    if(Teb)
     {
         /* Check if the thread isn't terminated and if we should free stack */
         if (!(Thread->Terminated) && (Teb->FreeStackOnTermination))
@@ -607,10 +606,11 @@
                                 &Dummy,
                                 MEM_RELEASE);
         }
-        
+
         /* Free the debug handle */
-        if (Teb->DbgSsReserved[2]) NtClose(Teb->DbgSsReserved[2]);
-        
+        if (Teb->DbgSsReserved[1]) ObCloseHandle(Teb->DbgSsReserved[1],
+                                                 UserMode);
+
         /* Decommit the TEB */
         MmDeleteTeb(CurrentProcess, Teb);
         Thread->Tcb.Teb = NULL;
@@ -680,7 +680,7 @@
 
     /* Flush the User APCs */
     FirstEntry = KeFlushQueueApc(&Thread->Tcb, UserMode);
-    if (FirstEntry != NULL)
+    if (FirstEntry)
     {
         CurrentEntry = FirstEntry;
         do
@@ -713,7 +713,8 @@
     if (Thread->Tcb.LegoData) PspRunLegoRoutine(&Thread->Tcb);
 
     /* Flush the APC queue, which should be empty */
-    if ((FirstEntry = KeFlushQueueApc(&Thread->Tcb, KernelMode)))
+    FirstEntry = KeFlushQueueApc(&Thread->Tcb, KernelMode);
+    if (FirstEntry)
     {
         /* Bugcheck time */
         KEBUGCHECKEX(KERNEL_APC_PENDING_DURING_EXIT,
@@ -740,10 +741,6 @@
 {
     NTSTATUS Status;
     PAGED_CODE();
-    DPRINT1("PsExitSpecialApc: [%p:%p], '%.16s')\n", 
-            PsGetCurrentThread(),
-            PsGetCurrentProcess(),
-            PsGetCurrentProcess()->ImageFileName);
 
     /* Don't do anything unless we are in User-Mode */
     if (Apc->SystemArgument2)
@@ -766,16 +763,11 @@
     PKAPC Apc = (PKAPC)SystemArgument1;
     PETHREAD Thread = PsGetCurrentThread();
     PAGED_CODE();
-    DPRINT1("PspExitNormalApc: [%p:%p], '%.16s')\n", 
-            PsGetCurrentThread(),
-            PsGetCurrentProcess(),
-            PsGetCurrentProcess()->ImageFileName);
 
     /* This should never happen */
     ASSERT(!(((ULONG_PTR)SystemArgument2) & 1));
 
     /* If we're here, this is not a System Thread, so kill it from User-Mode */
-    DPRINT1("Initializing User-Mode APC\n");
     KeInitializeApc(Apc,
                     &Thread->Tcb,
                     OriginalApcEnvironment,
@@ -812,7 +804,6 @@
     NTSTATUS Status = STATUS_SUCCESS;
     ULONG Flags;
     PAGED_CODE();
-    DPRINT1("PspTerminateThreadByPointer(%p, %lx)\n", Thread, ExitStatus);
 
     /* Check if this is a Critical Thread, and Bugcheck */
     if (Thread->BreakOnTermination)
@@ -823,7 +814,7 @@
     }
 
     /* Check if we are already inside the thread */
-    if (bSelf || (PsGetCurrentThread() == Thread))
+    if ((bSelf) || (PsGetCurrentThread() == Thread))
     {
         /* This should only happen at passive */
         ASSERT_IRQL(PASSIVE_LEVEL);
@@ -836,11 +827,7 @@
     }
 
     /* This shouldn't be a system thread */
-    if (Thread->SystemThread)
-    {
-        DPRINT1("A system thread is being illegaly terminated\n");
-        return STATUS_ACCESS_DENIED;
-    }
+    if (Thread->SystemThread) return STATUS_ACCESS_DENIED;
 
     /* Allocate the APC */
     Apc = ExAllocatePoolWithTag(NonPagedPool, sizeof(KAPC), TAG_TERMINATE_APC);
@@ -880,7 +867,6 @@
     ExFreePool(Apc);
 
     /* Return Status */
-    DPRINT1("Fail return: %lx\n", Status);
     return Status;
 }
 
@@ -891,7 +877,6 @@
 {
     ULONG Actual;
     PAGED_CODE();
-    DPRINT1("PspExitProcess %p\n", Process);
 
     /* Set Process Delete flag */
     InterlockedOr((PLONG)&Process->Flags, 4);
@@ -931,10 +916,8 @@
             /* Set it to default */
             ZwSetTimerResolution(KeMaximumIncrement, 0, &Actual);
         }
-    
-        /*
-         * Check if we are part of a Job, and if the job has a completion port
-         */
+
+        /* Check if we are part of a Job that has a completion port */
         if ((Process->Job) && (Process->Job->CompletionPort))
         {
             /* FIXME: Check job status code and do I/O completion if needed */
@@ -949,9 +932,34 @@
     }
 }
 
+/* PUBLIC FUNCTIONS **********************************************************/
+
+/*
+ * @implemented
+ */
 NTSTATUS
 NTAPI
-NtTerminateProcess(IN HANDLE ProcessHandle  OPTIONAL,
+PsTerminateSystemThread(NTSTATUS ExitStatus)
+{
+    PETHREAD Thread = PsGetCurrentThread();
+
+    /* Make sure this is a system thread */
+    if (Thread->SystemThread)
+    {
+        DPRINT1("Trying to Terminate a non-system thread!\n");
+        return STATUS_INVALID_PARAMETER;
+    }
+
+    /* Terminate it for real */
+    return PspTerminateThreadByPointer(Thread, ExitStatus, TRUE);
+}
+
+/*
+ * @implemented
+ */
+NTSTATUS
+NTAPI
+NtTerminateProcess(IN HANDLE ProcessHandle OPTIONAL,
                    IN NTSTATUS ExitStatus)
 {
     NTSTATUS Status;
@@ -959,8 +967,6 @@
     PETHREAD Thread, CurrentThread = PsGetCurrentThread();
     BOOLEAN KillByHandle;
     PAGED_CODE();
-    DPRINT1("NtTerminateProcess(ProcessHandle %x, ExitStatus %x)\n",
-            ProcessHandle, ExitStatus);
 
     /* Remember how we will kill it */
     KillByHandle = (ProcessHandle != NULL);
@@ -973,11 +979,7 @@
                                        KeGetPreviousMode(),
                                        (PVOID*)&Process,
                                        NULL);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Invalid handle to Process\n");
-        return(Status);
-    }
+    if (!NT_SUCCESS(Status)) return(Status);
 
     /* Check if this is a Critical Process, and Bugcheck */
     if (Process->BreakOnTermination)
@@ -988,14 +990,15 @@
     }
 
     /* Lock the Process */
-    PsLockProcess(Process, FALSE);
+    ExAcquireRundownProtection(&Process->RundownProtect);
 
     /* Set the exit flag */
     if (!KillByHandle) InterlockedOr((PLONG)&Process->Flags, 8);
 
     /* Get the first thread */
     Status = STATUS_NOTHING_TO_TERMINATE;
-    if ((Thread = PsGetNextProcessThread(Process, NULL)))
+    Thread = PsGetNextProcessThread(Process, NULL);
+    if (Thread)
     {
         /* We know we have at least a thread */
         Status = STATUS_SUCCESS;
@@ -1015,7 +1018,7 @@
     }
 
     /* Unlock the process */
-    PsUnlockProcess(Process);
+    ExReleaseRundownProtection(&Process->RundownProtect);
 
     /* Check if we are killing ourselves */
     if (Process != CurrentProcess)
@@ -1038,10 +1041,9 @@
 
     /* Check if there was nothing to terminate, or if we have a Debug Port */
     if ((Status == STATUS_NOTHING_TO_TERMINATE) ||
-        (Process->DebugPort && KillByHandle))
+        ((Process->DebugPort) && (KillByHandle)))
     {
         /* Clear the handle table */
-        DPRINT1("Clearing OB Table\n");
         ObClearProcessHandleTable(Process);
 
         /* Return status now */
@@ -1052,7 +1054,6 @@
     ObDereferenceObject(Process);
 
     /* Return status */
-    DPRINT1("Returning: %lx\n", Status);
     return Status;
 }
 
@@ -1065,8 +1066,7 @@
     PETHREAD CurrentThread = PsGetCurrentThread();
     NTSTATUS Status;
     PAGED_CODE();
-    DPRINT1("NtTerminateThread: %lx %lx\n", ThreadHandle, ExitStatus);
-    
+
     /* Handle the special NULL case */
     if (!ThreadHandle)
     {
@@ -1074,14 +1074,11 @@
         if (PsGetCurrentProcess()->ActiveThreads == 1)
         {
             /* This is invalid */
-            DPRINT1("Can't terminate self\n");
             return STATUS_CANT_TERMINATE_SELF;
         }
-        else
-        {
-            /* Terminate us directly */
-            goto TerminateSelf;
-        }
+
+        /* Terminate us directly */
+        goto TerminateSelf;
     }
     else if (ThreadHandle == NtCurrentThread())
     {
@@ -1099,16 +1096,12 @@
                                        KeGetPreviousMode(),
                                        (PVOID*)&Thread,
                                        NULL);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Could not reference thread object\n");
-        return Status;
-    }
+    if (!NT_SUCCESS(Status)) return Status;
 
     /* Check to see if we're running in the same thread */
     if (Thread != CurrentThread)
     {
-             /* Terminate it */
+        /* Terminate it */
         Status = PspTerminateThreadByPointer(Thread, ExitStatus, FALSE);
 
         /* Dereference the Thread and return */
@@ -1121,32 +1114,13 @@
         goto TerminateSelf;
     }
 
+    /* Return status */
     return Status;
 }
 
-/*
- * @implemented
- */
 NTSTATUS
 NTAPI
-PsTerminateSystemThread(NTSTATUS ExitStatus)
-{
-    PETHREAD Thread = PsGetCurrentThread();
-
-    /* Make sure this is a system thread */
-    if (Thread->SystemThread)
-    {
-        DPRINT1("Trying to Terminate a non-system thread!\n");
-        return STATUS_INVALID_PARAMETER;
-    }
-
-    /* Terminate it for real */
-    return PspTerminateThreadByPointer(Thread, ExitStatus, TRUE);
-}
-
-NTSTATUS
-NTAPI
-NtRegisterThreadTerminatePort(HANDLE PortHandle)
+NtRegisterThreadTerminatePort(IN HANDLE PortHandle)
 {
     NTSTATUS Status;
     PTERMINATION_PORT TerminationPort;
@@ -1161,17 +1135,14 @@
                                        KeGetPreviousMode(),
                                        &TerminationLpcPort,
                                        NULL);
-    if (!NT_SUCCESS(Status)) {
-
-        DPRINT1("Failed to reference Port\n");
-        return(Status);
-    }
+    if (!NT_SUCCESS(Status)) return(Status);
 
     /* Allocate the Port and make sure it suceeded */
-    if((TerminationPort = ExAllocatePoolWithTag(NonPagedPool,
-                                                sizeof(TERMINATION_PORT),
-                                                TAG('P', 's',
'T', '=')))) {
-
+    TerminationPort = ExAllocatePoolWithTag(NonPagedPool,
+                                            sizeof(TERMINATION_PORT),
+                                            TAG('P', 's', 'T',
'='));
+    if(TerminationPort)
+    {
         /* Associate the Port */
         Thread = PsGetCurrentThread();
         TerminationPort->Port = TerminationLpcPort;
@@ -1179,12 +1150,10 @@
         Thread->TerminationPort = TerminationPort;
 
         /* Return success */
-        return(STATUS_SUCCESS);
-
-    } else {
-
-        /* Dereference and Fail */
-        ObDereferenceObject(TerminationPort);
-        return(STATUS_INSUFFICIENT_RESOURCES);
-    }
-}
+        return STATUS_SUCCESS;
+    }
+
+    /* Dereference and Fail */
+    ObDereferenceObject(TerminationPort);
+    return STATUS_INSUFFICIENT_RESOURCES;
+}