From 2ed6687632ff2fdf42d1c0941698ee2f09dfcf7e Mon Sep 17 00:00:00 2001 From: Aleksey Bragin Date: Wed, 14 Jan 2009 09:51:50 +0000 Subject: [PATCH] - Revert 38750, a dereference happens when that hook is freed by IntFreeHook. - Change thread dereferencing mechanism in NtUserSetWindowsHookEx: * A thread may be referenced in some cases when it's a system-wide hook, however dereference flag was not set for these, so they leaked a reference. Add a ThreadReferenced boolean var to track real references, and set hook's flag according to it. * There should be no need to call ObDereferenceObject on a thread when a call to IntRemoveHook was already performed (in failure branches). IntFreeHook() will dereference the thread object. svn path=/trunk/; revision=38754 --- reactos/subsystems/win32/win32k/ntuser/hook.c | 45 +++++++------------ 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/reactos/subsystems/win32/win32k/ntuser/hook.c b/reactos/subsystems/win32/win32k/ntuser/hook.c index d0ff498fe00..230875716f6 100644 --- a/reactos/subsystems/win32/win32k/ntuser/hook.c +++ b/reactos/subsystems/win32/win32k/ntuser/hook.c @@ -969,6 +969,7 @@ NtUserSetWindowsHookEx( UNICODE_STRING ModuleName; NTSTATUS Status; HHOOK Handle; + BOOLEAN ThreadReferenced = FALSE; DECLARE_RETURN(HHOOK); DPRINT("Enter NtUserSetWindowsHookEx\n"); @@ -1008,6 +1009,8 @@ NtUserSetWindowsHookEx( SetLastWin32Error(ERROR_INVALID_PARAMETER); RETURN( NULL); } + /* Thread was referenced */ + ThreadReferenced = TRUE; if (Thread->ThreadsProcess != PsGetCurrentProcess()) { ObDereferenceObject(Thread); @@ -1032,6 +1035,9 @@ NtUserSetWindowsHookEx( SetLastNtError(Status); RETURN( (HANDLE) NULL); } + + /* Thread was referenced */ + ThreadReferenced = TRUE; } else if (NULL == Mod) { @@ -1056,10 +1062,8 @@ NtUserSetWindowsHookEx( DPRINT1("Not implemented: HookId %d Global %s\n", HookId, Global ? "TRUE" : "FALSE"); #endif - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } + /* Dereference thread if needed */ + if (ThreadReferenced) ObDereferenceObject(Thread); SetLastWin32Error(ERROR_NOT_SUPPORTED); RETURN( NULL); } @@ -1071,10 +1075,8 @@ NtUserSetWindowsHookEx( if (! NT_SUCCESS(Status)) { - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } + /* Dereference thread if needed */ + if (ThreadReferenced) ObDereferenceObject(Thread); SetLastNtError(Status); RETURN( (HANDLE) NULL); } @@ -1082,15 +1084,14 @@ NtUserSetWindowsHookEx( Hook = IntAddHook(Thread, HookId, Global, WinStaObj); if (NULL == Hook) { - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } + /* Dereference thread if needed */ + if (ThreadReferenced) ObDereferenceObject(Thread); ObDereferenceObject(WinStaObj); RETURN( NULL); } - if (NULL != Thread) + /* Let IntFreeHook now that this thread needs a dereference */ + if (ThreadReferenced) { Hook->Flags |= HOOK_THREAD_REFERENCED; } @@ -1102,10 +1103,6 @@ NtUserSetWindowsHookEx( { UserDereferenceObject(Hook); IntRemoveHook(Hook, WinStaObj, FALSE); - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } ObDereferenceObject(WinStaObj); SetLastNtError(Status); RETURN( NULL); @@ -1117,10 +1114,6 @@ NtUserSetWindowsHookEx( { UserDereferenceObject(Hook); IntRemoveHook(Hook, WinStaObj, FALSE); - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } ObDereferenceObject(WinStaObj); SetLastWin32Error(ERROR_NOT_ENOUGH_MEMORY); RETURN( NULL); @@ -1134,10 +1127,6 @@ NtUserSetWindowsHookEx( ExFreePoolWithTag(Hook->ModuleName.Buffer, TAG_HOOK); UserDereferenceObject(Hook); IntRemoveHook(Hook, WinStaObj, FALSE); - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } ObDereferenceObject(WinStaObj); SetLastNtError(Status); RETURN( NULL); @@ -1154,13 +1143,9 @@ NtUserSetWindowsHookEx( // Clear the client threads next hook. ClientInfo->phkCurrent = 0; - + UserDereferenceObject(Hook); - if (NULL != Thread) - { - ObDereferenceObject(Thread); - } ObDereferenceObject(WinStaObj); RETURN( Handle);