|SHELL32]
authorPierre Schweitzer <pierre@reactos.org>
Sun, 5 Jun 2016 09:26:00 +0000 (09:26 +0000)
committerPierre Schweitzer <pierre@reactos.org>
Sun, 5 Jun 2016 09:26:00 +0000 (09:26 +0000)
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

svn path=/trunk/; revision=71528

reactos/dll/win32/shell32/wine/changenotify.c

index c1483b0..9369379 100644 (file)
@@ -58,6 +58,7 @@ typedef struct {
     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 @@ typedef struct _NOTIFICATIONLIST
        LONG wEventMask;        /* subscribed events */
        DWORD dwFlags;          /* client flags */
        ULONG id;
+#ifdef __REACTOS__
+    volatile LONG wQueuedCount;
+#endif
 } NOTIFICATIONLIST, *LPNOTIFICATIONLIST;
 
 #ifdef __REACTOS__
@@ -156,9 +160,22 @@ static const char * NodeName(const NOTIFICATIONLIST *item)
 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 @@ SHChangeNotifyRegister(
     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 @@ SHChangeNotifyRegister(
         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 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
                        item->pidl,
                        NULL);
 
+#ifdef __REACTOS__
+        goto quit;
+#else
         return;
+#endif
     }
 
     /*
@@ -731,12 +757,21 @@ _NotificationCompletion(DWORD dwErrorCode, // completion code
     _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 @@ static VOID _BeginRead(LPNOTIFYREGISTER item )
                                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)