Avoid potential data races caused by qt_ntfs_permission_lookup

qt_ntfs_permission_lookup is a global, non-atomic variable which
could cause problems in case of multiple threads. Introduce a
new atomic variable to handle permission lookups but instead of
manual incrementation/decrementation, implement a class to manage
the variable.

Since the atomic variable is not directly available to the user,
implement helper functions to increase/decrease/check the status
of the variable.

Task-number: QTBUG-105804
Change-Id: If6cbcdd653c7f50ad9853a5c309e24fdeb520788
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>

5 files changed, 105 insertions, 3 deletions

diff --git a/src/corelib/doc/snippets/ntfsp.cpp b/src/corelib/doc/snippets/ntfsp.cpp
index b4d59e6ac5..18f9bd0c5e 100644
--- a/src/corelib/doc/snippets/ntfsp.cpp
+++ b/src/corelib/doc/snippets/ntfsp.cpp
@@ -11,3 +11,18 @@ qt_ntfs_permission_lookup++; // turn checking on
 qt_ntfs_permission_lookup--; // turn it off again
 //! [1]
 
+//! [raii]
+void complexFunction()
+{
+    QNtfsPermissionCheckGuard permissionGuard;  // check is enabled
+
+    // do complex things here that need permission check enabled
+
+}   // as the guard goes out of scope the check is disabled
+//! [raii]
+
+//! [free-funcs]
+qAreNtfsPermissionChecksEnabled();   // check status
+qEnableNtfsPermissionChecks();       // turn checking on
+qDisableNtfsPermissionChecks();      // turn it off again
+//! [free-funcs]
diff --git a/src/corelib/io/qfile.h b/src/corelib/io/qfile.h
index 3ac6795495..f608ca48f5 100644
--- a/src/corelib/io/qfile.h
+++ b/src/corelib/io/qfile.h
@@ -26,6 +26,27 @@ namespace std {
 
 QT_BEGIN_NAMESPACE
 
+#ifdef Q_OS_WIN
+Q_CORE_EXPORT bool qEnableNtfsPermissionChecks() noexcept;
+Q_CORE_EXPORT bool qDisableNtfsPermissionChecks() noexcept;
+Q_CORE_EXPORT bool qAreNtfsPermissionChecksEnabled() noexcept;
+
+class [[nodiscard]] QNtfsPermissionCheckGuard
+{
+    Q_DISABLE_COPY_MOVE(QNtfsPermissionCheckGuard)
+public:
+    QNtfsPermissionCheckGuard()
+    {
+        qEnableNtfsPermissionChecks();
+    }
+
+    ~QNtfsPermissionCheckGuard()
+    {
+        qDisableNtfsPermissionChecks();
+    }
+};
+#endif // Q_OS_WIN
+
 #if QT_CONFIG(cxx17_filesystem)
 namespace QtPrivate {
 inline QString fromFilesystemPath(const std::filesystem::path &path)
diff --git a/src/corelib/io/qfiledevice.cpp b/src/corelib/io/qfiledevice.cpp
index 6c790e9d0e..092b09ae05 100644
--- a/src/corelib/io/qfiledevice.cpp
+++ b/src/corelib/io/qfiledevice.cpp
@@ -114,6 +114,20 @@ void QFileDevicePrivate::setError(QFileDevice::FileError err, int errNum)
     to increment or decrement \c qt_ntfs_permission_lookup before any
     threads other than the main thread have started or after every thread
     other than the main thread has ended.
+
+    \note From Qt 6.6 the variable \c qt_ntfs_permission_lookup is
+    deprecated. Please use the following alternatives.
+
+    The safe and easy way to manage permission checks is to use the RAII class
+    \c QNtfsPermissionCheckGuard.
+
+    \snippet ntfsp.cpp raii
+
+    If you need more fine-grained control, it is possible to manage the permission
+    with the following functions instead:
+
+    \snippet ntfsp.cpp free-funcs
+
 */
 
 //************* QFileDevice
diff --git a/src/corelib/io/qfileinfo.cpp b/src/corelib/io/qfileinfo.cpp
index 49573347ca..5c9ae5df70 100644
--- a/src/corelib/io/qfileinfo.cpp
+++ b/src/corelib/io/qfileinfo.cpp
@@ -278,6 +278,19 @@ QDateTime &QFileInfoPrivate::getFileTime(QAbstractFileEngine::FileTime request)
     threads other than the main thread have started or after every thread
     other than the main thread has ended.
 
+    \note From Qt 6.6 the variable \c qt_ntfs_permission_lookup is
+    deprecated. Please use the following alternatives.
+
+    The safe and easy way to manage permission checks is to use the RAII class
+    \c QNtfsPermissionCheckGuard.
+
+    \snippet ntfsp.cpp raii
+
+    If you need more fine-grained control, it is possible to manage the permission
+    with the following functions instead:
+
+    \snippet ntfsp.cpp free-funcs
+
     \section1 Performance Considerations
 
     Some of QFileInfo's functions query the file system, but for
diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp
index 9cfa1d7b7d..fbc55954cb 100644
--- a/src/corelib/io/qfilesystemengine_win.cpp
+++ b/src/corelib/io/qfilesystemengine_win.cpp
@@ -384,6 +384,46 @@ constexpr QFileDevice::Permissions toSpecificPermissions(PermissionTag tag,
 
 Q_CORE_EXPORT int qt_ntfs_permission_lookup = 0;
 
+static QBasicAtomicInt qt_ntfs_permission_lookup_v2 = Q_BASIC_ATOMIC_INITIALIZER(0);
+
+/*!
+    \internal
+
+    Returns true if the check was previously enabled.
+*/
+
+bool qEnableNtfsPermissionChecks() noexcept
+{
+    return qt_ntfs_permission_lookup_v2.fetchAndAddRelaxed(1)
+        + qt_ntfs_permission_lookup
+        != 0;
+}
+
+/*!
+    \internal
+
+    Returns true if the check is disabled, i.e. there are no more users.
+*/
+
+bool qDisableNtfsPermissionChecks() noexcept
+{
+    return qt_ntfs_permission_lookup_v2.fetchAndSubRelaxed(1)
+        + qt_ntfs_permission_lookup
+        == 1;
+}
+
+/*!
+    \internal
+
+    Returns true if the check is enabled.
+*/
+
+bool qAreNtfsPermissionChecksEnabled() noexcept
+{
+    return qt_ntfs_permission_lookup_v2.loadRelaxed()
+        + qt_ntfs_permission_lookup;
+}
+
 /*!
     \class QNativeFilePermissions
     \internal
@@ -1078,8 +1118,7 @@ QString QFileSystemEngine::owner(const QFileSystemEntry &entry, QAbstractFileEng
 {
     QString name;
 #if QT_CONFIG(fslibs)
-    extern int qt_ntfs_permission_lookup;
-    if (qt_ntfs_permission_lookup > 0) {
+    if (qAreNtfsPermissionChecksEnabled()) {
         initGlobalSid();
         {
             PSID pOwner = 0;
@@ -1133,7 +1172,7 @@ bool QFileSystemEngine::fillPermissions(const QFileSystemEntry &entry, QFileSyst
                                         QFileSystemMetaData::MetaDataFlags what)
 {
 #if QT_CONFIG(fslibs)
-    if (qt_ntfs_permission_lookup > 0) {
+    if (qAreNtfsPermissionChecksEnabled()) {
         initGlobalSid();
 
         QString fname = entry.nativeFilePath();