Attachment 27585 Details for Bug 896 – 0001-Bug-896-Avoid-race-conditions-reading-message-from.patch

[patch] 0001-Bug-896-Avoid-race-conditions-reading-message-from.patch

0001-Bug-896-Avoid-race-conditions-reading-message-from.patch (text/plain), 5.58 KB, created by Colin Walters on 2009-07-10 18:37:15 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Walters

Created: 2009-07-10 18:37:15 UTC

Size: 5.58 KB

patch

obsolete

>From e4677cd87ff9e4e1a4e2716eacd30016d848bd08 Mon Sep 17 00:00:00 2001
>From: Colin Walters <walters@verbum.org>
>Date: Fri, 10 Jul 2009 21:33:02 -0400
>Subject: [PATCH] Bug 896 - Avoid race conditions reading message from exited process
>MIME-Version: 1.0
>Content-Type: text/plain; charset=utf-8
>Content-Transfer-Encoding: 8bit
>
>Patch based on extensive work from Michael Meeks <michael.meeks@novell.com>,
>thanks to Dafydd Harries <dafydd.harries@collabora.co.uk>,
>Kimmo HÃ¤mÃ¤lÃ¤inen <kimmo.hamalainen@nokia.com> and others.
>
>The basic idea with this bug is that we effectively ignore errors
>on write.  Only when we're done reading from a connection do we
>close down a connection.  This avoids a race condition where
>if a process (such as dbus-send) exited while we still had
>data to read in the buffer, we'd miss that data.
>---
> dbus/dbus-sysdeps.c          |   10 ++++++++++
> dbus/dbus-sysdeps.h          |    1 +
> dbus/dbus-transport-socket.c |   36 ++++++++++++++++++++++++++++--------
> dbus/dbus-watch.c            |    6 ++++++
> dbus/dbus-watch.h            |    1 +
> 5 files changed, 46 insertions(+), 8 deletions(-)
>
>diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
>index e0fe888..ccd80cc 100644
>--- a/dbus/dbus-sysdeps.c
>+++ b/dbus/dbus-sysdeps.c
>@@ -1078,6 +1078,16 @@ _dbus_get_is_errno_eintr (void)
> }
> 
> /**
>+ * See if errno is EPIPE
>+ * @returns #TRUE if errno == EPIPE
>+ */
>+dbus_bool_t
>+_dbus_get_is_errno_epipe (void)
>+{
>+  return errno == EPIPE;
>+}
>+
>+/**
>  * Get error message from errno
>  * @returns _dbus_strerror(errno)
>  */
>diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
>index 7af4287..7ef2df3 100644
>--- a/dbus/dbus-sysdeps.h
>+++ b/dbus/dbus-sysdeps.h
>@@ -362,6 +362,7 @@ dbus_bool_t _dbus_get_is_errno_nonzero               (void);
> dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (void);
> dbus_bool_t _dbus_get_is_errno_enomem                (void);
> dbus_bool_t _dbus_get_is_errno_eintr                 (void);
>+dbus_bool_t _dbus_get_is_errno_epipe                 (void);
> const char* _dbus_strerror_from_errno                (void);
> 
> void _dbus_disable_sigpipe (void);
>diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c
>index 46cbed9..8be4d13 100644
>--- a/dbus/dbus-transport-socket.c
>+++ b/dbus/dbus-transport-socket.c
>@@ -616,7 +616,11 @@ do_writing (DBusTransport *transport)
>         {
>           /* EINTR already handled for us */
>           
>-          if (_dbus_get_is_errno_eagain_or_ewouldblock ())
>+          /* For some discussion of why we also ignore EPIPE here, see
>+           * http://lists.freedesktop.org/archives/dbus/2008-March/009526.html
>+           */
>+          
>+          if (_dbus_get_is_errno_eagain_or_ewouldblock () || _dbus_get_is_errno_epipe ())
>             goto out;
>           else
>             {
>@@ -807,6 +811,25 @@ do_reading (DBusTransport *transport)
> }
> 
> static dbus_bool_t
>+unix_error_with_read_to_come (DBusTransport *itransport,
>+                              DBusWatch     *watch,
>+                              unsigned int   flags)
>+{
>+  DBusTransportSocket *transport = (DBusTransportSocket *) itransport;
>+
>+  if (!(flags & DBUS_WATCH_HANGUP || flags & DBUS_WATCH_ERROR))
>+    return FALSE;
>+   
>+  /* If we have a read watch enabled ...
>+     we -might have data incoming ... => handle the HANGUP there */
>+  if (watch != transport->read_watch &&
>+      _dbus_watch_get_enabled (transport->read_watch))
>+    return FALSE;
>+      
>+  return TRUE; 
>+}
>+
>+static dbus_bool_t
> socket_handle_watch (DBusTransport *transport,
>                    DBusWatch     *watch,
>                    unsigned int   flags)
>@@ -817,14 +840,11 @@ socket_handle_watch (DBusTransport *transport,
>                 watch == socket_transport->write_watch);
>   _dbus_assert (watch != NULL);
>   
>-  /* Disconnect in case of an error.  In case of hangup do not
>-   * disconnect the transport because data can still be in the buffer
>-   * and do_reading may need several iteration to read it all (because
>-   * of its max_bytes_read_per_iteration limit).  The condition where
>-   * flags == HANGUP (without READABLE) probably never happen in fact.
>+  /* If we hit an error here on a write watch, don't disconnect the transport yet because data can
>+   * still be in the buffer and do_reading may need several iteration to read
>+   * it all (because of its max_bytes_read_per_iteration limit). 
>    */
>-  if ((flags & DBUS_WATCH_ERROR) ||
>-      ((flags & DBUS_WATCH_HANGUP) && !(flags & DBUS_WATCH_READABLE)))
>+  if (!(flags & DBUS_WATCH_READABLE) && unix_error_with_read_to_come (transport, watch, flags))
>     {
>       _dbus_verbose ("Hang up or error on watch\n");
>       _dbus_transport_disconnect (transport);
>diff --git a/dbus/dbus-watch.c b/dbus/dbus-watch.c
>index 7ef27bf..bca699f 100644
>--- a/dbus/dbus-watch.c
>+++ b/dbus/dbus-watch.c
>@@ -51,6 +51,12 @@ struct DBusWatch
>   unsigned int enabled : 1;            /**< Whether it's enabled. */
> };
> 
>+dbus_bool_t
>+_dbus_watch_get_enabled (DBusWatch *watch)
>+{
>+  return watch->enabled;
>+}
>+
> /**
>  * Creates a new DBusWatch. Used to add a file descriptor to be polled
>  * by a main loop.
>diff --git a/dbus/dbus-watch.h b/dbus/dbus-watch.h
>index fd65ae3..fa953ec 100644
>--- a/dbus/dbus-watch.h
>+++ b/dbus/dbus-watch.h
>@@ -74,6 +74,7 @@ void           _dbus_watch_list_remove_watch  (DBusWatchList           *watch_li
> void           _dbus_watch_list_toggle_watch  (DBusWatchList           *watch_list,
>                                                DBusWatch               *watch,
>                                                dbus_bool_t              enabled);
>+dbus_bool_t    _dbus_watch_get_enabled        (DBusWatch              *watch);
> 
> /** @} */
> 
>-- 
>1.6.2.5
>

Actions: View | Splinter Review

Attachments on bug 896: 497 | 511 | 512 | 518 | 4976 | 24013 | 24014 | 24016 | 24019 | 24020 | 25304 | 27579 | 27585

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.