- Crash while in recursion through hook calls. Move fast calls to api. Placed safe guards for removing message queue structures. Lock threads while calling hooks. Just add paranoid code to prevent recursion freeing that will crash the kernel at some point. Fixes OpenMPT crashes, see CORE-8819. This might improve the same issues with CORE-6734.

svn path=/trunk/; revision=65815
This commit is contained in:
James Tabor 2014-12-23 19:05:44 +00:00
parent 117b0daf21
commit 4b6fb8acdf
6 changed files with 159 additions and 103 deletions

View file

@ -2,7 +2,7 @@
* COPYRIGHT: See COPYING in the top level directory
* PROJECT: ReactOS kernel
* PURPOSE: Window event handlers
* FILE: subsystems/win32/win32k/ntuser/event.c
* FILE: win32ss/user/ntuser/event.c
* PROGRAMER: James Tabor (james.tabor@rectos.org)
*/
@ -153,7 +153,7 @@ IntRemoveEvent(PVOID Object)
// Dispatch MsgQueue Event Call processor!
//
LRESULT
FASTCALL
APIENTRY
co_EVENT_CallEvents( DWORD event,
HWND hwnd,
UINT_PTR idObject,

View file

@ -2,7 +2,7 @@
* COPYRIGHT: See COPYING in the top level directory
* PROJECT: ReactOS kernel
* PURPOSE: Window hooks
* FILE: subsystems/win32/win32k/ntuser/hook.c
* FILE: win32ss/user/ntuser/hook.c
* PROGRAMER: Casper S. Hornstrup (chorns@users.sourceforge.net)
* James Tabor (james.tabor@rectos.org)
* Rafal Harabien (rafalh@reactos.org)
@ -92,7 +92,7 @@ IntHookModuleUnloaded(PDESKTOP pdesk, int iHookID, HHOOK hHook)
PLIST_ENTRY ListEntry;
PPROCESSINFO ppiCsr;
ERR("IntHookModuleUnloaded: iHookID=%d\n", iHookID);
TRACE("IntHookModuleUnloaded: iHookID=%d\n", iHookID);
ppiCsr = PsGetProcessWin32Process(gpepCSRSS);
@ -222,7 +222,7 @@ UserUnregisterUserApiHook(VOID)
return FALSE;
}
ERR("UserUnregisterUserApiHook. Server PID: %p\n", PsGetProcessId(pti->ppi->peProcess));
TRACE("UserUnregisterUserApiHook. Server PID: %p\n", PsGetProcessId(pti->ppi->peProcess));
/* Unregister the api hook */
gpsi->dwSRVIFlags &= ~SRVINFO_APIHOOK;
@ -318,7 +318,7 @@ co_IntCallLowLevelHook(PHOOK Hook,
// Dispatch MsgQueue Hook Call processor!
//
LRESULT
FASTCALL
APIENTRY
co_CallHook( INT HookId,
INT Code,
WPARAM wParam,
@ -335,14 +335,13 @@ co_CallHook( INT HookId,
{
case WH_JOURNALPLAYBACK:
case WH_JOURNALRECORD:
case WH_KEYBOARD:
case WH_KEYBOARD_LL:
case WH_MOUSE_LL:
case WH_MOUSE:
lParam = (LPARAM)pHP->pHookStructs;
case WH_KEYBOARD:
break;
}
/* The odds are high for this to be a Global call. */
Result = co_IntCallHookProc( HookId,
Code,
@ -362,7 +361,7 @@ co_CallHook( INT HookId,
static
LRESULT
FASTCALL
APIENTRY
co_HOOK_CallHookNext( PHOOK Hook,
INT Code,
WPARAM wParam,
@ -516,7 +515,7 @@ co_IntCallDebugHook(PHOOK Hook,
static
LRESULT
FASTCALL
APIENTRY
co_UserCallNextHookEx(PHOOK Hook,
int Code,
WPARAM wParam,
@ -1080,7 +1079,7 @@ IntRemoveHook(PVOID Object)
Win32k Kernel Space Hook Caller.
*/
LRESULT
FASTCALL
APIENTRY
co_HOOK_CallHooks( INT HookId,
INT Code,
WPARAM wParam,
@ -1150,6 +1149,8 @@ co_HOOK_CallHooks( INT HookId,
}
Hook = CONTAINING_RECORD(pLastHead->Flink, HOOK, Chain);
ObReferenceObject(pti->pEThread);
IntReferenceThreadInfo(pti);
UserRefObjectCo(Hook, &Ref);
ClientInfo = pti->pClientInfo;
@ -1199,6 +1200,8 @@ co_HOOK_CallHooks( INT HookId,
pti->sphkCurrent = SaveHook;
Hook->phkNext = NULL;
UserDerefObjectCo(Hook);
IntDereferenceThreadInfo(pti);
ObDereferenceObject(pti->pEThread);
}
if ( Global )
@ -1224,7 +1227,6 @@ co_HOOK_CallHooks( INT HookId,
ERR("Invalid hook!\n");
continue;
}
UserRefObjectCo(Hook, &Ref);
/* Hook->Thread is null, we hax around this with Hook->head.pti. */
ptiHook = Hook->head.pti;
@ -1234,6 +1236,7 @@ co_HOOK_CallHooks( INT HookId,
TRACE("Next Hook %p, %p\n", ptiHook->rpdesk, pdo);
continue;
}
UserRefObjectCo(Hook, &Ref);
if (ptiHook != pti )
{
@ -1251,7 +1254,9 @@ co_HOOK_CallHooks( INT HookId,
}
else
{ /* Make the direct call. */
TRACE("Local Hook calling to Thread! %d\n",HookId );
TRACE("Global going Local Hook calling to Thread! %d\n",HookId );
ObReferenceObject(pti->pEThread);
IntReferenceThreadInfo(pti);
Result = co_IntCallHookProc( HookId,
Code,
wParam,
@ -1261,6 +1266,8 @@ co_HOOK_CallHooks( INT HookId,
Hook->offPfn,
Hook->Ansi,
&Hook->ModuleName);
IntDereferenceThreadInfo(pti);
ObDereferenceObject(pti->pEThread);
}
UserDerefObjectCo(Hook);
}
@ -1434,7 +1441,7 @@ NtUserSetWindowsHookEx( HINSTANCE Mod,
HookId == WH_MOUSE_LL ||
HookId == WH_SYSMSGFILTER)
{
ERR("Local hook installing Global HookId: %d\n",HookId);
TRACE("Local hook installing Global HookId: %d\n",HookId);
/* these can only be global */
EngSetLastError(ERROR_GLOBAL_ONLY_HOOK);
RETURN( NULL);

View file

@ -40,12 +40,12 @@ typedef struct _NOTIFYEVENT
DWORD flags;
} NOTIFYEVENT, *PNOTIFYEVENT;
LRESULT FASTCALL co_CallHook(INT HookId, INT Code, WPARAM wParam, LPARAM lParam);
LRESULT FASTCALL co_HOOK_CallHooks(INT HookId, INT Code, WPARAM wParam, LPARAM lParam);
LRESULT FASTCALL co_EVENT_CallEvents(DWORD, HWND, UINT_PTR, LONG_PTR);
LRESULT APIENTRY co_CallHook(INT HookId, INT Code, WPARAM wParam, LPARAM lParam);
LRESULT APIENTRY co_HOOK_CallHooks(INT HookId, INT Code, WPARAM wParam, LPARAM lParam);
LRESULT APIENTRY co_EVENT_CallEvents(DWORD, HWND, UINT_PTR, LONG_PTR);
PHOOK FASTCALL IntGetHookObject(HHOOK);
PHOOK FASTCALL IntGetNextHook(PHOOK Hook);
LRESULT FASTCALL UserCallNextHookEx( PHOOK pHook, int Code, WPARAM wParam, LPARAM lParam, BOOL Ansi);
LRESULT APIENTRY UserCallNextHookEx( PHOOK pHook, int Code, WPARAM wParam, LPARAM lParam, BOOL Ansi);
BOOL FASTCALL IntUnhookWindowsHook(int,HOOKPROC);
BOOLEAN IntRemoveHook(PVOID Object);
BOOLEAN IntRemoveEvent(PVOID Object);

View file

@ -766,7 +766,7 @@ IntDispatchMessage(PMSG pMsg)
* WM_PAINT messages
* WM_TIMER messages
*/
BOOL FASTCALL
BOOL APIENTRY
co_IntPeekMessage( PMSG Msg,
PWND Window,
UINT MsgFilterMin,
@ -952,7 +952,7 @@ co_IntWaitMessage( PWND Window,
return FALSE;
}
BOOL FASTCALL
BOOL APIENTRY
co_IntGetPeekMessage( PMSG pMsg,
HWND hWnd,
UINT MsgFilterMin,

View file

@ -26,9 +26,9 @@ NTSTATUS
NTAPI
MsqInitializeImpl(VOID)
{
pgMessageLookasideList = ExAllocatePoolWithTag(NonPagedPool, sizeof(PAGED_LOOKASIDE_LIST), TAG_USRMSG);
if(!pgMessageLookasideList)
return STATUS_NO_MEMORY;
pgMessageLookasideList = ExAllocatePoolWithTag(NonPagedPool, sizeof(PAGED_LOOKASIDE_LIST), TAG_USRMSG);
if (!pgMessageLookasideList)
return STATUS_NO_MEMORY;
ExInitializePagedLookasideList(pgMessageLookasideList,
NULL,
NULL,
@ -732,6 +732,7 @@ MsqCreateMessage(LPMSG Msg)
return NULL;
}
RtlZeroMemory(Message, sizeof(Message));
RtlMoveMemory(&Message->Msg, Msg, sizeof(MSG));
return Message;
@ -740,6 +741,12 @@ MsqCreateMessage(LPMSG Msg)
VOID FASTCALL
MsqDestroyMessage(PUSER_MESSAGE Message)
{
if (Message->pti == NULL)
{
ERR("Double Free Message\n");
return;
}
Message->pti = NULL;
ExFreeToPagedLookasideList(pgMessageLookasideList, Message);
}
@ -1141,7 +1148,6 @@ co_MsqSendMessage(PTHREADINFO ptirec,
UserLeaveCo();
/* Don't process messages sent to the thread */
WaitStatus = KeWaitForMultipleObjects(2, WaitObjects, WaitAny, UserRequest,
UserMode, FALSE, (uTimeout ? &Timeout : NULL), NULL);
@ -1306,7 +1312,7 @@ co_MsqSendMessage(PTHREADINFO ptirec,
ERR("User APC Returned\n"); // Should not see this message.
}
if (WaitStatus != STATUS_TIMEOUT)
if ( WaitStatus != STATUS_TIMEOUT )
{
if (uResult)
{
@ -1344,18 +1350,15 @@ MsqPostMessage(PTHREADINFO pti,
if (dwQEvent)
{
ERR("Post Msg; System Qeued Event Message!\n");
InsertHeadList(&pti->PostedMessagesListHead,
&Message->ListEntry);
InsertHeadList(&pti->PostedMessagesListHead, &Message->ListEntry);
}
else if (!HardwareMessage)
{
InsertTailList(&pti->PostedMessagesListHead,
&Message->ListEntry);
InsertTailList(&pti->PostedMessagesListHead, &Message->ListEntry);
}
else
{
InsertTailList(&MessageQueue->HardwareMessagesListHead,
&Message->ListEntry);
InsertTailList(&MessageQueue->HardwareMessagesListHead, &Message->ListEntry);
}
if (Msg->message == WM_HOTKEY) MessageBits |= QS_HOTKEY; // Justin Case, just set it.
@ -1669,6 +1672,7 @@ BOOL co_IntProcessMouseMessage(MSG* msg, BOOL* RemoveMessages, UINT first, UINT
{
/* Remove and ignore the message */
*RemoveMessages = TRUE;
TRACE("Remove and ignore the message\n");
RETURN(FALSE);
}
}
@ -1784,6 +1788,10 @@ CLEANUP:
BOOL co_IntProcessKeyboardMessage(MSG* Msg, BOOL* RemoveMessages)
{
EVENTMSG Event;
USER_REFERENCE_ENTRY Ref;
PWND pWnd;
BOOL Ret = TRUE;
PTHREADINFO pti = PsGetCurrentThreadWin32Thread();
if (Msg->message == WM_KEYDOWN || Msg->message == WM_SYSKEYDOWN ||
Msg->message == WM_KEYUP || Msg->message == WM_SYSKEYUP)
@ -1802,6 +1810,9 @@ BOOL co_IntProcessKeyboardMessage(MSG* Msg, BOOL* RemoveMessages)
}
}
pWnd = ValidateHwndNoErr(Msg->hwnd);
if (pWnd) UserRefObjectCo(pWnd, &Ref);
Event.message = Msg->message;
Event.hwnd = Msg->hwnd;
Event.time = Msg->time;
@ -1810,8 +1821,33 @@ BOOL co_IntProcessKeyboardMessage(MSG* Msg, BOOL* RemoveMessages)
if (HIWORD(Msg->lParam) & 0x0100) Event.paramH |= 0x8000;
co_HOOK_CallHooks( WH_JOURNALRECORD, HC_ACTION, 0, (LPARAM)&Event);
if (*RemoveMessages)
{
if ((Msg->message == WM_KEYDOWN) &&
(Msg->hwnd != IntGetDesktopWindow()))
{
/* Handle F1 key by sending out WM_HELP message */
if (Msg->wParam == VK_F1)
{
UserPostMessage( Msg->hwnd, WM_KEYF1, 0, 0 );
}
else if (Msg->wParam >= VK_BROWSER_BACK &&
Msg->wParam <= VK_LAUNCH_APP2)
{
/* FIXME: Process keystate */
co_IntSendMessage(Msg->hwnd, WM_APPCOMMAND, (WPARAM)Msg->hwnd, MAKELPARAM(0, (FAPPCOMMAND_KEY | (Msg->wParam - VK_BROWSER_BACK + 1))));
}
}
else if (Msg->message == WM_KEYUP)
{
/* Handle VK_APPS key by posting a WM_CONTEXTMENU message */
if (Msg->wParam == VK_APPS && pti->MessageQueue->MenuOwner == NULL)
UserPostMessage( Msg->hwnd, WM_CONTEXTMENU, (WPARAM)Msg->hwnd, -1 );
}
}
if (co_HOOK_CallHooks( WH_KEYBOARD,
*RemoveMessages ? HC_ACTION : HC_NOREMOVE,
*RemoveMessages ? HC_ACTION : HC_NOREMOVE,
LOWORD(Msg->wParam),
Msg->lParam))
{
@ -1820,10 +1856,15 @@ BOOL co_IntProcessKeyboardMessage(MSG* Msg, BOOL* RemoveMessages)
HCBT_KEYSKIPPED,
LOWORD(Msg->wParam),
Msg->lParam );
ERR("KeyboardMessage WH_CBT Call Hook return!\n");
return FALSE;
ERR("KeyboardMessage WH_KEYBOARD Call Hook return!\n");
*RemoveMessages = TRUE;
Ret = FALSE;
}
return TRUE;
if (pWnd) UserDerefObjectCo(pWnd);
return Ret;
}
BOOL co_IntProcessHardwareMessage(MSG* Msg, BOOL* RemoveMessages, UINT first, UINT last)
@ -1866,41 +1907,43 @@ co_MsqPeekHardwareMessage(IN PTHREADINFO pti,
IN UINT QSflags,
OUT MSG* pMsg)
{
BOOL AcceptMessage;
PUSER_MESSAGE CurrentMessage;
PLIST_ENTRY ListHead;
MSG msg;
ULONG_PTR idSave;
DWORD QS_Flags;
BOOL Ret = FALSE;
PUSER_MESSAGE_QUEUE MessageQueue = pti->MessageQueue;
BOOL AcceptMessage;
PUSER_MESSAGE CurrentMessage;
PLIST_ENTRY ListHead, CurrentEntry = NULL;
MSG msg;
BOOL Ret = FALSE;
PUSER_MESSAGE_QUEUE MessageQueue = pti->MessageQueue;
if (!filter_contains_hw_range( MsgFilterLow, MsgFilterHigh )) return FALSE;
if (!filter_contains_hw_range( MsgFilterLow, MsgFilterHigh )) return FALSE;
ListHead = MessageQueue->HardwareMessagesListHead.Flink;
ListHead = &MessageQueue->HardwareMessagesListHead;
CurrentEntry = ListHead->Flink;
if (IsListEmpty(ListHead)) return FALSE;
if (IsListEmpty(CurrentEntry)) return FALSE;
if (!MessageQueue->ptiSysLock)
{
MessageQueue->ptiSysLock = pti;
pti->pcti->CTI_flags |= CTI_THREADSYSLOCK;
}
if (!MessageQueue->ptiSysLock)
{
MessageQueue->ptiSysLock = pti;
pti->pcti->CTI_flags |= CTI_THREADSYSLOCK;
}
if (MessageQueue->ptiSysLock != pti)
{
ERR("MsqPeekHardwareMessage: Thread Q is locked to another pti!\n");
return FALSE;
}
if (MessageQueue->ptiSysLock != pti)
{
ERR("MsqPeekHardwareMessage: Thread Q is locked to another pti!\n");
return FALSE;
}
while (ListHead != &MessageQueue->HardwareMessagesListHead)
{
CurrentMessage = CONTAINING_RECORD(ListHead, USER_MESSAGE, ListEntry);
ListHead = ListHead->Flink;
CurrentMessage = CONTAINING_RECORD(CurrentEntry, USER_MESSAGE,
ListEntry);
do
{
if (IsListEmpty(CurrentEntry)) break;
if (!CurrentMessage) break;
CurrentEntry = CurrentMessage->ListEntry.Flink;
if (!CurrentEntry) break; //// Fix CORE-6734 reported crash.
if (MessageQueue->idSysPeek == (ULONG_PTR)CurrentMessage)
{
TRACE("Skip this message due to it is in play!\n");
continue;
}
/*
MSDN:
1: any window that belongs to the current thread, and any messages on the current thread's message queue whose hwnd value is NULL.
@ -1913,33 +1956,40 @@ co_MsqPeekHardwareMessage(IN PTHREADINFO pti,
( CurrentMessage->Msg.message == WM_MOUSEMOVE ) ) && // Null window for mouse moves.
( ( ( MsgFilterLow == 0 && MsgFilterHigh == 0 ) && CurrentMessage->QS_Flags & QSflags ) ||
( MsgFilterLow <= CurrentMessage->Msg.message && MsgFilterHigh >= CurrentMessage->Msg.message ) ) )
{
msg = CurrentMessage->Msg;
{
idSave = MessageQueue->idSysPeek;
MessageQueue->idSysPeek = (ULONG_PTR)CurrentMessage;
UpdateKeyStateFromMsg(MessageQueue, &msg);
AcceptMessage = co_IntProcessHardwareMessage(&msg, &Remove, MsgFilterLow, MsgFilterHigh);
msg = CurrentMessage->Msg;
QS_Flags = CurrentMessage->QS_Flags;
if (Remove)
{
RemoveEntryList(&CurrentMessage->ListEntry);
ClearMsgBitsMask(pti, CurrentMessage->QS_Flags);
MsqDestroyMessage(CurrentMessage);
}
UpdateKeyStateFromMsg(MessageQueue, &msg);
AcceptMessage = co_IntProcessHardwareMessage(&msg, &Remove, MsgFilterLow, MsgFilterHigh);
if (AcceptMessage)
{
*pMsg = msg;
Ret = TRUE;
break;
}
}
CurrentMessage = CONTAINING_RECORD(CurrentEntry, USER_MESSAGE, ListEntry);
}
while(CurrentEntry != ListHead);
if (Remove)
{
if (CurrentMessage->pti != NULL)
{
RemoveEntryList(&CurrentMessage->ListEntry);
MsqDestroyMessage(CurrentMessage);
}
ClearMsgBitsMask(pti, QS_Flags);
}
MessageQueue->ptiSysLock = NULL;
pti->pcti->CTI_flags &= ~CTI_THREADSYSLOCK;
return Ret;
MessageQueue->idSysPeek = idSave;
if (AcceptMessage)
{
*pMsg = msg;
Ret = TRUE;
break;
}
}
}
MessageQueue->ptiSysLock = NULL;
pti->pcti->CTI_flags &= ~CTI_THREADSYSLOCK;
return Ret;
}
BOOLEAN APIENTRY
@ -1951,23 +2001,19 @@ MsqPeekMessage(IN PTHREADINFO pti,
IN UINT QSflags,
OUT PMSG Message)
{
PLIST_ENTRY CurrentEntry;
PUSER_MESSAGE CurrentMessage;
PLIST_ENTRY ListHead;
DWORD QS_Flags;
BOOL Ret = FALSE;
CurrentEntry = pti->PostedMessagesListHead.Flink;
ListHead = &pti->PostedMessagesListHead;
ListHead = pti->PostedMessagesListHead.Flink;
if (IsListEmpty(CurrentEntry)) return FALSE;
if (IsListEmpty(ListHead)) return FALSE;
CurrentMessage = CONTAINING_RECORD(CurrentEntry, USER_MESSAGE,
ListEntry);
do
while(ListHead != &pti->PostedMessagesListHead)
{
if (IsListEmpty(CurrentEntry)) break;
if (!CurrentMessage) break;
CurrentEntry = CurrentEntry->Flink;
CurrentMessage = CONTAINING_RECORD(ListHead, USER_MESSAGE, ListEntry);
ListHead = ListHead->Flink;
/*
MSDN:
1: any window that belongs to the current thread, and any messages on the current thread's message queue whose hwnd value is NULL.
@ -1981,20 +2027,21 @@ MsqPeekMessage(IN PTHREADINFO pti,
( MsgFilterLow <= CurrentMessage->Msg.message && MsgFilterHigh >= CurrentMessage->Msg.message ) ) )
{
*Message = CurrentMessage->Msg;
QS_Flags = CurrentMessage->QS_Flags;
if (Remove)
{
RemoveEntryList(&CurrentMessage->ListEntry);
ClearMsgBitsMask(pti, CurrentMessage->QS_Flags);
MsqDestroyMessage(CurrentMessage);
if (CurrentMessage->pti != NULL)
{
RemoveEntryList(&CurrentMessage->ListEntry);
MsqDestroyMessage(CurrentMessage);
}
ClearMsgBitsMask(pti, QS_Flags);
}
Ret = TRUE;
break;
}
CurrentMessage = CONTAINING_RECORD(CurrentEntry, USER_MESSAGE,
ListEntry);
}
while (CurrentEntry != ListHead);
return Ret;
}

View file

@ -46,6 +46,8 @@ typedef struct _USER_MESSAGE_QUEUE
struct _DESKTOP *Desktop;
PTHREADINFO ptiSysLock;
ULONG_PTR idSysLock;
ULONG_PTR idSysPeek;
PTHREADINFO ptiMouse;
PTHREADINFO ptiKeyboard;
@ -257,7 +259,7 @@ DWORD APIENTRY IntGetQueueStatus(DWORD);
UINT lParamMemorySize(UINT Msg, WPARAM wParam, LPARAM lParam);
BOOL FASTCALL
BOOL APIENTRY
co_IntGetPeekMessage( PMSG pMsg,
HWND hWnd,
UINT MsgFilterMin,