[jimtabor] 65744: [Win32k] - Crash is due to receiving an APC for the current sending thread. Prematurely freeing the data block and while the receiving thread did not have a chance to process it. ...

Author: jimtabor
Date: Thu Dec 18 23:45:11 2014
New Revision: 65744

URL: http://svn.reactos.org/svn/reactos?rev=65744&view=rev
Log:
[Win32k]
- Crash is due to receiving an APC for the current sending thread. Prematurely freeing the
data block and while the receiving thread did not have a chance to process it.
Interception and calling back to user mode will allow thread to go away. What about the
memory block, is it still allocated? A cleaver Hack can fix this, but still the
application crashed leaving issues and trash laying about. See CORE-8779.

Reference:
http://msdn.microsoft.com/en-us/library/windows/hardware/ff565592(v=vs.85).…
"Yes, for thread termination. No, for user APCs."

Modified:
    trunk/reactos/win32ss/include/callback.h
    trunk/reactos/win32ss/user/ntuser/callback.c
    trunk/reactos/win32ss/user/ntuser/callback.h
    trunk/reactos/win32ss/user/ntuser/msgqueue.c
    trunk/reactos/win32ss/user/user32/misc/dllmain.c

Modified: trunk/reactos/win32ss/include/callback.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/include/callback.h…
==============================================================================
--- trunk/reactos/win32ss/include/callback.h	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/include/callback.h	[iso-8859-1] Thu Dec 18 23:45:11 2014
@@ -13,7 +13,8 @@
 #define USER32_CALLBACK_GETCHARSETINFO        (9)
 #define USER32_CALLBACK_COPYIMAGE             (10)
 #define USER32_CALLBACK_SETWNDICONS           (11)
-#define USER32_CALLBACK_MAXIMUM               (11)
+#define USER32_CALLBACK_DELIVERUSERAPC        (12)
+#define USER32_CALLBACK_MAXIMUM               (12)
 
 typedef struct _WINDOWPROC_CALLBACK_ARGUMENTS
 {
@@ -138,4 +139,6 @@
 User32CallClientLoadLibraryFromKernel(PVOID Arguments, ULONG ArgumentLength);
 NTSTATUS WINAPI
 User32CallGetCharsetInfo(PVOID Arguments, ULONG ArgumentLength);
+NTSTATUS WINAPI
+User32DeliverUserAPC(PVOID Arguments, ULONG ArgumentLength);
 #endif /* __INCLUDE_USER32_CALLBACK_H */

Modified: trunk/reactos/win32ss/user/ntuser/callback.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/callba…
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/callback.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/callback.c	[iso-8859-1] Thu Dec 18 23:45:11 2014
@@ -1100,4 +1100,24 @@
    return TRUE;
 }
 
+VOID FASTCALL
+co_IntDeliverUserAPC(VOID)
+{
+   NTSTATUS Status;
+   UserLeaveCo();
+
+   Status = KeUserModeCallback(USER32_CALLBACK_DELIVERUSERAPC,
+                               0,
+                               0,
+                               NULL,
+                               NULL);
+
+
+   UserEnterCo();
+
+   if (!NT_SUCCESS(Status))
+   {
+      ERR("Delivering User APC callback failed!\n");
+   }   
+}
 /* EOF */

Modified: trunk/reactos/win32ss/user/ntuser/callback.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/callba…
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/callback.h	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/callback.h	[iso-8859-1] Thu Dec 18 23:45:11 2014
@@ -71,3 +71,4 @@
 HANDLE FASTCALL co_IntCopyImage(HANDLE,UINT,INT,INT,UINT);
 
 BOOL FASTCALL co_IntSetWndIcons(VOID);
+VOID FASTCALL co_IntDeliverUserAPC(VOID);

Modified: trunk/reactos/win32ss/user/ntuser/msgqueue.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/msgque…
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/msgqueue.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/msgqueue.c	[iso-8859-1] Thu Dec 18 23:45:11 2014
@@ -1132,17 +1132,22 @@
 
    /* We can't access the Message anymore since it could have already been deleted!
*/
 
-   if(Block)
-   {
+   if (Block)
+   {
+      PVOID WaitObjects[2];
+
+      WaitObjects[0] = &CompletionEvent;       // Wait 0
+      WaitObjects[1] = ptirec->pEThread;       // Wait 1
+
       UserLeaveCo();
 
       /* Don't process messages sent to the thread */
-      WaitStatus = KeWaitForSingleObject(&CompletionEvent, UserRequest, UserMode,
-                                         FALSE, (uTimeout ? &Timeout : NULL));
+      WaitStatus = KeWaitForMultipleObjects(2, WaitObjects, WaitAny, UserRequest,
+                                            UserMode, FALSE, (uTimeout ? &Timeout :
NULL), NULL);
 
       UserEnterCo();
 
-      if(WaitStatus == STATUS_TIMEOUT)
+      if (WaitStatus == STATUS_TIMEOUT || WaitStatus == STATUS_USER_APC)
       {
          /* Look up if the message has not yet dispatched, if so
             make sure it can't pass a result and it must not set the completion event
anymore */
@@ -1182,7 +1187,27 @@
             Entry = Entry->Flink;
          }
 
-         TRACE("MsqSendMessage (blocked) timed out 1\n");
+         TRACE("MsqSendMessage (blocked) timed out 1 Status %p\n",WaitStatus);
+
+       }
+      // Receiving thread passed on and left us hanging with issues still pending.
+      if ( WaitStatus == STATUS_WAIT_1 )
+      {
+         ERR("Bk Receiving Thread woken up dead!\n");
+         Entry = pti->DispatchingMessagesHead.Flink;
+         while (Entry != &pti->DispatchingMessagesHead)
+         {
+            if ((PUSER_SENT_MESSAGE) CONTAINING_RECORD(Entry, USER_SENT_MESSAGE,
DispatchingListEntry)
+                  == Message)
+            {
+               Message->CompletionEvent = NULL;
+               Message->Result = NULL;
+               RemoveEntryList(&Message->DispatchingListEntry);
+               Message->DispatchingListEntry.Flink = NULL;
+               break;
+            }
+            Entry = Entry->Flink;
+         }
       }
       while (co_MsqDispatchOneSentMessage(pti))
          ;
@@ -1204,7 +1229,7 @@
 
          UserEnterCo();
 
-         if(WaitStatus == STATUS_TIMEOUT)
+         if (WaitStatus == STATUS_TIMEOUT || WaitStatus == STATUS_USER_APC)
          {
             /* Look up if the message has not yet been dispatched, if so
                make sure it can't pass a result and it must not set the completion
event anymore */
@@ -1244,13 +1269,14 @@
                Entry = Entry->Flink;
             }
 
-            TRACE("MsqSendMessage timed out 2\n");
+            TRACE("MsqSendMessage timed out 2 Status %p\n",WaitStatus);
+ 
             break;
          }
          // Receiving thread passed on and left us hanging with issues still pending.
          if ( WaitStatus == STATUS_WAIT_2 )
          {
-            ERR("Receiving Thread woken up dead!\n");
+            ERR("NB Receiving Thread woken up dead!\n");
             Entry = pti->DispatchingMessagesHead.Flink;
             while (Entry != &pti->DispatchingMessagesHead)
             {
@@ -1272,8 +1298,21 @@
       while (NT_SUCCESS(WaitStatus) && WaitStatus == STATUS_WAIT_1);
    }
 
-   if(WaitStatus != STATUS_TIMEOUT)
-      if (uResult) *uResult = (STATUS_WAIT_0 == WaitStatus ? Result : -1);
+   if ( WaitStatus == STATUS_USER_APC )
+   {
+     // The current thread is dying!
+     TRACE("User APC\n");
+     co_IntDeliverUserAPC();
+     ERR("User APC Returned\n"); // Should not see this message.
+   }
+
+   if (WaitStatus != STATUS_TIMEOUT)
+   {
+      if (uResult)
+      {
+         *uResult = (STATUS_WAIT_0 == WaitStatus ? Result : -1);
+      }
+   }
 
    return WaitStatus;
 }
@@ -1972,6 +2011,11 @@
                                 FALSE,
                                 NULL );
    UserEnterCo();
+   if ( ret == STATUS_USER_APC )
+   {
+      TRACE("MWFNW User APC\n");
+      co_IntDeliverUserAPC();
+   }
    return ret;
 }
 

Modified: trunk/reactos/win32ss/user/user32/misc/dllmain.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/user32/misc/d…
==============================================================================
--- trunk/reactos/win32ss/user/user32/misc/dllmain.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/user32/misc/dllmain.c	[iso-8859-1] Thu Dec 18 23:45:11
2014
@@ -200,6 +200,7 @@
     User32CallGetCharsetInfo,
     User32CallCopyImageFromKernel,
     User32CallSetWndIconsFromKernel,
+    User32DeliverUserAPC,
 };
 
 /*
@@ -468,3 +469,10 @@
   ERR("hIconSmWindows %p hIconWindows %p \n",hIconSmWindows,hIconWindows);
   return ZwCallbackReturn(Arguments, ArgumentLength, STATUS_SUCCESS);
 }
+
+NTSTATUS
+WINAPI
+User32DeliverUserAPC(PVOID Arguments, ULONG ArgumentLength)
+{
+  return ZwCallbackReturn(0, 0, STATUS_SUCCESS);
+}