[ros-diffs] [gadamopoulos] 47058: [win32k] - Do not do callbacks to user mode while holding a lock - Fix when WH_MOUSE_LL and WH_JOURNALRECORD are called See issue #4926 for more details.

gadamopoulos at svn.reactos.org gadamopoulos at svn.reactos.org
Thu Apr 29 09:48:43 CEST 2010


Author: gadamopoulos
Date: Thu Apr 29 09:48:43 2010
New Revision: 47058

URL: http://svn.reactos.org/svn/reactos?rev=47058&view=rev
Log:
[win32k]
- Do not do callbacks to user mode while holding a lock
- Fix when WH_MOUSE_LL and WH_JOURNALRECORD are called
See issue #4926 for more details.

Modified:
    trunk/reactos/subsystems/win32/win32k/include/msgqueue.h
    trunk/reactos/subsystems/win32/win32k/ntuser/input.c
    trunk/reactos/subsystems/win32/win32k/ntuser/message.c
    trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c

Modified: trunk/reactos/subsystems/win32/win32k/include/msgqueue.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/include/msgqueue.h?rev=47058&r1=47057&r2=47058&view=diff
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/include/msgqueue.h [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/include/msgqueue.h [iso-8859-1] Thu Apr 29 09:48:43 2010
@@ -248,6 +248,16 @@
    (message) == WM_NCRBUTTON##code || \
    (message) == WM_NCXBUTTON##code )
 
+#define WM_NCMOUSEFIRST WM_NCMOUSEMOVE
+#define WM_NCMOUSELAST  (WM_NCMOUSEFIRST+(WM_MOUSELAST-WM_MOUSEFIRST))
+
+#define IS_MOUSE_MESSAGE(message) \
+    ((message >= WM_NCMOUSEFIRST && message <= WM_NCMOUSELAST) || \
+            (message >= WM_MOUSEFIRST && message <= WM_MOUSELAST))
+
+#define IS_KBD_MESSAGE(message) \
+    (message == WM_KEYDOWN || message == WM_KEYUP)
+
 HANDLE FASTCALL
 IntMsqSetWakeMask(DWORD WakeMask);
 

Modified: trunk/reactos/subsystems/win32/win32k/ntuser/input.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntuser/input.c?rev=47058&r1=47057&r2=47058&view=diff
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/ntuser/input.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/ntuser/input.c [iso-8859-1] Thu Apr 29 09:48:43 2010
@@ -203,6 +203,16 @@
    IO_STATUS_BLOCK Iosb;
    NTSTATUS Status;
    MOUSE_ATTRIBUTES MouseAttr;
+
+   Status = Win32kInitWin32Thread(PsGetCurrentThread());
+   if (!NT_SUCCESS(Status))
+   {
+      DPRINT1("Win32K: Failed making keyboard thread a win32 thread.\n");
+      return; //(Status);
+   }
+
+   KeSetPriorityThread(&PsGetCurrentThread()->Tcb,
+                       LOW_REALTIME_PRIORITY + 3);
 
    InitializeObjectAttributes(&MouseObjectAttributes,
                               &MouseDeviceName,

Modified: trunk/reactos/subsystems/win32/win32k/ntuser/message.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntuser/message.c?rev=47058&r1=47057&r2=47058&view=diff
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/ntuser/message.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/ntuser/message.c [iso-8859-1] Thu Apr 29 09:48:43 2010
@@ -645,6 +645,74 @@
    return FALSE;
 }
 
+BOOL ProcessMouseMessage(MSG* Msg, USHORT HitTest, UINT RemoveMsg)
+{
+    MOUSEHOOKSTRUCT MHook;
+    EVENTMSG Event;
+
+    Event.message = Msg->message;
+    Event.time    = Msg->time;
+    Event.hwnd    = Msg->hwnd;
+    Event.paramL  = Msg->pt.x;
+    Event.paramH  = Msg->pt.y;
+    co_HOOK_CallHooks( WH_JOURNALRECORD, HC_ACTION, 0, (LPARAM)&Event);
+
+
+    MHook.pt           = Msg->pt;
+    MHook.hwnd         = Msg->hwnd;
+    MHook.wHitTestCode = HitTest;
+    MHook.dwExtraInfo  = 0;
+    if (co_HOOK_CallHooks( WH_MOUSE,
+                           RemoveMsg ? HC_ACTION : HC_NOREMOVE,
+                           Msg->message,
+                           (LPARAM)&MHook ))
+    {
+        if (ISITHOOKED(WH_CBT))
+        {
+            MHook.pt           = Msg->pt;
+            MHook.hwnd         = Msg->hwnd;
+            MHook.wHitTestCode = HitTest;
+            MHook.dwExtraInfo  = 0;
+            co_HOOK_CallHooks( WH_CBT,
+                               HCBT_CLICKSKIPPED,
+                               Msg->message,
+                               (LPARAM)&MHook);
+        }
+        return FALSE;
+    }
+
+	return TRUE;
+}
+
+BOOL ProcessKeyboardMessage(MSG* Msg, UINT RemoveMsg)
+{
+   EVENTMSG Event;
+
+   Event.message = Msg->message;
+   Event.hwnd    = Msg->hwnd;
+   Event.time    = Msg->time;
+   Event.paramL  = (Msg->wParam & 0xFF) | (HIWORD(Msg->lParam) << 8);
+   Event.paramH  = Msg->lParam & 0x7FFF;
+   if (HIWORD(Msg->lParam) & 0x0100) Event.paramH |= 0x8000;
+   co_HOOK_CallHooks( WH_JOURNALRECORD, HC_ACTION, 0, (LPARAM)&Event);
+
+    if (co_HOOK_CallHooks( WH_KEYBOARD,
+                           RemoveMsg ? HC_ACTION : HC_NOREMOVE,
+                           LOWORD(Msg->wParam),
+                           Msg->lParam))
+    {
+        if (ISITHOOKED(WH_CBT))
+        {
+            /* skip this message */
+            co_HOOK_CallHooks( WH_CBT,
+                               HCBT_KEYSKIPPED,
+                               LOWORD(Msg->wParam),
+                               Msg->lParam );
+        }
+        return FALSE;
+    }
+	return TRUE;
+}
 /*
  * Internal version of PeekMessage() doing all the work
  */
@@ -662,7 +730,6 @@
    BOOL Present, RemoveMessages;
    USER_REFERENCE_ENTRY Ref;
    USHORT HitTest;
-   MOUSEHOOKSTRUCT MHook;
 
    /* The queues and order in which they are checked are documented in the MSDN
       article on GetMessage() */
@@ -867,52 +934,20 @@
       }
 
 MsgExit:
-      if ( ISITHOOKED(WH_MOUSE) &&
-           Msg->Msg.message >= WM_MOUSEFIRST &&
-           Msg->Msg.message <= WM_MOUSELAST )
-      {
-         MHook.pt           = Msg->Msg.pt;
-         MHook.hwnd         = Msg->Msg.hwnd;
-         MHook.wHitTestCode = HitTest;
-         MHook.dwExtraInfo  = 0;
-         if (co_HOOK_CallHooks( WH_MOUSE,
-                                RemoveMsg ? HC_ACTION : HC_NOREMOVE,
-                                Msg->Msg.message,
-                                (LPARAM)&MHook ))
-         {
-            if (ISITHOOKED(WH_CBT))
-            {
-                MHook.pt           = Msg->Msg.pt;
-                MHook.hwnd         = Msg->Msg.hwnd;
-                MHook.wHitTestCode = HitTest;
-                MHook.dwExtraInfo  = 0;
-                co_HOOK_CallHooks( WH_CBT,
-                                   HCBT_CLICKSKIPPED,
-                                   Msg->Msg.message,
-                                  (LPARAM)&MHook);
-            }
-            return FALSE;
-         }
-      }
-
-      if ( ISITHOOKED(WH_KEYBOARD) &&
-          (Msg->Msg.message == WM_KEYDOWN || Msg->Msg.message == WM_KEYUP) )
-      {
-         if (co_HOOK_CallHooks( WH_KEYBOARD,
-                                RemoveMsg ? HC_ACTION : HC_NOREMOVE,
-                                LOWORD(Msg->Msg.wParam),
-                                Msg->Msg.lParam))
-         {
-            if (ISITHOOKED(WH_CBT))
-            {
-               /* skip this message */
-               co_HOOK_CallHooks( WH_CBT,
-                                  HCBT_KEYSKIPPED,
-                                  LOWORD(Msg->Msg.wParam),
-                                  Msg->Msg.lParam );
-            }
-            return FALSE;
-         }
+      if ( ISITHOOKED(WH_MOUSE) && IS_MOUSE_MESSAGE(Msg->Msg.message))
+      {
+          if(!ProcessMouseMessage(&Msg->Msg, HitTest, RemoveMsg))
+		  {
+			  return FALSE;
+		  }
+	  }
+
+      if ( ISITHOOKED(WH_KEYBOARD) && IS_KBD_MESSAGE(Msg->Msg.message))
+      {
+          if(!ProcessKeyboardMessage(&Msg->Msg, RemoveMsg))
+          {
+              return FALSE;
+          }
       }
       // The WH_GETMESSAGE hook enables an application to monitor messages about to
       // be returned by the GetMessage or PeekMessage function.

Modified: trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c?rev=47058&r1=47057&r2=47058&view=diff
==============================================================================
--- trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c [iso-8859-1] (original)
+++ trunk/reactos/subsystems/win32/win32k/ntuser/msgqueue.c [iso-8859-1] Thu Apr 29 09:48:43 2010
@@ -174,36 +174,53 @@
    LARGE_INTEGER LargeTickCount;
    KIRQL OldIrql;
    ULONG Prev;
-   EVENTMSG Event;
-
-   IntLockSystemMessageQueue(OldIrql);
-
-   /*
-    * Bail out if the queue is full. FIXME: We should handle this case
-    * more gracefully.
-    */
-
-   if (SystemMessageQueueCount == SYSTEM_MESSAGE_QUEUE_SIZE)
-   {
-      IntUnLockSystemMessageQueue(OldIrql);
-      return;
-   }
+   MSLLHOOKSTRUCT MouseHookData;
 
    KeQueryTickCount(&LargeTickCount);
    Msg->time = MsqCalculateMessageTime(&LargeTickCount);
 
-   Event.message = Msg->message;
-   Event.time    = Msg->time;
-   Event.hwnd    = Msg->hwnd;
-   Event.paramL  = Msg->pt.x;
-   Event.paramH  = Msg->pt.y;
-   co_HOOK_CallHooks( WH_JOURNALRECORD, HC_ACTION, 0, (LPARAM)&Event);
+   MouseHookData.pt.x = LOWORD(Msg->lParam);
+   MouseHookData.pt.y = HIWORD(Msg->lParam);
+   switch(Msg->message)
+   {
+        case WM_MOUSEWHEEL:
+           MouseHookData.mouseData = MAKELONG(0, GET_WHEEL_DELTA_WPARAM(Msg->wParam));
+           break;
+        case WM_XBUTTONDOWN:
+        case WM_XBUTTONUP:
+        case WM_XBUTTONDBLCLK:
+        case WM_NCXBUTTONDOWN:
+        case WM_NCXBUTTONUP:
+        case WM_NCXBUTTONDBLCLK:
+           MouseHookData.mouseData = MAKELONG(0, HIWORD(Msg->wParam));
+           break;
+        default:
+           MouseHookData.mouseData = 0;
+           break;
+     }
+     MouseHookData.flags = 0;
+     MouseHookData.time = Msg->time;
+     MouseHookData.dwExtraInfo = 0;
+     if( co_HOOK_CallHooks(WH_MOUSE_LL, HC_ACTION, Msg->message, (LPARAM) &MouseHookData))
+         return;
 
    /*
     * If we got WM_MOUSEMOVE and there are already messages in the
     * system message queue, check if the last message is mouse move
     * and if it is then just overwrite it.
     */
+   IntLockSystemMessageQueue(OldIrql);
+
+   /*
+    * Bail out if the queue is full. FIXME: We should handle this case
+    * more gracefully.
+    */
+
+   if (SystemMessageQueueCount == SYSTEM_MESSAGE_QUEUE_SIZE)
+   {
+      IntUnLockSystemMessageQueue(OldIrql);
+      return;
+   }
 
    if (Msg->message == WM_MOUSEMOVE && SystemMessageQueueCount)
    {
@@ -623,7 +640,6 @@
    {
       PUSER_MESSAGE UserMsg;
       MSG Msg;
-      BOOL ProcessMessage;
 
       ASSERT(SystemMessageQueueHead < SYSTEM_MESSAGE_QUEUE_SIZE);
       Msg = SystemMessageQueue[SystemMessageQueueHead];
@@ -631,48 +647,14 @@
          (SystemMessageQueueHead + 1) % SYSTEM_MESSAGE_QUEUE_SIZE;
       SystemMessageQueueCount--;
       IntUnLockSystemMessageQueue(OldIrql);
-      if (WM_MOUSEFIRST <= Msg.message && Msg.message <= WM_MOUSELAST)
-      {
-         MSLLHOOKSTRUCT MouseHookData;
-
-         MouseHookData.pt.x = LOWORD(Msg.lParam);
-         MouseHookData.pt.y = HIWORD(Msg.lParam);
-         switch(Msg.message)
-         {
-            case WM_MOUSEWHEEL:
-               MouseHookData.mouseData = MAKELONG(0, GET_WHEEL_DELTA_WPARAM(Msg.wParam));
-               break;
-            case WM_XBUTTONDOWN:
-            case WM_XBUTTONUP:
-            case WM_XBUTTONDBLCLK:
-            case WM_NCXBUTTONDOWN:
-            case WM_NCXBUTTONUP:
-            case WM_NCXBUTTONDBLCLK:
-               MouseHookData.mouseData = MAKELONG(0, HIWORD(Msg.wParam));
-               break;
-            default:
-               MouseHookData.mouseData = 0;
-               break;
-         }
-         MouseHookData.flags = 0;
-         MouseHookData.time = Msg.time;
-         MouseHookData.dwExtraInfo = 0;
-         ProcessMessage = (0 == co_HOOK_CallHooks(WH_MOUSE_LL, HC_ACTION,
-                           Msg.message, (LPARAM) &MouseHookData));
-      }
-      else
-      {
-         ProcessMessage = TRUE;
-      }
-      if (ProcessMessage)
-      {
-         UserMsg = ExAllocateFromPagedLookasideList(&MessageLookasideList);
-         /* What to do if out of memory? For now we just panic a bit in debug */
-         ASSERT(UserMsg);
-         UserMsg->FreeLParam = FALSE;
-         UserMsg->Msg = Msg;
-         InsertTailList(&HardwareMessageQueueHead, &UserMsg->ListEntry);
-      }
+
+      UserMsg = ExAllocateFromPagedLookasideList(&MessageLookasideList);
+      /* What to do if out of memory? For now we just panic a bit in debug */
+      ASSERT(UserMsg);
+      UserMsg->FreeLParam = FALSE;
+      UserMsg->Msg = Msg;
+      InsertTailList(&HardwareMessageQueueHead, &UserMsg->ListEntry);
+
       IntLockSystemMessageQueue(OldIrql);
    }
    HardwareMessageQueueStamp++;
@@ -767,7 +749,6 @@
    MSG Msg;
    LARGE_INTEGER LargeTickCount;
    KBDLLHOOKSTRUCT KbdHookData;
-   EVENTMSG Event;
    BOOLEAN Entered = FALSE;
 
    DPRINT("MsqPostKeyboardMessage(uMsg 0x%x, wParam 0x%x, lParam 0x%x)\n",
@@ -794,14 +775,6 @@
 
    KeQueryTickCount(&LargeTickCount);
    Msg.time = MsqCalculateMessageTime(&LargeTickCount);
-
-   Event.message = Msg.message;
-   Event.hwnd    = Msg.hwnd;
-   Event.time    = Msg.time;
-   Event.paramL  = (Msg.wParam & 0xFF) | (HIWORD(Msg.lParam) << 8);
-   Event.paramH  = Msg.lParam & 0x7FFF;
-   if (HIWORD(Msg.lParam) & 0x0100) Event.paramH |= 0x8000;
-   co_HOOK_CallHooks( WH_JOURNALRECORD, HC_ACTION, 0, (LPARAM)&Event);
 
    /* We can't get the Msg.pt point here since we don't know thread
       (and thus the window station) the message will end up in yet. */




More information about the Ros-diffs mailing list