Attachment 102996 Details for Bug 81469 – [PATCH 3/3] enforce new limit max_connections_per_cgroup

[patch] [PATCH 3/3] enforce new limit max_connections_per_cgroup

0003-enforce-new-limit-max_connections_per_cgroup.patch (text/plain), 10.54 KB, created by Alban Crequy on 2014-07-17 18:41:49 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Alban Crequy

Created: 2014-07-17 18:41:49 UTC

Size: 10.54 KB

patch

obsolete

>From de4a8d4f35ccb9918326fb1b893b62c68e1967c5 Mon Sep 17 00:00:00 2001
>From: Alban Crequy <alban.crequy@collabora.co.uk>
>Date: Wed, 16 Jul 2014 16:36:52 +0100
>Subject: [PATCH 3/3] enforce new limit max_connections_per_cgroup
>
>When it is not possible to get the Unix pid of a connection, that connection
>bypass the limit. It can happen on Windows or on the non-supported case where
>dbus-daemon is configured to listen on TCP rather than a Unix socket.
>---
> bus/connection.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 166 insertions(+), 9 deletions(-)
>
>diff --git a/bus/connection.c b/bus/connection.c
>index ea2d155..e064a0c 100644
>--- a/bus/connection.c
>+++ b/bus/connection.c
>@@ -59,6 +59,7 @@ struct BusConnections
>   int n_incomplete;     /**< Length of incomplete list */
>   BusContext *context;
>   DBusHashTable *completed_by_user; /**< Number of completed connections for each UID */
>+  DBusHashTable *completed_by_cgroup; /**< Number of completed connections for each cgroup */
>   DBusTimeout *expire_timeout; /**< Timeout for expiring incomplete connections. */
>   int stamp;                   /**< Incrementing number */
>   BusExpireList *pending_replies; /**< List of pending replies */
>@@ -90,6 +91,8 @@ typedef struct
>   DBusMessage *oom_message;
>   DBusPreallocatedSend *oom_preallocated;
>   BusClientPolicy *policy;
>+  dbus_bool_t tracking_cgroup;
>+  DBusString cgroup;
> 
>   char *cached_loginfo_string;
>   BusSELinuxID *selinux_id;
>@@ -127,6 +130,23 @@ connection_get_loop (DBusConnection *connection)
> 
> 
> static int
>+get_connections_for_cgroup (BusConnections *connections,
>+                            DBusString     *cgroup)
>+{
>+  void *val;
>+  int current_count;
>+
>+  /* val is NULL is 0 when it isn't in the hash yet */
>+
>+  val = _dbus_hash_table_lookup_string (connections->completed_by_cgroup,
>+                                        _dbus_string_get_data (cgroup));
>+
>+  current_count = _DBUS_POINTER_TO_INT (val);
>+
>+  return current_count;
>+}
>+
>+static int
> get_connections_for_uid (BusConnections *connections,
>                          dbus_uid_t      uid)
> {
>@@ -144,6 +164,72 @@ get_connections_for_uid (BusConnections *connections,
> }
> 
> static dbus_bool_t
>+adjust_connections_for_cgroup (BusConnections *connections,
>+                               DBusString     *cgroup,
>+                               int             adjustment)
>+{
>+  int current_count;
>+  char *cgroup_str;
>+
>+  cgroup_str = _dbus_string_get_data (cgroup);
>+
>+  current_count = get_connections_for_cgroup (connections, cgroup);
>+
>+  _dbus_verbose ("Adjusting connection count for cgroup %s: "
>+                 "was %d adjustment %d making %d\n",
>+                 cgroup_str, current_count, adjustment,
>+                 current_count + adjustment);
>+
>+  _dbus_assert (current_count >= 0);
>+
>+  current_count += adjustment;
>+
>+  _dbus_assert (current_count >= 0);
>+
>+  if (current_count == 0)
>+    {
>+      _dbus_hash_table_remove_string (connections->completed_by_cgroup,
>+                                      cgroup_str);
>+      return TRUE;
>+    }
>+  else
>+    {
>+      dbus_bool_t retval;
>+      DBusString copy;
>+
>+      /* duplicate the key */
>+
>+      if (!_dbus_string_init (&copy))
>+        return FALSE;
>+
>+      if (!_dbus_string_copy (cgroup, 0, &copy, 0))
>+        {
>+          _dbus_string_free (&copy);
>+          return FALSE;
>+        }
>+
>+      if (!_dbus_string_steal_data (&copy, &cgroup_str))
>+        {
>+          _dbus_string_free (&copy);
>+          return FALSE;
>+        }
>+
>+      _dbus_string_free (&copy);
>+
>+      retval = _dbus_hash_table_insert_string (connections->completed_by_cgroup,
>+                   cgroup_str, _DBUS_INT_TO_POINTER (current_count));
>+
>+      /* only positive adjustment can fail as otherwise
>+       * a hash entry should already exist
>+       */
>+      _dbus_assert (adjustment > 0 ||
>+                    (adjustment <= 0 && retval));
>+
>+      return retval;
>+    }
>+}
>+
>+static dbus_bool_t
> adjust_connections_for_uid (BusConnections *connections,
>                             dbus_uid_t      uid,
>                             int             adjustment)
>@@ -286,6 +372,12 @@ bus_connection_disconnected (DBusConnection *connection)
>               if (!adjust_connections_for_uid (d->connections,
>                                                uid, -1))
>                 _dbus_assert_not_reached ("adjusting downward should never fail");
>+
>+            }
>+          if (d->tracking_cgroup)
>+            {
>+              if (!adjust_connections_for_cgroup (d->connections, &d->cgroup, -1))
>+                _dbus_assert_not_reached ("adjusting downward should never fail");
>             }
>         }
>       else
>@@ -299,6 +391,8 @@ bus_connection_disconnected (DBusConnection *connection)
>       _dbus_assert (d->connections->n_completed >= 0);
>     }
> 
>+  _dbus_string_free (&d->cgroup);
>+
>   bus_connection_drop_pending_replies (d->connections, connection);
>   
>   /* frees "d" as side effect */
>@@ -429,11 +523,16 @@ bus_connections_new (BusContext *context)
>   if (connections->completed_by_user == NULL)
>     goto failed_2;
> 
>+  connections->completed_by_cgroup = _dbus_hash_table_new (DBUS_HASH_STRING,
>+                                                         dbus_free, NULL);
>+  if (connections->completed_by_cgroup == NULL)
>+    goto failed_3;
>+
>   connections->expire_timeout = _dbus_timeout_new (100, /* irrelevant */
>                                                    expire_incomplete_timeout,
>                                                    connections, NULL);
>   if (connections->expire_timeout == NULL)
>-    goto failed_3;
>+    goto failed_4;
> 
>   _dbus_timeout_set_enabled (connections->expire_timeout, FALSE);
> 
>@@ -442,21 +541,23 @@ bus_connections_new (BusContext *context)
>                                                       bus_pending_reply_expired,
>                                                       connections);
>   if (connections->pending_replies == NULL)
>-    goto failed_4;
>+    goto failed_5;
>   
>   if (!_dbus_loop_add_timeout (bus_context_get_loop (context),
>                                connections->expire_timeout))
>-    goto failed_5;
>+    goto failed_6;
>   
>   connections->refcount = 1;
>   connections->context = context;
>   
>   return connections;
> 
>- failed_5:
>+ failed_6:
>   bus_expire_list_free (connections->pending_replies);
>- failed_4:
>+ failed_5:
>   _dbus_timeout_unref (connections->expire_timeout);
>+ failed_4:
>+  _dbus_hash_table_unref (connections->completed_by_cgroup);
>  failed_3:
>   _dbus_hash_table_unref (connections->completed_by_user);
>  failed_2:
>@@ -521,6 +622,7 @@ bus_connections_unref (BusConnections *connections)
>       _dbus_timeout_unref (connections->expire_timeout);
>       
>       _dbus_hash_table_unref (connections->completed_by_user);
>+      _dbus_hash_table_unref (connections->completed_by_cgroup);
>       
>       dbus_free (connections);
> 
>@@ -596,7 +698,6 @@ bus_connections_setup_connection (BusConnections *connections,
>   BusConnectionData *d;
>   dbus_bool_t retval;
>   DBusError error;
>-
>   
>   d = dbus_new0 (BusConnectionData, 1);
>   
>@@ -661,6 +762,9 @@ bus_connections_setup_connection (BusConnections *connections,
>                                           allow_unix_user_function,
>                                           NULL, NULL);
> 
>+  if (!_dbus_string_init (&d->cgroup))
>+    goto out;
>+
>   dbus_connection_set_dispatch_status_function (connection,
>                                                 dispatch_status_function,
>                                                 bus_context_get_loop (connections->context),
>@@ -738,6 +842,8 @@ bus_connections_setup_connection (BusConnections *connections,
>       dbus_connection_set_unix_user_function (connection,
>                                               NULL, NULL, NULL);
> 
>+      _dbus_string_free (&d->cgroup);
>+
>       dbus_connection_set_windows_user_function (connection,
>                                                  NULL, NULL, NULL);
>       
>@@ -1358,6 +1464,7 @@ bus_connection_complete (DBusConnection   *connection,
> {
>   BusConnectionData *d;
>   unsigned long uid;
>+  unsigned long pid;
>   
>   d = BUS_CONNECTION_DATA (connection);
>   _dbus_assert (d != NULL);
>@@ -1397,8 +1504,17 @@ bus_connection_complete (DBusConnection   *connection,
>   
>   if (dbus_connection_get_unix_user (connection, &uid))
>     {
>-      if (!adjust_connections_for_uid (d->connections,
>-                                       uid, 1))
>+      if (!adjust_connections_for_uid (d->connections, uid, 1))
>+        goto fail;
>+
>+    }
>+
>+  if (d->tracking_cgroup)
>+    {
>+      if (!dbus_connection_get_unix_process_id (connection, &pid))
>+        goto fail;
>+
>+      if (!adjust_connections_for_cgroup (d->connections, &d->cgroup, 1))
>         goto fail;
>     }
> 
>@@ -1492,6 +1608,13 @@ bus_connections_check_limits (BusConnections  *connections,
>                               DBusError       *error)
> {
>   unsigned long uid;
>+  unsigned long pid;
>+  int count;
>+  BusConnectionData *d;
>+
>+  d = BUS_CONNECTION_DATA (requesting_completion);
>+  _dbus_assert (d != NULL);
>+
> 
>   if (connections->n_completed >=
>       bus_context_get_max_completed_connections (connections->context))
>@@ -1512,7 +1635,41 @@ bus_connections_check_limits (BusConnections  *connections,
>           return FALSE;
>         }
>     }
>-  
>+
>+  /* bus_connections_check_limits() could be called several times on the same
>+   * connection. Make sure to be idempotent. */
>+  if (!d->tracking_cgroup &&
>+      dbus_connection_get_unix_process_id (requesting_completion, &pid))
>+    {
>+      d->tracking_cgroup = TRUE;
>+
>+      if (!_dbus_cgroup_for_pid (pid, &d->cgroup, 4096, error))
>+        {
>+          _dbus_verbose ("Couldn't get connection's cgroup\n");
>+          dbus_error_free (error);
>+          /* ignore the error: the connection will be accounted for cgroup ""
>+           */
>+        }
>+    }
>+
>+  _dbus_verbose ("Checking max_connections_per_cgroup? %d\n", d->tracking_cgroup);
>+  if (d->tracking_cgroup)
>+    {
>+      _dbus_verbose ("Checking max_connections_per_cgroup: current count=%d max=%d\n",
>+                     get_connections_for_cgroup (connections, &d->cgroup),
>+                     bus_context_get_max_connections_per_cgroup (connections->context));
>+
>+      count = get_connections_for_cgroup (connections, &d->cgroup);
>+      if (count >=
>+          bus_context_get_max_connections_per_cgroup (connections->context))
>+        {
>+          dbus_set_error (error, DBUS_ERROR_LIMITS_EXCEEDED,
>+                          "The maximum number of active connections for cgroup %s has been reached",
>+                          _dbus_string_get_data (&d->cgroup));
>+          return FALSE;
>+        }
>+    }
>+
>   return TRUE;
> }
> 
>-- 
>1.8.5.3
>

Actions: View | Splinter Review

Attachments on bug 81469: 102994 | 102995 | 102996 | 104444 | 104445 | 104446 | 104447 | 104664 | 104665 | 104666

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.