Attachment 84715 Details for Bug 68610 – _dbus_get_tmpdir: be thread-safe

[patch] _dbus_get_tmpdir: be thread-safe

0001-_dbus_get_tmpdir-be-thread-safe.patch (text/plain), 4.27 KB, created by Simon McVittie on 2013-08-27 13:41:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-08-27 13:41:05 UTC

Size: 4.27 KB

patch

obsolete

>From ebc99ef71af107e08b92777decc303a359f1c5a6 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View | Splinter Review

Attachments on bug 68610: 84714 | 84715 | 84830 | 84831 | 84832 | 84833 | 84834 | 84837 | 85080 | 85113 | 85241

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.