1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid potential data races caused by qt_ntfs_permission_lookup

Browse Source 
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>
This commit is contained in:
Mate Barany 2023-01-13 18:18:08 +01:00
parent 214c3a033a
commit 84f0596c0c
8 changed files with 124 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/corelib/doc/snippets/ntfsp.cpp
View File

@ -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]

21
src/corelib/io/qfile.h
View File

@ -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)

14
src/corelib/io/qfiledevice.cpp
View File

@ -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

13
src/corelib/io/qfileinfo.cpp
View File

@ -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

45
src/corelib/io/qfilesystemengine_win.cpp
View File

@ -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();

5
tests/auto/corelib/io/qdir/tst_qdir.cpp
View File

@ -12,7 +12,6 @@
#include <qdebug.h>
#include <qdir.h>
#include <qfileinfo.h>
#include <qscopedvaluerollback.h>
#include <qstringlist.h>
#if defined(Q_OS_WIN)
@ -40,7 +39,6 @@
#ifdef Q_OS_WIN
#define DRIVE "Q:"
extern Q_CORE_EXPORT int qt_ntfs_permission_lookup;
#else
#define DRIVE
#endif
@ -453,8 +451,7 @@ void tst_QDir::mkdirWithPermissions()
QFETCH(QFile::Permissions, permissions);
#ifdef Q_OS_WIN
QScopedValueRollback<int> ntfsMode(qt_ntfs_permission_lookup);
++qt_ntfs_permission_lookup;
QNtfsPermissionCheckGuard permissionGuard;
#endif
#ifdef Q_OS_UNIX
auto restoreMask = qScopeGuard([oldMask = umask(0)] { umask(oldMask); });

21
tests/auto/corelib/io/qfile/tst_qfile.cpp
View File

@ -171,6 +171,7 @@ private slots:
#ifdef Q_OS_WIN
void permissionsNtfs_data();
void permissionsNtfs();
void deprecatedNtfsPermissionCheck();
#endif
void setPermissions_data();
void setPermissions();
@ -1267,8 +1268,7 @@ void tst_QFile::createFilePermissions()
QFETCH(QFile::Permissions, permissions);
#ifdef Q_OS_WIN
QScopedValueRollback<int> ntfsMode(qt_ntfs_permission_lookup);
++qt_ntfs_permission_lookup;
QNtfsPermissionCheckGuard permissionGuard;
#endif
#ifdef Q_OS_UNIX
auto restoreMask = qScopeGuard([oldMask = umask(0)] { umask(oldMask); });
@ -1392,7 +1392,7 @@ void tst_QFile::permissions()
}
#if defined(Q_OS_WIN)
if (qt_ntfs_permission_lookup)
if (qAreNtfsPermissionChecksEnabled())
QEXPECT_FAIL("readonly", "QTBUG-25630", Abort);
#endif
#ifdef Q_OS_UNIX
@ -1414,10 +1414,21 @@ void tst_QFile::permissionsNtfs_data()
void tst_QFile::permissionsNtfs()
{
QScopedValueRollback<int> ntfsMode(qt_ntfs_permission_lookup);
qt_ntfs_permission_lookup++;
QNtfsPermissionCheckGuard permissionGuard;
permissions();
}
void tst_QFile::deprecatedNtfsPermissionCheck()
{
QScopedValueRollback<int> guard(qt_ntfs_permission_lookup);
QCOMPARE(qAreNtfsPermissionChecksEnabled(), false);
qt_ntfs_permission_lookup++;
QCOMPARE(qAreNtfsPermissionChecksEnabled(), true);
qt_ntfs_permission_lookup--;
QCOMPARE(qAreNtfsPermissionChecksEnabled(), false);
}
#endif
void tst_QFile::setPermissions_data()

12
tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp
View File

@ -4,7 +4,6 @@
#include <QTest>
#include <QStandardPaths>
#include <QScopeGuard>
#include <QScopedValueRollback>
#include <qfile.h>
#include <qdir.h>
@ -43,9 +42,6 @@
#endif
#if defined(Q_OS_WIN)
QT_BEGIN_NAMESPACE
extern Q_CORE_EXPORT int qt_ntfs_permission_lookup;
QT_END_NAMESPACE
bool IsUserAdmin();
#endif
@ -1905,8 +1901,7 @@ void tst_QFileInfo::isWritable()
#endif
#if defined (Q_OS_WIN)
QScopedValueRollback<int> ntfsMode(qt_ntfs_permission_lookup);
qt_ntfs_permission_lookup = 1;
QNtfsPermissionCheckGuard permissionGuard;
QFileInfo fi2(QFile::decodeName(qgetenv("SystemRoot") + "/system.ini"));
QVERIFY(fi2.exists());
QCOMPARE(fi2.isWritable(), IsUserAdmin());
@ -2156,7 +2151,7 @@ void tst_QFileInfo::owner()
NetApiBufferFree(pBuf);
}
}
qt_ntfs_permission_lookup = 1;
QNtfsPermissionCheckGuard permissionGuard;
#endif
if (userName.isEmpty())
QSKIP("Can't retrieve the user name");
@ -2173,9 +2168,6 @@ void tst_QFileInfo::owner()
QCOMPARE(fi.owner(), userName);
QFile::remove(fileName);
#if defined(Q_OS_WIN)
qt_ntfs_permission_lookup = 0;
#endif
}
void tst_QFileInfo::group()