From ce7fb8c1b94b75f8c7b0808e2214287bafa76897 Mon Sep 17 00:00:00 2001 From: Alex Ionescu Date: Mon, 18 Jul 2005 19:50:23 +0000 Subject: [PATCH] - 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. svn path=/trunk/; revision=16635 --- reactos/ntoskrnl/ke/apc.c | 54 ++++++++++++++--------------- reactos/ntoskrnl/ps/kill.c | 71 +++++++++++++++++++++++++++++--------- 2 files changed, 81 insertions(+), 44 deletions(-) diff --git a/reactos/ntoskrnl/ke/apc.c b/reactos/ntoskrnl/ke/apc.c index 242c4248d41..e19a09ffa6f 100644 --- a/reactos/ntoskrnl/ke/apc.c +++ b/reactos/ntoskrnl/ke/apc.c @@ -567,9 +567,9 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, 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 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, if (NormalRoutine == NULL) { /* Remove the APC from the list */ - Apc->Inserted = FALSE; RemoveEntryList(ApcListEntry); + Apc->Inserted = FALSE; /* Go back to APC_LEVEL */ KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql); @@ -586,10 +586,10 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, /* 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 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, } /* Dequeue the APC */ - RemoveEntryList(ApcListEntry); Apc->Inserted = FALSE; + RemoveEntryList(ApcListEntry); /* Go back to APC_LEVEL */ KeReleaseSpinLock(&Thread->ApcQueueLock, OldIrql); @@ -621,10 +621,10 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, /* 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 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, /* 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 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, 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); + 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 @@ KiDeliverApc(KPROCESSOR_MODE DeliveryMode, /* 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 { diff --git a/reactos/ntoskrnl/ps/kill.c b/reactos/ntoskrnl/ps/kill.c index b4816b01641..f45cd2ae74d 100644 --- a/reactos/ntoskrnl/ps/kill.c +++ b/reactos/ntoskrnl/ps/kill.c @@ -346,18 +346,23 @@ PsExitSpecialApc(PKAPC Apc, 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 @@ PspExitNormalApc(PVOID NormalContext, 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 @@ PspTerminateThreadByPointer(PETHREAD Thread, /* 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 */