|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

svn path=/trunk/; revision=71528
This commit is contained in:
Pierre Schweitzer 2016-06-05 09:26:00 +00:00
parent 326b3d7e91
commit 4322caeede

View 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)