From b9750a98ff46c21f12c13a59489d0c8f4f1b646a Mon Sep 17 00:00:00 2001 From: Evgen Pervenenko Date: Mon, 4 Nov 2024 19:12:32 +0300 Subject: [PATCH] QNetworkConnectionMonitor[Mac]: Fix potential use-after/during-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is a potential race condition on Mac that can lead to crashes when destroying `QNetworkConnectionMonitor`. This issue arises when `QNetworkConnectionMonitorPrivate` has already been destroyed, but is still being accessed through `QNCMP::probeCallback()` from another thread. To fix the issue, a reference counter is used. This counter indicates whether the callback is being used in monitoring. It increases when information is retained (when we set the callback, the info retains to 1) and decreases when information is released (the info releases to 0, when QNCMP object is not used in monitoring). A waitCondition is used to notify all threads when the reference counter reaches zero. The QNCMP::reset function waits until the probeCallback is disconnected and the reference counter is zero. This indicates that the info resource is free, allowing us to safely destroy the QNCMP object. The conditional variable protects the QNetworkConnectionMonitorPrivate object and ensures that the callback is disconnected properly. Fixes: QTBUG-130552 Pick-to: 6.5 Change-Id: I1259c429e92bc20c382604192243d6d8fadb5c25 Reviewed-by: MÃ¥rten Nordheim Reviewed-by: Vladimir Belyavsky (cherry picked from commit 8a535cc1049c4304754b380daaefdece3a7c2e5b) Reviewed-by: Qt Cherry-pick Bot --- src/network/kernel/qnetconmonitor_darwin.mm | 46 +++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/network/kernel/qnetconmonitor_darwin.mm b/src/network/kernel/qnetconmonitor_darwin.mm index b94cbabab1a..0bb4329cac7 100644 --- a/src/network/kernel/qnetconmonitor_darwin.mm +++ b/src/network/kernel/qnetconmonitor_darwin.mm @@ -11,6 +11,9 @@ #include +#include +#include + #include QT_BEGIN_NAMESPACE @@ -85,6 +88,10 @@ public: SCNetworkReachabilityFlags state = kSCNetworkReachabilityFlagsIsLocalAddress; bool scheduled = false; + QWaitCondition refCounterWaitCondition; + QMutex refCounterMutex; + quint32 refCounter = 0; + void updateState(SCNetworkReachabilityFlags newState); void reset(); bool isReachable() const; @@ -92,7 +99,30 @@ public: bool isWwan() const; #endif + void retain() + { + QMutexLocker locker(&refCounterMutex); + ++refCounter; + } + + void release() + { + QMutexLocker locker(&refCounterMutex); + if (--refCounter == 0) + refCounterWaitCondition.wakeAll(); + } + + void waitForRefCountZero() + { + QMutexLocker locker(&refCounterMutex); + while (refCounter > 0) { + refCounterWaitCondition.wait(&refCounterMutex); + } + } + static void probeCallback(SCNetworkReachabilityRef probe, SCNetworkReachabilityFlags flags, void *info); + static const void *retainInfo(const void* info); + static void releaseInfo(const void* info); Q_DECLARE_PUBLIC(QNetworkConnectionMonitor) }; @@ -130,6 +160,7 @@ void QNetworkConnectionMonitorPrivate::reset() state = kSCNetworkReachabilityFlagsIsLocalAddress; scheduled = false; + waitForRefCountZero(); } bool QNetworkConnectionMonitorPrivate::isReachable() const @@ -154,6 +185,19 @@ void QNetworkConnectionMonitorPrivate::probeCallback(SCNetworkReachabilityRef pr monitorPrivate->updateState(flags); } +const void *QNetworkConnectionMonitorPrivate::retainInfo(const void *info) +{ + auto monitorPrivate = static_cast(const_cast(info)); + monitorPrivate->retain(); + return info; +} + +void QNetworkConnectionMonitorPrivate::releaseInfo(const void *info) +{ + auto monitorPrivate = static_cast(const_cast(info)); + monitorPrivate->release(); +} + QNetworkConnectionMonitor::QNetworkConnectionMonitor() : QObject(*new QNetworkConnectionMonitorPrivate) { @@ -235,6 +279,8 @@ bool QNetworkConnectionMonitor::startMonitoring() SCNetworkReachabilityContext context = {}; context.info = d; + context.retain = QNetworkConnectionMonitorPrivate::retainInfo; + context.release = QNetworkConnectionMonitorPrivate::releaseInfo; if (!SCNetworkReachabilitySetCallback(d->probe, QNetworkConnectionMonitorPrivate::probeCallback, &context)) { qCWarning(lcNetMon, "Failed to set a reachability callback"); return false;