Attachment 59863 Details for Bug 48580 – Patch v3

[patch] Patch v3

0001-Avoid-using-monotonic-time-in-the-DBUS_COOKIE_SHA1-a.patch (text/plain), 14.77 KB, created by David Zeuthen (not reading bugmail) on 2012-04-12 07:55:04 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: David Zeuthen (not reading bugmail)

Created: 2012-04-12 07:55:04 UTC

Size: 14.77 KB

patch

obsolete

>From 8734e4a16ff220a7af0fd718ba50f92d23c496cf Mon Sep 17 00:00:00 2001
>From: David Zeuthen <davidz@redhat.com>
>Date: Wed, 11 Apr 2012 23:05:33 -0400
>Subject: [PATCH] Avoid using monotonic time in the DBUS_COOKIE_SHA1
> authentication method
>
>When libdbus-1 moved to using monotonic time support for the
>DBUS_COOKIE_SHA1 authentication was broken, in particular
>interoperability with non-libdbus-1 implementations such as GDBus.
>
>The problem is that if monotonic clocks are available in the OS,
>_dbus_get_current_time() will not return the number of seconds since
>the Epoch so using it for DBUS_COOKIE_SHA1 will violate the D-Bus
>specification. If both peers are using libdbus-1 it's not a problem
>since both ends will use the wrong time and thus agree. However, if
>the other end is another implementation and following the spec it will
>not work.
>
>First, we change _dbus_get_current_time() back so it always returns
>time since the Epoch and we then rename it _dbus_get_real_time() to
>make this clear. We then introduce _dbus_get_monotonic_time() and
>carefully make all current users of _dbus_get_current_time() use it,
>if applicable. During this audit, one of the callers,
>_dbus_generate_uuid(), was currently using monotonic time but it was
>decided to make it use real time instead.
>
>Signed-off-by: David Zeuthen <davidz@redhat.com>
>Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=48580
>---
> bus/connection.c                            |   10 +++++-----
> bus/expirelist.c                            |   24 ++++++++++++------------
> dbus/dbus-connection.c                      |    4 ++--
> dbus/dbus-internals.c                       |    5 ++++-
> dbus/dbus-keyring.c                         |    6 +++---
> dbus/dbus-mainloop.c                        |    8 ++++----
> dbus/dbus-sysdeps-unix.c                    |   27 ++++++++++++++++++++++++---
> dbus/dbus-sysdeps-win.c                     |   21 ++++++++++++++++++---
> dbus/dbus-sysdeps.c                         |    2 +-
> dbus/dbus-sysdeps.h                         |    7 +++++--
> test/break-loader.c                         |    2 +-
> test/name-test/test-pending-call-dispatch.c |    4 ++--
> test/name-test/test-pending-call-timeout.c  |    4 ++--
> 13 files changed, 83 insertions(+), 41 deletions(-)
>
>diff --git a/bus/connection.c b/bus/connection.c
>index 97e5f64..d69758c 100644
>--- a/bus/connection.c
>+++ b/bus/connection.c
>@@ -606,8 +606,8 @@ bus_connections_setup_connection (BusConnections *connections,
>   d->connections = connections;
>   d->connection = connection;
>   
>-  _dbus_get_current_time (&d->connection_tv_sec,
>-                          &d->connection_tv_usec);
>+  _dbus_get_monotonic_time (&d->connection_tv_sec,
>+                            &d->connection_tv_usec);
>   
>   _dbus_assert (connection_data_slot >= 0);
>   
>@@ -776,7 +776,7 @@ bus_connections_expire_incomplete (BusConnections *connections)
>       DBusList *link;
>       int auth_timeout;
>       
>-      _dbus_get_current_time (&tv_sec, &tv_usec);
>+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
>       auth_timeout = bus_context_get_auth_timeout (connections->context);
>   
>       link = _dbus_list_get_first_link (&connections->incomplete);
>@@ -1772,8 +1772,8 @@ bus_connections_expect_reply (BusConnections  *connections,
>   cprd->pending = pending;
>   cprd->connections = connections;
>   
>-  _dbus_get_current_time (&pending->expire_item.added_tv_sec,
>-                          &pending->expire_item.added_tv_usec);
>+  _dbus_get_monotonic_time (&pending->expire_item.added_tv_sec,
>+                            &pending->expire_item.added_tv_usec);
> 
>   _dbus_verbose ("Added pending reply %p, replier %p receiver %p serial %u\n",
>                  pending,
>diff --git a/bus/expirelist.c b/bus/expirelist.c
>index 3c87c11..1a12ea4 100644
>--- a/bus/expirelist.c
>+++ b/bus/expirelist.c
>@@ -123,9 +123,9 @@ bus_expire_list_recheck_immediately (BusExpireList *list)
> }
> 
> static int
>-do_expiration_with_current_time (BusExpireList *list,
>-                                 long           tv_sec,
>-                                 long           tv_usec)
>+do_expiration_with_monotonic_time (BusExpireList *list,
>+                                   long           tv_sec,
>+                                   long           tv_usec)
> {
>   DBusList *link;
>   int next_interval, min_wait_time, items_to_expire;
>@@ -194,9 +194,9 @@ bus_expirelist_expire (BusExpireList *list)
>     {
>       long tv_sec, tv_usec;
> 
>-      _dbus_get_current_time (&tv_sec, &tv_usec);
>+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
> 
>-      next_interval = do_expiration_with_current_time (list, tv_sec, tv_usec);
>+      next_interval = do_expiration_with_monotonic_time (list, tv_sec, tv_usec);
>     }
> 
>   bus_expire_timeout_set_interval (list->timeout, next_interval);
>@@ -340,7 +340,7 @@ bus_expire_list_test (const DBusString *test_data_dir)
>                               test_expire_func, NULL);
>   _dbus_assert (list != NULL);
> 
>-  _dbus_get_current_time (&tv_sec, &tv_usec);
>+  _dbus_get_monotonic_time (&tv_sec, &tv_usec);
> 
>   tv_sec_not_expired = tv_sec;
>   tv_usec_not_expired = tv_usec;
>@@ -367,22 +367,22 @@ bus_expire_list_test (const DBusString *test_data_dir)
>     _dbus_assert_not_reached ("out of memory");
> 
>   next_interval =
>-    do_expiration_with_current_time (list, tv_sec_not_expired,
>-                                     tv_usec_not_expired);
>+    do_expiration_with_monotonic_time (list, tv_sec_not_expired,
>+                                       tv_usec_not_expired);
>   _dbus_assert (item->expire_count == 0);
>   _dbus_verbose ("next_interval = %d\n", next_interval);
>   _dbus_assert (next_interval == 1);
>   
>   next_interval =
>-    do_expiration_with_current_time (list, tv_sec_expired,
>-                                     tv_usec_expired);
>+    do_expiration_with_monotonic_time (list, tv_sec_expired,
>+                                       tv_usec_expired);
>   _dbus_assert (item->expire_count == 1);
>   _dbus_verbose ("next_interval = %d\n", next_interval);
>   _dbus_assert (next_interval == -1);
> 
>   next_interval =
>-    do_expiration_with_current_time (list, tv_sec_past,
>-                                     tv_usec_past);
>+    do_expiration_with_monotonic_time (list, tv_sec_past,
>+                                       tv_usec_past);
>   _dbus_assert (item->expire_count == 1);
>   _dbus_verbose ("next_interval = %d\n", next_interval);
>   _dbus_assert (next_interval == 1000 + EXPIRE_AFTER);
>diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
>index ac0b2c0..ee33b6c 100644
>--- a/dbus/dbus-connection.c
>+++ b/dbus/dbus-connection.c
>@@ -2388,7 +2388,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
>    * below
>    */
>   timeout = _dbus_pending_call_get_timeout_unlocked (pending);
>-  _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
>+  _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
>   if (timeout)
>     {
>       timeout_milliseconds = dbus_timeout_get_interval (timeout);
>@@ -2445,7 +2445,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
>         return;
>     }
>   
>-  _dbus_get_current_time (&tv_sec, &tv_usec);
>+  _dbus_get_monotonic_time (&tv_sec, &tv_usec);
>   elapsed_milliseconds = (tv_sec - start_tv_sec) * 1000 +
> 	  (tv_usec - start_tv_usec) / 1000;
>   
>diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
>index a5db146..0e5d807 100644
>--- a/dbus/dbus-internals.c
>+++ b/dbus/dbus-internals.c
>@@ -660,7 +660,10 @@ _dbus_generate_uuid (DBusGUID *uuid)
> {
>   long now;
> 
>-  _dbus_get_current_time (&now, NULL);
>+  /* don't use monotonic time because the UUID may be saved to disk, e.g.
>+   * it may persist across reboots
>+   */
>+  _dbus_get_real_time (&now, NULL);
> 
>   uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now);
>   
>diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c
>index 35e8d74..23b9df5 100644
>--- a/dbus/dbus-keyring.c
>+++ b/dbus/dbus-keyring.c
>@@ -353,7 +353,7 @@ add_new_key (DBusKey  **keys_p,
>       goto out;
>     }
> 
>-  _dbus_get_current_time (&timestamp, NULL);
>+  _dbus_get_real_time (&timestamp, NULL);
>       
>   keys[n_keys-1].id = id;
>   keys[n_keys-1].creation_time = timestamp;
>@@ -428,7 +428,7 @@ _dbus_keyring_reload (DBusKeyring *keyring,
>   retval = FALSE;
>   have_lock = FALSE;
> 
>-  _dbus_get_current_time (&now, NULL);
>+  _dbus_get_real_time (&now, NULL);
>   
>   if (add_new)
>     {
>@@ -908,7 +908,7 @@ find_recent_key (DBusKeyring *keyring)
>   int i;
>   long tv_sec, tv_usec;
> 
>-  _dbus_get_current_time (&tv_sec, &tv_usec);
>+  _dbus_get_real_time (&tv_sec, &tv_usec);
>   
>   i = 0;
>   while (i < keyring->n_keys)
>diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c
>index 8a30f7f..cef676a 100644
>--- a/dbus/dbus-mainloop.c
>+++ b/dbus/dbus-mainloop.c
>@@ -91,8 +91,8 @@ timeout_callback_new (DBusTimeout         *timeout)
>     return NULL;
> 
>   cb->timeout = timeout;
>-  _dbus_get_current_time (&cb->last_tv_sec,
>-                          &cb->last_tv_usec);
>+  _dbus_get_monotonic_time (&cb->last_tv_sec,
>+                            &cb->last_tv_usec);
>   return cb;
> }
> 
>@@ -619,7 +619,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
>       unsigned long tv_sec;
>       unsigned long tv_usec;
>       
>-      _dbus_get_current_time (&tv_sec, &tv_usec);
>+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
> 
>       link = _dbus_list_get_first_link (&loop->timeouts);
>       while (link != NULL)
>@@ -728,7 +728,7 @@ _dbus_loop_iterate (DBusLoop     *loop,
>       unsigned long tv_sec;
>       unsigned long tv_usec;
> 
>-      _dbus_get_current_time (&tv_sec, &tv_usec);
>+      _dbus_get_monotonic_time (&tv_sec, &tv_usec);
> 
>       /* It'd be nice to avoid this O(n) thingy here */
>       link = _dbus_list_get_first_link (&loop->timeouts);
>diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
>index 9fddca7..cef8bd3 100644
>--- a/dbus/dbus-sysdeps-unix.c
>+++ b/dbus/dbus-sysdeps-unix.c
>@@ -2598,11 +2598,11 @@ _dbus_poll (DBusPollFD *fds,
>  * available, to avoid problems when the system time changes.
>  *
>  * @param tv_sec return location for number of seconds
>- * @param tv_usec return location for number of microseconds (thousandths)
>+ * @param tv_usec return location for number of microseconds
>  */
> void
>-_dbus_get_current_time (long *tv_sec,
>-                        long *tv_usec)
>+_dbus_get_monotonic_time (long *tv_sec,
>+                          long *tv_usec)
> {
> #ifdef HAVE_MONOTONIC_CLOCK
>   struct timespec ts;
>@@ -2625,6 +2625,27 @@ _dbus_get_current_time (long *tv_sec,
> }
> 
> /**
>+ * Get current time, as in gettimeofday(). Never uses the monotonic
>+ * clock.
>+ *
>+ * @param tv_sec return location for number of seconds
>+ * @param tv_usec return location for number of microseconds
>+ */
>+void
>+_dbus_get_real_time (long *tv_sec,
>+                     long *tv_usec)
>+{
>+  struct timeval t;
>+
>+  gettimeofday (&t, NULL);
>+
>+  if (tv_sec)
>+    *tv_sec = t.tv_sec;
>+  if (tv_usec)
>+    *tv_usec = t.tv_usec;
>+}
>+
>+/**
>  * Creates a directory; succeeds if the directory
>  * is created or already existed.
>  *
>diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
>index e30e92f..397520a 100644
>--- a/dbus/dbus-sysdeps-win.c
>+++ b/dbus/dbus-sysdeps-win.c
>@@ -1872,14 +1872,15 @@ _dbus_sleep_milliseconds (int milliseconds)
> 
> 
> /**
>- * Get current time, as in gettimeofday().
>+ * Get current time, as in gettimeofday(). Never uses the monotonic
>+ * clock.
>  *
>  * @param tv_sec return location for number of seconds
>  * @param tv_usec return location for number of microseconds
>  */
> void
>-_dbus_get_current_time (long *tv_sec,
>-                        long *tv_usec)
>+_dbus_get_real_time (long *tv_sec,
>+                     long *tv_usec)
> {
>   FILETIME ft;
>   dbus_uint64_t time64;
>@@ -1901,6 +1902,20 @@ _dbus_get_current_time (long *tv_sec,
>     *tv_usec = time64 % 1000000;
> }
> 
>+/**
>+ * Get current time, as in gettimeofday(). Use the monotonic clock if
>+ * available, to avoid problems when the system time changes.
>+ *
>+ * @param tv_sec return location for number of seconds
>+ * @param tv_usec return location for number of microseconds
>+ */
>+void
>+_dbus_get_monotonic_time (long *tv_sec,
>+                          long *tv_usec)
>+{
>+  /* no implementation yet, fall back to wall-clock time */
>+  _dbus_get_real_time (tv_sec, tv_usec);
>+}
> 
> /**
>  * signal (SIGPIPE, SIG_IGN);
>diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
>index 1a93cea..861bfec 100644
>--- a/dbus/dbus-sysdeps.c
>+++ b/dbus/dbus-sysdeps.c
>@@ -508,7 +508,7 @@ _dbus_generate_pseudorandom_bytes_buffer (char *buffer,
>   _dbus_verbose ("Falling back to pseudorandom for %d bytes\n",
>                  n_bytes);
>   
>-  _dbus_get_current_time (NULL, &tv_usec);
>+  _dbus_get_real_time (NULL, &tv_usec);
>   srand (tv_usec);
>   
>   i = 0;
>diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
>index f8438f4..c03e2f5 100644
>--- a/dbus/dbus-sysdeps.h
>+++ b/dbus/dbus-sysdeps.h
>@@ -308,8 +308,11 @@ int _dbus_poll (DBusPollFD *fds,
> 
> void _dbus_sleep_milliseconds (int milliseconds);
> 
>-void _dbus_get_current_time (long *tv_sec,
>-                             long *tv_usec);
>+void _dbus_get_real_time (long *tv_sec,
>+                          long *tv_usec);
>+
>+void _dbus_get_monotonic_time (long *tv_sec,
>+                               long *tv_usec);
> 
> /**
>  * directory interface
>diff --git a/test/break-loader.c b/test/break-loader.c
>index 542f36f..e62b8c2 100644
>--- a/test/break-loader.c
>+++ b/test/break-loader.c
>@@ -667,7 +667,7 @@ get_random_seed (void)
> 
>     fprintf (stderr, "could not open/read /dev/urandom, using current time for seed\n");
> 
>-    _dbus_get_current_time (NULL, &tv_usec);
>+    _dbus_get_monotonic_time (NULL, &tv_usec);
> 
>     seed = tv_usec;
>   }
>diff --git a/test/name-test/test-pending-call-dispatch.c b/test/name-test/test-pending-call-dispatch.c
>index 57582d4..c8b5a46 100644
>--- a/test/name-test/test-pending-call-dispatch.c
>+++ b/test/name-test/test-pending-call-dispatch.c
>@@ -98,9 +98,9 @@ main (int argc, char *argv[])
>     {
>       long delta;
>       
>-      _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
>+      _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
>       _run_iteration (conn);
>-      _dbus_get_current_time (&end_tv_sec, &end_tv_usec);
>+      _dbus_get_monotonic_time (&end_tv_sec, &end_tv_usec);
> 
>       /* we just care about seconds */
>       delta = end_tv_sec - start_tv_sec;
>diff --git a/test/name-test/test-pending-call-timeout.c b/test/name-test/test-pending-call-timeout.c
>index 381113b..d051fab 100644
>--- a/test/name-test/test-pending-call-timeout.c
>+++ b/test/name-test/test-pending-call-timeout.c
>@@ -82,9 +82,9 @@ main (int argc, char *argv[])
>     {
>       long delta;
>       
>-      _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
>+      _dbus_get_monotonic_time (&start_tv_sec, &start_tv_usec);
>       _run_iteration (conn);
>-      _dbus_get_current_time (&end_tv_sec, &end_tv_usec);
>+      _dbus_get_monotonic_time (&end_tv_sec, &end_tv_usec);
> 
>       /* we just care about seconds */
>       delta = end_tv_sec - start_tv_sec;
>-- 
>1.7.10
>

Actions: View | Splinter Review

Attachments on bug 48580: 59826 | 59858 | 59863

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.