From 536471106d47bb99680f8e0dbb448c9671914309 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 15 Nov 2021 13:06:46 +0100 Subject: [PATCH] QAbstractFileEngine: fix UB (data race) on qt_file_engine_handlers_in_use While all writers of the variable hold fileEngineHandlerMutex for writing, the qt_custom_file_engine_handler_create() function checks the value before entering a fileEngineHandlerMutex read-side critical section, thereby causing a C++11 data race. Fix by making the variable atomic. Interestingly enough, relaxed atomic operations suffice here, since the actual synchronization happens in read- and write-side critical sections, and if qt_file_engine_handlers_in_use is wrong w.r.t. to the actual list, the critical sections will still work. We just mustn't cause UB by reading and writing to a simple bool without proper synchronization. Pick-to: 6.2 5.15 Change-Id: I30469504cdbc90e2ab27125181e53d74305f13fd Reviewed-by: Thiago Macieira Reviewed-by: Fabian Kosmale --- src/corelib/io/qabstractfileengine.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/corelib/io/qabstractfileengine.cpp b/src/corelib/io/qabstractfileengine.cpp index 4cf3906a16..9b2690ca15 100644 --- a/src/corelib/io/qabstractfileengine.cpp +++ b/src/corelib/io/qabstractfileengine.cpp @@ -101,7 +101,7 @@ QT_BEGIN_NAMESPACE \sa QAbstractFileEngine, QAbstractFileEngine::create() */ -static bool qt_file_engine_handlers_in_use = false; +static QBasicAtomicInt qt_file_engine_handlers_in_use = Q_BASIC_ATOMIC_INITIALIZER(false); /* All application-wide handlers are stored in this list. The mutex must be @@ -132,7 +132,7 @@ Q_GLOBAL_STATIC(QAbstractFileEngineHandlerList, fileEngineHandlers) QAbstractFileEngineHandler::QAbstractFileEngineHandler() { QWriteLocker locker(fileEngineHandlerMutex()); - qt_file_engine_handlers_in_use = true; + qt_file_engine_handlers_in_use.storeRelaxed(true); fileEngineHandlers()->prepend(this); } @@ -148,7 +148,7 @@ QAbstractFileEngineHandler::~QAbstractFileEngineHandler() QAbstractFileEngineHandlerList *handlers = fileEngineHandlers(); handlers->removeOne(this); if (handlers->isEmpty()) - qt_file_engine_handlers_in_use = false; + qt_file_engine_handlers_in_use.storeRelaxed(false); } } @@ -161,7 +161,7 @@ QAbstractFileEngine *qt_custom_file_engine_handler_create(const QString &path) { QAbstractFileEngine *engine = nullptr; - if (qt_file_engine_handlers_in_use) { + if (qt_file_engine_handlers_in_use.loadRelaxed()) { QReadLocker locker(fileEngineHandlerMutex()); // check for registered handlers that can load the file