From 11becbe910f806570ca9aa6a131b5887303e2a00 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Tue, 4 Jan 2022 18:03:22 +0100 Subject: [PATCH] QTzTimeZonePrivate: fix UB (data race on m_icu) The fallback m_icu QIcuTimeZonePrivate is lazily constructed, which means that two threads each with their own copy of a QTimeZone with a shared QTzTimeZonePrivate will race over who gets to set m_icu, e.g. when concurrently calling QTimeZone::displayName(). Fix by protecting m_icu with a mutex. For simplicity, use a static mutex, not a per-instance one (which would delete the QTzTimeZonePrivate copy constructor, which clone() relies on). This is sufficient for 5.15. For Qt 6, going forward, we could make this lock-less, too. [ChangeLog][QtCore][QTimeZone] Fixed a data race on Unix platforms when implicitly-shared copies of QTimeZone objects were used in certain ways (e.g. calling displayName()) from different threads and Qt was configured with ICU support. Pick-to: 6.3 6.2 5.15 Change-Id: I7e57aef3dd44a90289ad86d0578ece1e54920730 Reviewed-by: Qt CI Bot Reviewed-by: Lars Knoll Reviewed-by: Edward Welbourne --- src/corelib/time/qtimezoneprivate_tz.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/corelib/time/qtimezoneprivate_tz.cpp b/src/corelib/time/qtimezoneprivate_tz.cpp index b23ccfb3b2..8dfd88b281 100644 --- a/src/corelib/time/qtimezoneprivate_tz.cpp +++ b/src/corelib/time/qtimezoneprivate_tz.cpp @@ -42,6 +42,7 @@ #include "qtimezone.h" #include "qtimezoneprivate_p.h" #include "private/qlocale_tools_p.h" +#include "private/qlocking_p.h" #include #include @@ -62,6 +63,10 @@ QT_BEGIN_NAMESPACE +#if QT_CONFIG(icu) +static QBasicMutex s_icu_mutex; +#endif + /* Private @@ -729,6 +734,9 @@ QTzTimeZonePrivate::~QTzTimeZonePrivate() QTzTimeZonePrivate *QTzTimeZonePrivate::clone() const { +#if QT_CONFIG(icu) + const auto lock = qt_scoped_lock(s_icu_mutex); +#endif return new QTzTimeZonePrivate(*this); } @@ -989,12 +997,14 @@ QString QTzTimeZonePrivate::displayName(qint64 atMSecsSinceEpoch, const QLocale &locale) const { #if QT_CONFIG(icu) + auto lock = qt_unique_lock(s_icu_mutex); if (!m_icu) m_icu = new QIcuTimeZonePrivate(m_id); // TODO small risk may not match if tran times differ due to outdated files // TODO Some valid TZ names are not valid ICU names, use translation table? if (m_icu->isValid()) return m_icu->displayName(atMSecsSinceEpoch, nameType, locale); + lock.unlock(); #else Q_UNUSED(nameType); Q_UNUSED(locale); @@ -1008,12 +1018,14 @@ QString QTzTimeZonePrivate::displayName(QTimeZone::TimeType timeType, const QLocale &locale) const { #if QT_CONFIG(icu) + auto lock = qt_unique_lock(s_icu_mutex); if (!m_icu) m_icu = new QIcuTimeZonePrivate(m_id); // TODO small risk may not match if tran times differ due to outdated files // TODO Some valid TZ names are not valid ICU names, use translation table? if (m_icu->isValid()) return m_icu->displayName(timeType, nameType, locale); + lock.unlock(); #else Q_UNUSED(timeType); Q_UNUSED(nameType);