From fe51dbac3d20ef9275dd1a9070f8185dc7705ad9 Mon Sep 17 00:00:00 2001 From: Maurice Kalinowski Date: Mon, 31 Oct 2016 12:57:38 +0100 Subject: [PATCH] winrt: Fix potential memory corruption The timerInfo list might get accessed concurrently and cause references to become dangling. Hence, we need to protect usages with a mutex. According to tests/benchmark there is no impact on performance. Task-number: QTBUG-56756 Change-Id: I4bdffccff70d2dca99f4a39defad438afe571ada Reviewed-by: Oliver Wolff --- src/corelib/kernel/qeventdispatcher_winrt.cpp | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/src/corelib/kernel/qeventdispatcher_winrt.cpp b/src/corelib/kernel/qeventdispatcher_winrt.cpp index d115a3db2a..4a2e2d887f 100644 --- a/src/corelib/kernel/qeventdispatcher_winrt.cpp +++ b/src/corelib/kernel/qeventdispatcher_winrt.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -100,6 +101,7 @@ public: private: QHash timerIdToObject; QVector timerInfos; + mutable QMutex timerInfoLock; QHash timerHandleToId; QHash timerIdToHandle; QHash timerIdToCancelHandle; @@ -116,6 +118,7 @@ private: timerIdToObject.insert(id, obj); const quint64 targetTime = qt_msectime() + interval; const WinRTTimerInfo info(id, interval, type, obj, targetTime); + QMutexLocker locker(&timerInfoLock); if (id >= timerInfos.size()) timerInfos.resize(id + 1); timerInfos[id] = info; @@ -124,6 +127,7 @@ private: bool removeTimer(int id) { + QMutexLocker locker(&timerInfoLock); if (id >= timerInfos.size()) return false; @@ -247,14 +251,18 @@ bool QEventDispatcherWinRT::processEvents(QEventLoop::ProcessEventsFlags flags) if (timerId == INTERRUPT_HANDLE) break; - WinRTTimerInfo &info = d->timerInfos[timerId]; - Q_ASSERT(info.timerId != INVALID_TIMER_ID); + { + QMutexLocker locker(&d->timerInfoLock); - QCoreApplication::postEvent(this, new QTimerEvent(timerId)); + WinRTTimerInfo &info = d->timerInfos[timerId]; + Q_ASSERT(info.timerId != INVALID_TIMER_ID); - // Update timer's targetTime - const quint64 targetTime = qt_msectime() + info.interval; - info.targetTime = targetTime; + QCoreApplication::postEvent(this, new QTimerEvent(timerId)); + + // Update timer's targetTime + const quint64 targetTime = qt_msectime() + info.interval; + info.targetTime = targetTime; + } waitResult = WaitForMultipleObjectsEx(timerHandles.count(), timerHandles.constData(), FALSE, 0, TRUE); } emit awake(); @@ -421,6 +429,7 @@ QList QEventDispatcherWinRT::registeredTime } Q_D(const QEventDispatcherWinRT); + QMutexLocker locker(&d->timerInfoLock); QList timerInfos; foreach (const WinRTTimerInfo &info, d->timerInfos) { if (info.object == object && info.timerId != INVALID_TIMER_ID) @@ -452,6 +461,7 @@ int QEventDispatcherWinRT::remainingTime(int timerId) } Q_D(QEventDispatcherWinRT); + QMutexLocker locker(&d->timerInfoLock); const WinRTTimerInfo timerInfo = d->timerInfos.at(timerId); if (timerInfo.timerId == INVALID_TIMER_ID) { #ifndef QT_NO_DEBUG @@ -500,6 +510,9 @@ bool QEventDispatcherWinRT::event(QEvent *e) case QEvent::Timer: { QTimerEvent *timerEvent = static_cast(e); const int id = timerEvent->timerId(); + + QMutexLocker locker(&d->timerInfoLock); + Q_ASSERT(id < d->timerInfos.size()); WinRTTimerInfo &info = d->timerInfos[id]; Q_ASSERT(info.timerId != INVALID_TIMER_ID); @@ -508,9 +521,13 @@ bool QEventDispatcherWinRT::event(QEvent *e) break; info.inEvent = true; + locker.unlock(); + QTimerEvent te(id); QCoreApplication::sendEvent(d->timerIdToObject.value(id), &te); + locker.relock(); + // The timer might have been removed in the meanwhile if (id >= d->timerInfos.size()) break;