[ros-diffs] [ion] 16635: - Fix nasty APC delivery bug (in case a Kernel-Mode Special APC still returned with a Normal Routine, the Normal Routine was called with incorrect values (Special Routines take PVOID* arguments, while Normal Routines do not!))

- Fix nasty APC delivery bug (in case a Kernel-Mode Special APC still
returned with a Normal Routine, the Normal Routine was called with
incorrect values (Special Routines take PVOID* arguments, while Normal
Routines do not!))
 - Remove APC from list before setting it to non-inserted.
 - Do proper thread termination piggybacking; terminate threads in
user-mode as Hartmut correctly fixed.
Modified: trunk/reactos/ntoskrnl/ke/apc.c
Modified: trunk/reactos/ntoskrnl/ps/kill.c
  _____  

Modified: trunk/reactos/ntoskrnl/ke/apc.c
--- trunk/reactos/ntoskrnl/ke/apc.c	2005-07-18 15:05:58 UTC (rev
16634)
+++ trunk/reactos/ntoskrnl/ke/apc.c	2005-07-18 19:50:23 UTC (rev
16635)
@@ -567,9 +567,9 @@

         Apc = CONTAINING_RECORD(ApcListEntry, KAPC, ApcListEntry);
 
         /* Save Parameters so that it's safe to free the Object in
Kernel Routine*/
-        NormalRoutine   = Apc->NormalRoutine;
-        KernelRoutine   = Apc->KernelRoutine;
-        NormalContext   = Apc->NormalContext;
+        NormalRoutine = Apc->NormalRoutine;
+        KernelRoutine = Apc->KernelRoutine;
+        NormalContext = Apc->NormalContext;
         SystemArgument1 = Apc->SystemArgument1;
         SystemArgument2 = Apc->SystemArgument2;
 
@@ -577,8 +577,8 @@
        if (NormalRoutine == NULL) {
 
             /* Remove the APC from the list */
+            RemoveEntryList(ApcListEntry);
             Apc->Inserted = FALSE;
-            RemoveEntryList(ApcListEntry);
 
             /* Go back to APC_LEVEL */
             KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql);
@@ -586,10 +586,10 @@
             /* Call the Special APC */
             DPRINT("Delivering a Special APC: %x\n", Apc);
             KernelRoutine(Apc,
-                      &NormalRoutine,
-                      &NormalContext,
-                      &SystemArgument1,
-                      &SystemArgument2);
+                          &NormalRoutine,
+                          &NormalContext,
+                          &SystemArgument1,
+                          &SystemArgument2);
 
             /* Raise IRQL and Lock again */
             KeAcquireSpinLock(&Thread->ApcQueueLock, &OldIrql);
@@ -612,8 +612,8 @@
             }
 
             /* Dequeue the APC */
+            Apc->Inserted = FALSE;
             RemoveEntryList(ApcListEntry);
-            Apc->Inserted = FALSE;
 
             /* Go back to APC_LEVEL */
             KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql);
@@ -621,10 +621,10 @@
             /* Call the Kernel APC */
             DPRINT("Delivering a Normal APC: %x\n", Apc);
             KernelRoutine(Apc,
-                      &NormalRoutine,
-                      &NormalContext,
-                      &SystemArgument1,
-                      &SystemArgument2);
+                          &NormalRoutine,
+                          &NormalContext,
+                          &SystemArgument1,
+                          &SystemArgument2);
 
             /* If There still is a Normal Routine, then we need to call
this at PASSIVE_LEVEL */
             if (NormalRoutine != NULL) {
@@ -635,7 +635,7 @@
 
                 /* Call and Raise IRQ back to APC_LEVEL */
                 DPRINT("Calling the Normal Routine for a Normal APC:
%x\n", Apc);
-                NormalRoutine(&NormalContext, &SystemArgument1,
&SystemArgument2);
+                NormalRoutine(NormalContext, SystemArgument1,
SystemArgument2);
                 KeRaiseIrql(APC_LEVEL, &OldIrql);
             }
 
@@ -657,23 +657,23 @@
         Apc = CONTAINING_RECORD(ApcListEntry, KAPC, ApcListEntry);
 
         /* Save Parameters so that it's safe to free the Object in
Kernel Routine*/
-        NormalRoutine   = Apc->NormalRoutine;
-        KernelRoutine   = Apc->KernelRoutine;
-        NormalContext   = Apc->NormalContext;
+        NormalRoutine = Apc->NormalRoutine;
+        KernelRoutine = Apc->KernelRoutine;
+        NormalContext = Apc->NormalContext;
         SystemArgument1 = Apc->SystemArgument1;
         SystemArgument2 = Apc->SystemArgument2;
 
         /* Remove the APC from Queue, restore IRQL and call the APC */
         RemoveEntryList(ApcListEntry);
         Apc->Inserted = FALSE;
+        KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql);
 
-        KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql);
         DPRINT("Calling the Kernel Routine for for a User APC: %x\n",
Apc);
         KernelRoutine(Apc,
-                  &NormalRoutine,
-                  &NormalContext,
-                  &SystemArgument1,
-                  &SystemArgument2);
+                      &NormalRoutine,
+                      &NormalContext,
+                      &SystemArgument1,
+                      &SystemArgument2);
 
         if (NormalRoutine == NULL) {
 
@@ -685,11 +685,11 @@
             /* Set up the Trap Frame and prepare for Execution in
NTDLL.DLL */
             DPRINT("Delivering a User APC: %x\n", Apc);
             KiInitializeUserApc(Reserved,
-                        TrapFrame,
-                        NormalRoutine,
-                        NormalContext,
-                        SystemArgument1,
-                        SystemArgument2);
+                                TrapFrame,
+                                NormalRoutine,
+                                NormalContext,
+                                SystemArgument1,
+                                SystemArgument2);
         }
 
     } else {
  _____  

Modified: trunk/reactos/ntoskrnl/ps/kill.c
--- trunk/reactos/ntoskrnl/ps/kill.c	2005-07-18 15:05:58 UTC (rev
16634)
+++ trunk/reactos/ntoskrnl/ps/kill.c	2005-07-18 19:50:23 UTC (rev
16635)
@@ -346,18 +346,23 @@

                  PVOID* SystemArgument1,
                  PVOID* SystemArguemnt2)
 {
-    NTSTATUS ExitStatus = (NTSTATUS)Apc->NormalContext;
+    DPRINT1("PsExitSpecialApc called: 0x%x (proc: 0x%x)\n", 
+            PsGetCurrentThread(), PsGetCurrentProcess());
 
-    DPRINT("PsExitSpecialApc called: 0x%x (proc: 0x%x)\n",
PsGetCurrentThread(), PsGetCurrentProcess());
+    /* Don't do anything unless we are in User-Mode */
+    if (Apc->SystemArgument2)
+    {
+        NTSTATUS ExitStatus = (NTSTATUS)Apc->NormalContext;
 
-    /* Free the APC */
-    ExFreePool(Apc);
+        /* Free the APC */
+        ExFreePool(Apc);
 
-    /* Terminate the Thread */
-    PspExitThread(ExitStatus);
+        /* Terminate the Thread */
+        PspExitThread(ExitStatus);
 
-    /* we should never reach this point! */
-    KEBUGCHECK(0);
+        /* we should never reach this point! */
+        KEBUGCHECK(0);
+    }
 }
 
 VOID
@@ -366,14 +371,46 @@
                  PVOID SystemArgument1,
                  PVOID SystemArgument2)
 {
-    /* Not fully supported yet... must work out some issues that
-     * I don't understand yet -- Alex
-     */
-    DPRINT1("APC2\n");
-    PspExitThread((NTSTATUS)NormalContext);
+    PKAPC Apc = (PKAPC)SystemArgument1;
+    PETHREAD Thread = PsGetCurrentThread();
+    NTSTATUS ExitStatus;
+        
+    DPRINT1("PspExitNormalApc called: 0x%x (proc: 0x%x)\n", 
+            PsGetCurrentThread(), PsGetCurrentProcess());
 
-    /* we should never reach this point! */
-    KEBUGCHECK(0);
+    /* This should never happen */
+    ASSERT(!SystemArgument2);
+
+    /* If this is a system thread, we can safely kill it from
Kernel-Mode */
+    if (PsIsSystemThread(Thread))
+    {
+        /* Get the Exit Status */
+        DPRINT1("Killing System Thread\n");
+        ExitStatus = (NTSTATUS)Apc->NormalContext;
+
+        /* Free the APC */
+        ExFreePool(Apc);
+
+        /* Exit the Thread */
+        PspExitThread(ExitStatus);
+    }
+
+    /* 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,
+                    PsExitSpecialApc,
+                    NULL,
+                    PspExitNormalApc,
+                    UserMode,
+                    NormalContext);
+
+    /* Now insert the APC with the User-Mode Flag */
+    KeInsertQueueApc(Apc, Apc, (PVOID)UserMode, 2);
+
+    /* Forcefully resume the thread */
+    KeForceResumeThread(&Thread->Tcb);
 }
 
 /*
@@ -402,14 +439,14 @@
     /* Allocate the APC */
     Apc = ExAllocatePoolWithTag(NonPagedPool, sizeof(KAPC),
TAG_TERMINATE_APC);
 
-    /* Initialize a User Mode APC to Kill the Thread */
+    /* Initialize a Kernel Mode APC to Kill the Thread */
     KeInitializeApc(Apc,
                     &Thread->Tcb,
                     OriginalApcEnvironment,
                     PsExitSpecialApc,
                     NULL,
                     PspExitNormalApc,
-                    UserMode,
+                    KernelMode,
                     (PVOID)ExitStatus);
 
     /* Insert it into the APC Queue */