Fix crashes when accessing environment variables concurrently

We've seen crashes with QThreadPrivate::start using qgetenv during the creation
of the event dispatcher, while another thread (for example the gui thread)
called qputenv. This is inherently thread-unsafe and there are many places
where we make the assumption that using the environment is safe. However access
to the environment is inherently unsafe in the C runtime and the best that we
can do is add a mutex around the Qt environment access functions, to at least
protect ourselves and our users.

Change-Id: Ie9a718d9f7ce63c423c645f0be3e3f4933e1cb08
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Reviewed-by: Jørgen Lind <jorgen.lind@theqtcompany.com>
This commit is contained in:
Simon Hausmann 2015-04-24 10:47:29 +02:00
parent 4b9cf0379f
commit 8272f5415b
5 changed files with 18 additions and 7 deletions

View File

@ -42,6 +42,7 @@
#include <private/qlocale_tools_p.h>
#include <private/qsystemlibrary_p.h>
#include <qmutex.h>
#ifndef QT_NO_QOBJECT
#include <private/qthread_p.h>
@ -3029,6 +3030,10 @@ QString qt_error_string(int errorCode)
return ret.trimmed();
}
// In the C runtime on all platforms access to the environment is not thread-safe. We
// add thread-safety for the Qt wrappers.
static QBasicMutex environmentMutex;
// getenv is declared as deprecated in VS2005. This function
// makes use of the new secure getenv function.
/*!
@ -3046,6 +3051,7 @@ QString qt_error_string(int errorCode)
*/
QByteArray qgetenv(const char *varName)
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
size_t requiredSize = 0;
QByteArray buffer;
@ -3079,6 +3085,7 @@ QByteArray qgetenv(const char *varName)
*/
bool qEnvironmentVariableIsEmpty(const char *varName) Q_DECL_NOEXCEPT
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
// we provide a buffer that can only hold the empty string, so
// when the env.var isn't empty, we'll get an ERANGE error (buffer
@ -3110,6 +3117,7 @@ bool qEnvironmentVariableIsEmpty(const char *varName) Q_DECL_NOEXCEPT
*/
int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
// we provide a buffer that can hold any int value:
static const int NumBinaryDigitsPerOctalDigit = 3;
@ -3158,6 +3166,7 @@ int qEnvironmentVariableIntValue(const char *varName, bool *ok) Q_DECL_NOEXCEPT
*/
bool qEnvironmentVariableIsSet(const char *varName) Q_DECL_NOEXCEPT
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
size_t requiredSize = 0;
(void)getenv_s(&requiredSize, 0, 0, varName);
@ -3187,6 +3196,7 @@ bool qEnvironmentVariableIsSet(const char *varName) Q_DECL_NOEXCEPT
*/
bool qputenv(const char *varName, const QByteArray& value)
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
return _putenv_s(varName, value.constData()) == 0;
#elif (defined(_POSIX_VERSION) && (_POSIX_VERSION-0) >= 200112L) || defined(Q_OS_HAIKU)
@ -3217,6 +3227,7 @@ bool qputenv(const char *varName, const QByteArray& value)
*/
bool qunsetenv(const char *varName)
{
QMutexLocker locker(&environmentMutex);
#if defined(_MSC_VER) && _MSC_VER >= 1400
return _putenv_s(varName, "") == 0;
#elif (defined(_POSIX_VERSION) && (_POSIX_VERSION-0) >= 200112L) || defined(Q_OS_BSD4) || defined(Q_OS_HAIKU)

View File

@ -405,7 +405,7 @@ void QProcessPrivate::startProcess()
char **path = 0;
int pathc = 0;
if (!program.contains(QLatin1Char('/'))) {
const QString pathEnv = QString::fromLocal8Bit(::getenv("PATH"));
const QString pathEnv = QString::fromLocal8Bit(qgetenv("PATH"));
if (!pathEnv.isEmpty()) {
QStringList pathEntries = pathEnv.split(QLatin1Char(':'), QString::SkipEmptyParts);
if (!pathEntries.isEmpty()) {
@ -1187,7 +1187,7 @@ bool QProcessPrivate::startDetached(const QString &program, const QStringList &a
argv[arguments.size() + 1] = 0;
if (!program.contains(QLatin1Char('/'))) {
const QString path = QString::fromLocal8Bit(::getenv("PATH"));
const QString path = QString::fromLocal8Bit(qgetenv("PATH"));
if (!path.isEmpty()) {
QStringList pathEntries = path.split(QLatin1Char(':'));
for (int k = 0; k < pathEntries.size(); ++k) {

View File

@ -1048,12 +1048,12 @@ static void initDefaultPaths(QMutexLocker *locker)
// Non XDG platforms (OS X, iOS, Blackberry, Android...) have used this code path erroneously
// for some time now. Moving away from that would require migrating existing settings.
QString userPath;
char *env = getenv("XDG_CONFIG_HOME");
if (env == 0) {
QByteArray env = qgetenv("XDG_CONFIG_HOME");
if (env.isEmpty()) {
userPath = QDir::homePath();
userPath += QLatin1Char('/');
userPath += QLatin1String(".config");
} else if (*env == '/') {
} else if (env.startsWith('/')) {
userPath = QFile::decodeName(env);
} else {
userPath = QDir::homePath();

View File

@ -340,7 +340,7 @@ QIBusPlatformInputContextPrivate::QIBusPlatformInputContextPrivate()
QDBusConnection *QIBusPlatformInputContextPrivate::createConnection()
{
QByteArray display(getenv("DISPLAY"));
QByteArray display(qgetenv("DISPLAY"));
QByteArray host = "unix";
QByteArray displayNumber = "0";

View File

@ -3768,7 +3768,7 @@ QString QFileDialogPrivate::getEnvironmentVariable(const QString &string)
{
#ifdef Q_OS_UNIX
if (string.size() > 1 && string.startsWith(QLatin1Char('$'))) {
return QString::fromLocal8Bit(getenv(string.mid(1).toLatin1().constData()));
return QString::fromLocal8Bit(qgetenv(string.mid(1).toLatin1().constData()));
}
#else
if (string.size() > 2 && string.startsWith(QLatin1Char('%')) && string.endsWith(QLatin1Char('%'))) {