Attachment 79929 Details for Bug 50199 – When "activating" systemd, handle its special case better

[patch] When "activating" systemd, handle its special case better

0001-When-activating-systemd-handle-its-special-case-bett.patch (text/plain), 4.73 KB, created by Chengwei Yang on 2013-05-29 01:13:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chengwei Yang

Created: 2013-05-29 01:13:23 UTC

Size: 4.73 KB

patch

obsolete

>From d07b90e20f230e605754710c2a91d319e3e1a2de Mon Sep 17 00:00:00 2001
>From: Chengwei Yang <chengwei.yang@intel.com>
>Date: Tue, 28 May 2013 21:02:05 +0800
>Subject: [PATCH v3] When "activating" systemd, handle its special case better
>
>When dbus-daemon receives a request to activate a systemd service before
>systemd has connected to it, it enqueues a fake request to "activate"
>systemd itself (as a way to get a BusPendingActivationEntry to track the
>process of waiting for systemd). When systemd later joins the bus,
>dbus-daemon sends the actual activation message; any future activation
>messages are sent directly to systemd.
>
>In the "pending" code path, the activation messages are currently
>dispatched as though they had been sent by the same process that sent
>the original activation request, which is wrong: the bus security
>policy probably doesn't allow that process to talk to systemd directly.
>They should be dispatched as though they had been sent by the
>dbus-daemon itself (connection == NULL), the same as in the non-pending
>code path.
>
>In the worst case, if the attempt to activate systemd timed out, the
>dbus-daemon would crash with a (fatal) warning, because in this special
>case, activation_message is a signal with no serial number, whereas the
>code to send an error reply is expecting a method call with a serial
>number.
>
>It looks as though try_send_activation_failure() still doesn't handle
>the (entry->connection == NULL) case. As far as I can tell, it should
>probably be:
>
>-    if (dbus_connection_get_is_connected (entry->connection))
>+    if (entry->connection != NULL && dbus_connection_get_is_connected
>(entry->connection))
>
>because if the connection is NULL, the message came from dbus-daemon
>itself (and is a signal, not a method call, so replying to it would be
>an error).
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=50199
>
>Signed-off-by: Chengwei Yang <chengwei.yang@intel.com>
>Tested-by: Ma Yu <yu.ma@intel.com>
>---
> bus/activation.c |   17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
>diff --git a/bus/activation.c b/bus/activation.c
>index 3dfba78..8de3997 100644
>--- a/bus/activation.c
>+++ b/bus/activation.c
>@@ -80,7 +80,14 @@ typedef struct BusPendingActivationEntry BusPendingActivationEntry;
> 
> struct BusPendingActivationEntry
> {
>+  /* Normally a method call, but if connection is NULL, this is a signal
>+   * instead.
>+   */
>   DBusMessage *activation_message;
>+  /* NULL if this activation entry is for the dbus-daemon itself,
>+   * waiting for systemd to start. In this case, auto_activation is always
>+   * TRUE.
>+   */
>   DBusConnection *connection;
> 
>   dbus_bool_t auto_activation;
>@@ -1106,7 +1113,8 @@ bus_activation_service_created (BusActivation  *activation,
>       BusPendingActivationEntry *entry = link->data;
>       DBusList *next = _dbus_list_get_next_link (&pending_activation->entries, link);
> 
>-      if (dbus_connection_get_is_connected (entry->connection))
>+      /* entry->connection is NULL for activating systemd */
>+      if (entry->connection && dbus_connection_get_is_connected (entry->connection))
>         {
>           /* Only send activation replies to regular activation requests. */
>           if (!entry->auto_activation)
>@@ -1175,7 +1183,7 @@ bus_activation_send_pending_auto_activation_messages (BusActivation  *activation
>       BusPendingActivationEntry *entry = link->data;
>       DBusList *next = _dbus_list_get_next_link (&pending_activation->entries, link);
> 
>-      if (entry->auto_activation && dbus_connection_get_is_connected (entry->connection))
>+      if (entry->auto_activation && (entry->connection == NULL || dbus_connection_get_is_connected (entry->connection)))
>         {
>           DBusConnection *addressed_recipient;
> 
>@@ -1759,7 +1767,8 @@ bus_activation_activate_service (BusActivation  *activation,
>   pending_activation_entry->activation_message = activation_message;
>   dbus_message_ref (activation_message);
>   pending_activation_entry->connection = connection;
>-  dbus_connection_ref (connection);
>+  if (connection)
>+    dbus_connection_ref (connection);
> 
>   /* Check if the service is being activated */
>   pending_activation = _dbus_hash_table_lookup_string (activation->pending_activations, service_name);
>@@ -1972,7 +1981,7 @@ bus_activation_activate_service (BusActivation  *activation,
>                                service_name,
>                                entry->systemd_service);
>               /* systemd is not around, let's "activate" it. */
>-              retval = bus_activation_activate_service (activation, connection, activation_transaction, TRUE,
>+              retval = bus_activation_activate_service (activation, NULL, activation_transaction, TRUE,
>                                                         message, "org.freedesktop.systemd1", error);
>             }
> 
>-- 
>1.7.9.5
>

Actions: View | Splinter Review

Attachments on bug 50199: 61951 | 79841 | 79929 | 79961

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.