Attachment 123507 Details for Bug 95263 – third try

[patch] third try

0001-bus-fix-in-flight-fd-counting-logic.patch (text/plain), 4.57 KB, created by Lennart Poettering on 2016-05-05 22:51:14 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lennart Poettering

Created: 2016-05-05 22:51:14 UTC

Size: 4.57 KB

patch

obsolete

>From f5ba4c9dd83357d10eb9709c9629da3c1ae07365 Mon Sep 17 00:00:00 2001
>From: Lennart Poettering <lennart@poettering.net>
>Date: Thu, 5 May 2016 12:59:52 +0200
>Subject: [PATCH] bus: fix in-flight fd counting logic
>
>When a busy connection manages to always keep an fd queued, but still is
>processing messages, then this might cause dbus connection to kick it
>from the bus anyway, because the number of queued fds never goes down to
>zero.
>
>This patch changes that, and resets the in-flight timer each time any
>change on the fd queue is made, and thus is not stuck or stale.
>
>Note that the actual resetting of the in-flight timer was previously
>already broken, as _dbus_connection_toggle_timeout_unlocked() wasn't
>called to properly restart the timeout from the current time. This adds
>the necessary calls.
>
>This also adds a log message whenever the fd-in-flight timer hits, since
>the timer elapsing should usually be a very exceptional condition, and a
>clear indication of a bug or attack somewhere.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=95263
>https://github.com/systemd/systemd/issues/1961
>---
> bus/connection.c     | 17 +++++++++++------
> dbus/dbus-mainloop.c | 26 ++++++++++++++++++++++++++
> dbus/dbus-mainloop.h |  2 ++
> 3 files changed, 39 insertions(+), 6 deletions(-)
>
>diff --git a/bus/connection.c b/bus/connection.c
>index 67793ba..7ed67b3 100644
>--- a/bus/connection.c
>+++ b/bus/connection.c
>@@ -651,19 +651,23 @@ check_pending_fds_cb (DBusConnection *connection)
>   _dbus_verbose ("Pending fds count changed on connection %p: %d -> %d\n",
>                  connection, n_pending_unix_fds_old, n_pending_unix_fds_new);
> 
>-  if (n_pending_unix_fds_old == 0 && n_pending_unix_fds_new > 0)
>+  if (n_pending_unix_fds_new == 0)
>     {
>+      /* Stop timer if there are no fds leftâ¦ */
>+      _dbus_timeout_set_enabled (d->pending_unix_fds_timeout, FALSE);
>+    }
>+  else if (n_pending_unix_fds_old != n_pending_unix_fds_new)
>+    {
>+      /* Start timer each time an fd is added or removed, i.e. the list of fds isn't stale or stuck */
>       _dbus_timeout_set_interval (d->pending_unix_fds_timeout,
>               bus_context_get_pending_fd_timeout (d->connections->context));
>       _dbus_timeout_set_enabled (d->pending_unix_fds_timeout, TRUE);
>-    }
> 
>-  if (n_pending_unix_fds_old > 0 && n_pending_unix_fds_new == 0)
>-    {
>-      _dbus_timeout_set_enabled (d->pending_unix_fds_timeout, FALSE);
>+      /* Restart the time-out with the current time. */
>+      _dbus_loop_restart_timeout (bus_context_get_loop (d->connections->context),
>+                                  d->pending_unix_fds_timeout);
>     }
> 
>-
>   d->n_pending_unix_fds = n_pending_unix_fds_new;
> }
> 
>@@ -671,6 +675,7 @@ static dbus_bool_t
> pending_unix_fds_timeout_cb (void *data)
> {
>   DBusConnection *connection = data;
>+  _dbus_warn ("Client sent too many file descriptors, disconnecting.");
>   dbus_connection_close (connection);
>   return TRUE;
> }
>diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c
>index f6736fe..3d3ac80 100644
>--- a/dbus/dbus-mainloop.c
>+++ b/dbus/dbus-mainloop.c
>@@ -418,6 +418,32 @@ _dbus_loop_remove_timeout (DBusLoop           *loop,
>   _dbus_warn ("could not find timeout %p to remove\n", timeout);
> }
> 
>+void
>+_dbus_loop_restart_timeout (DBusLoop            *loop,
>+                            DBusTimeout         *timeout) {
>+
>+  DBusList *link;
>+
>+  link = _dbus_list_get_first_link (&loop->timeouts);
>+  while (link != NULL)
>+    {
>+      DBusList *next = _dbus_list_get_next_link (&loop->timeouts, link);
>+      TimeoutCallback *this = link->data;
>+
>+      if (this->timeout == timeout)
>+        {
>+          _dbus_get_monotonic_time (&this->last_tv_sec,
>+                                    &this->last_tv_usec);
>+          return;
>+        }
>+
>+      link = next;
>+    }
>+
>+  _dbus_warn ("could not find timeout %p to restart\n", timeout);
>+}
>+
>+
> /* Convolutions from GLib, there really must be a better way
>  * to do this.
>  */
>diff --git a/dbus/dbus-mainloop.h b/dbus/dbus-mainloop.h
>index a76cb6f..cc60500 100644
>--- a/dbus/dbus-mainloop.h
>+++ b/dbus/dbus-mainloop.h
>@@ -47,6 +47,8 @@ dbus_bool_t _dbus_loop_add_timeout    (DBusLoop            *loop,
>                                        DBusTimeout         *timeout);
> void        _dbus_loop_remove_timeout (DBusLoop            *loop,
>                                        DBusTimeout         *timeout);
>+void        _dbus_loop_restart_timeout(DBusLoop            *loop,
>+                                       DBusTimeout         *timeout);
> 
> dbus_bool_t _dbus_loop_queue_dispatch (DBusLoop            *loop,
>                                        DBusConnection      *connection);
>-- 
>2.7.4
>

Actions: View | Splinter Review

Attachments on bug 95263: 123493 | 123506 | 123507 | 123568 | 124840 | 124841 | 124842 | 129002

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.