From ebc99ef71af107e08b92777decc303a359f1c5a6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 27 Aug 2013 14:35:47 +0100 Subject: [PATCH] _dbus_get_tmpdir: be thread-safe Sharing a static variable between threads is not safe in general, and this function is used in the shared libdbus (for nonce files), so it can't rely on being single-threaded. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=68610 --- bus/activation.c | 8 +++++++- dbus/dbus-internals.h | 3 ++- dbus/dbus-nonce.c | 6 +++++- dbus/dbus-sysdeps-unix.c | 8 +++++++- dbus/dbus-sysdeps-win.c | 8 +++++++- 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 4269440..e03b6fe 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -2539,11 +2539,17 @@ dbus_bool_t bus_activation_service_reload_test (const DBusString *test_data_dir) { DBusString directory; + const char *tmp; if (!_dbus_string_init (&directory)) return FALSE; - if (!_dbus_string_append (&directory, _dbus_get_tmpdir())) + tmp = _dbus_get_tmpdir (); + + if (tmp == NULL) + return FALSE; + + if (!_dbus_string_append (&directory, tmp)) return FALSE; if (!_dbus_string_append (&directory, "/dbus-reload-test-") || diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 0856f8f..e9ffb9e 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -316,9 +316,10 @@ typedef enum _DBUS_LOCK_shutdown_funcs, _DBUS_LOCK_system_users, _DBUS_LOCK_message_cache, - /* index 10-11 */ + /* index 10-12 */ _DBUS_LOCK_shared_connections, _DBUS_LOCK_machine_uuid, + _DBUS_LOCK_sysdeps, _DBUS_N_GLOBAL_LOCKS } DBusGlobalLock; diff --git a/dbus/dbus-nonce.c b/dbus/dbus-nonce.c index e74c2dd..ef037ef 100644 --- a/dbus/dbus-nonce.c +++ b/dbus/dbus-nonce.c @@ -240,6 +240,7 @@ do_noncefile_create (DBusNonceFile *noncefile, dbus_bool_t use_subdir) { DBusString randomStr; + const char *tmp; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -257,8 +258,11 @@ do_noncefile_create (DBusNonceFile *noncefile, goto on_error; } + tmp = _dbus_get_tmpdir (); + if (!_dbus_string_init (&noncefile->dir) - || !_dbus_string_append (&noncefile->dir, _dbus_get_tmpdir())) + || tmp == NULL + || !_dbus_string_append (&noncefile->dir, tmp)) { dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL); goto on_error; diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index 66f55d7..1dcb6f8 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -3197,13 +3197,17 @@ _dbus_printf_string_upper_bound (const char *format, * Gets the temporary files directory by inspecting the environment variables * TMPDIR, TMP, and TEMP in that order. If none of those are set "/tmp" is returned * - * @returns location of temp directory + * @returns location of temp directory, or #NULL if no memory for locking */ const char* _dbus_get_tmpdir(void) { + /* Protected by _DBUS_LOCK_sysdeps */ static const char* tmpdir = NULL; + if (!_DBUS_LOCK (sysdeps)) + return NULL; + if (tmpdir == NULL) { /* TMPDIR is what glibc uses, then @@ -3226,6 +3230,8 @@ _dbus_get_tmpdir(void) tmpdir = "/tmp"; } + _DBUS_UNLOCK (sysdeps); + _dbus_assert(tmpdir != NULL); return tmpdir; diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index 38758bd..94293d9 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -2256,14 +2256,18 @@ _dbus_generate_random_bytes (DBusString *str, * Gets the temporary files directory by inspecting the environment variables * TMPDIR, TMP, and TEMP in that order. If none of those are set "/tmp" is returned * - * @returns location of temp directory + * @returns location of temp directory, or #NULL if no memory for locking */ const char* _dbus_get_tmpdir(void) { + /* Protected by _DBUS_LOCK_sysdeps */ static const char* tmpdir = NULL; static char buf[1000]; + if (!_DBUS_LOCK (sysdeps)) + return NULL; + if (tmpdir == NULL) { char *last_slash; @@ -2285,6 +2289,8 @@ _dbus_get_tmpdir(void) tmpdir = buf; } + _DBUS_UNLOCK (sysdeps); + _dbus_assert(tmpdir != NULL); return tmpdir; -- 1.8.4.rc3