[pschweitzer] 71528: |SHELL32] Don't blindly delete notification item while there are still ongoing user APC. To do so, we use reference count, and attempt to release in various places: after APC e...

Author: pschweitzer
Date: Sun Jun  5 09:26:00 2016
New Revision: 71528

URL: http://svn.reactos.org/svn/reactos?rev=71528&view=rev
Log:
|SHELL32]
Don't blindly delete notification item while there are still ongoing user APC.
To do so, we use reference count, and attempt to release in various places: after APC ended, and on notification unregistration.
This avoids race condition with item between usage and freeing and thus use-afree-free (leading to explorer crash) while browsing rapidly accross directories.

CORE-10941 #resolve

Modified:
    trunk/reactos/dll/win32/shell32/wine/changenotify.c

Modified: trunk/reactos/dll/win32/shell32/wine/changenotify.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/shell32/wine/chan...
==============================================================================
--- trunk/reactos/dll/win32/shell32/wine/changenotify.c	[iso-8859-1] (original)
+++ trunk/reactos/dll/win32/shell32/wine/changenotify.c	[iso-8859-1] Sun Jun  5 09:26:00 2016
@@ -58,6 +58,7 @@
     OVERLAPPED overlapped; /* Overlapped structure */
     BYTE *buffer; /* Async buffer to fill */
     BYTE *backBuffer; /* Back buffer to swap buffer into */
+    struct _NOTIFICATIONLIST * pParent;
 } SHChangeNotifyEntryInternal, *LPNOTIFYREGISTER;
 #else
 typedef SHChangeNotifyEntry *LPNOTIFYREGISTER;
@@ -74,6 +75,9 @@
    LONG wEventMask;	/* subscribed events */
    DWORD dwFlags;		/* client flags */
    ULONG id;
+#ifdef __REACTOS__
+    volatile LONG wQueuedCount;
+#endif
 } NOTIFICATIONLIST, *LPNOTIFICATIONLIST;

#ifdef __REACTOS__
@@ -156,8 +160,21 @@
 static void DeleteNode(LPNOTIFICATIONLIST item)
 {
     UINT i;
+#ifdef __REACTOS__
+    LONG queued;
+#endif

TRACE("item=%p\n", item);
+
+#ifdef __REACTOS__
+    queued = InterlockedCompareExchange(&item->wQueuedCount, 0, 0);
+    if (queued != 0)
+    {
+        TRACE("Not freeing, still %d queued events\n", queued);
+        return;
+    }
+    TRACE("Freeing for real!\n");
+#endif

/* remove item from list */
     list_remove( &item->entry );
@@ -235,6 +252,7 @@
     item->cidl = cItems;
 #ifdef __REACTOS__
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntryInternal) * cItems);
+    item->wQueuedCount = 0;
 #else
     item->apidl = SHAlloc(sizeof(SHChangeNotifyEntry) * cItems);
 #endif
@@ -249,11 +267,15 @@
         item->apidl[i].buffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].backBuffer = SHAlloc(BUFFER_SIZE);
         item->apidl[i].overlapped.hEvent = &item->apidl[i];
+        item->apidl[i].pParent = item;

if (fSources & SHCNRF_InterruptLevel)
         {
             if (_OpenDirectory( &item->apidl[i] ))
+            {
+                InterlockedIncrement(&item->wQueuedCount);
                 QueueUserAPC( _AddDirectoryProc, m_hThread, (ULONG_PTR) &item->apidl[i] );
+            }
             else
             {
                 CHAR buffer[MAX_PATH];
@@ -714,7 +736,11 @@
                        item->pidl,
                        NULL);

+#ifdef __REACTOS__
+        goto quit;
+#else
         return;
+#endif
     }

/*
@@ -731,12 +757,21 @@
     _BeginRead(item);

_ProcessNotification(item);
+
+#ifdef __REACTOS__
+quit:
+    InterlockedDecrement(&item->pParent->wQueuedCount);
+    DeleteNode(item->pParent);
+#endif
 }

static VOID _BeginRead(LPNOTIFYREGISTER item )
 {
     TRACE("_BeginRead %p \n", item->hDirectory);

+#ifdef __REACTOS__
+    InterlockedIncrement(&item->pParent->wQueuedCount);
+#endif
     /* This call needs to be reissued after every APC. */
     if (!ReadDirectoryChangesW(item->hDirectory, // handle to directory
                                item->buffer, // read results buffer
@@ -746,12 +781,20 @@
                                NULL, // bytes returned
                                &item->overlapped, // overlapped buffer
                                _NotificationCompletion)) // completion routine
+#ifdef __REACTOS__
+    {
+#endif
         ERR("ReadDirectoryChangesW failed. (%p, %p, %p, %p) Code: %u \n",
             item->hDirectory,
             item->buffer,
             &item->overlapped,
             _NotificationCompletion,
             GetLastError());
+#ifdef __REACTOS__
+        InterlockedDecrement(&item->pParent->wQueuedCount);
+        DeleteNode(item->pParent);
+    }
+#endif
 }

DWORD _MapAction(DWORD dwAction, BOOL isDir)