Zbigniew Jędrzejewski-Szmek 62fe94
From 943c3f94e2f8b8b35ef6a40220bbe4c06510930c Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek 62fe94
From: David Herrmann <dh.herrmann@gmail.com>
Zbigniew Jędrzejewski-Szmek 62fe94
Date: Wed, 17 Sep 2014 09:28:09 +0200
Zbigniew Jędrzejewski-Szmek 62fe94
Subject: [PATCH] bus: never respond to GetManagedObjects() on sub-paths
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
The dbus-spec clearly specifies that GetManagedObjects() should only work
Zbigniew Jędrzejewski-Szmek 62fe94
on the root-path of an object-tree. But on that path, it works regardless
Zbigniew Jędrzejewski-Szmek 62fe94
whether there are any objects available or not.
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
We could, technically, define all sub-paths as a root-path of its own
Zbigniew Jędrzejewski-Szmek 62fe94
sub-tree. However, if we do that, we enter undefined territory:
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
    Imagine only a fallback vtable is registered. We want
Zbigniew Jędrzejewski-Szmek 62fe94
    GetManagedObjects() to *NOT* fail with UNKNOWN_METHOD if it is called
Zbigniew Jędrzejewski-Szmek 62fe94
    on a valid sub-tree of the fallback. On the other hand, we don't want
Zbigniew Jędrzejewski-Szmek 62fe94
    it to work on arbitrary sub-tree. Something like:
Zbigniew Jędrzejewski-Szmek 62fe94
        /path/to/fallback/foobar/foobar/foobar/invalid/foobar
Zbigniew Jędrzejewski-Szmek 62fe94
    should not work.
Zbigniew Jędrzejewski-Szmek 62fe94
    However, there is no way to know which paths on a fallback are valid
Zbigniew Jędrzejewski-Szmek 62fe94
    without looking at there registered objects. If no objects are
Zbigniew Jędrzejewski-Szmek 62fe94
    registered, we have no way to figure it out.
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
Therefore, we now try to follow the dbus spec by only returning valid data
Zbigniew Jędrzejewski-Szmek 62fe94
on registered root-paths. We treat each path as root which was registered
Zbigniew Jędrzejewski-Szmek 62fe94
an object-manager on via add_object_manager(). So applications can now
Zbigniew Jędrzejewski-Szmek 62fe94
directly control which paths to place an object-manager on.
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
We also fix the introspection to not return object-manager interfaces on
Zbigniew Jędrzejewski-Szmek 62fe94
non-root paths.
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
Also fixes some dead-code paths initially reported by Philippe De Swert.
Zbigniew Jędrzejewski-Szmek 62fe94
---
Zbigniew Jędrzejewski-Szmek 62fe94
 src/libsystemd/sd-bus/bus-objects.c | 63 ++++++++-----------------------------
Zbigniew Jędrzejewski-Szmek 62fe94
 1 file changed, 13 insertions(+), 50 deletions(-)
Zbigniew Jędrzejewski-Szmek 62fe94
Zbigniew Jędrzejewski-Szmek 62fe94
diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c
Zbigniew Jędrzejewski-Szmek 62fe94
index 81c840f129..ecac73a94d 100644
Zbigniew Jędrzejewski-Szmek 62fe94
--- a/src/libsystemd/sd-bus/bus-objects.c
Zbigniew Jędrzejewski-Szmek 62fe94
+++ b/src/libsystemd/sd-bus/bus-objects.c
Zbigniew Jędrzejewski-Szmek 62fe94
@@ -820,19 +820,6 @@ static int property_get_all_callbacks_run(
Zbigniew Jędrzejewski-Szmek 62fe94
         return 1;
Zbigniew Jędrzejewski-Szmek 62fe94
 }
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
-static bool bus_node_with_object_manager(sd_bus *bus, struct node *n) {
Zbigniew Jędrzejewski-Szmek 62fe94
-        assert(bus);
Zbigniew Jędrzejewski-Szmek 62fe94
-        assert(n);
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-        if (n->object_managers)
Zbigniew Jędrzejewski-Szmek 62fe94
-                return true;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-        if (n->parent)
Zbigniew Jędrzejewski-Szmek 62fe94
-                return bus_node_with_object_manager(bus, n->parent);
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-        return false;
Zbigniew Jędrzejewski-Szmek 62fe94
-}
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
 static bool bus_node_exists(
Zbigniew Jędrzejewski-Szmek 62fe94
                 sd_bus *bus,
Zbigniew Jędrzejewski-Szmek 62fe94
                 struct node *n,
Zbigniew Jędrzejewski-Szmek 62fe94
@@ -902,7 +889,7 @@ static int process_introspect(
Zbigniew Jędrzejewski-Szmek 62fe94
         if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
                 return r;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
-        r = introspect_write_default_interfaces(&intro, bus_node_with_object_manager(bus, n));
Zbigniew Jędrzejewski-Szmek 62fe94
+        r = introspect_write_default_interfaces(&intro, !require_fallback && n->object_managers);
Zbigniew Jędrzejewski-Szmek 62fe94
         if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
                 return r;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
@@ -1142,7 +1129,8 @@ static int process_get_managed_objects(
Zbigniew Jędrzejewski-Szmek 62fe94
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
Zbigniew Jędrzejewski-Szmek 62fe94
         _cleanup_bus_message_unref_ sd_bus_message *reply = NULL;
Zbigniew Jędrzejewski-Szmek 62fe94
         _cleanup_set_free_free_ Set *s = NULL;
Zbigniew Jędrzejewski-Szmek 62fe94
-        bool empty;
Zbigniew Jędrzejewski-Szmek 62fe94
+        Iterator i;
Zbigniew Jędrzejewski-Szmek 62fe94
+        char *path;
Zbigniew Jędrzejewski-Szmek 62fe94
         int r;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
         assert(bus);
Zbigniew Jędrzejewski-Szmek 62fe94
@@ -1150,7 +1138,11 @@ static int process_get_managed_objects(
Zbigniew Jędrzejewski-Szmek 62fe94
         assert(n);
Zbigniew Jędrzejewski-Szmek 62fe94
         assert(found_object);
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
-        if (!bus_node_with_object_manager(bus, n))
Zbigniew Jędrzejewski-Szmek 62fe94
+        /* Spec says, GetManagedObjects() is only implemented on the root of a
Zbigniew Jędrzejewski-Szmek 62fe94
+         * sub-tree. Therefore, we require a registered object-manager on
Zbigniew Jędrzejewski-Szmek 62fe94
+         * exactly the queried path, otherwise, we refuse to respond. */
Zbigniew Jędrzejewski-Szmek 62fe94
+
Zbigniew Jędrzejewski-Szmek 62fe94
+        if (require_fallback || !n->object_managers)
Zbigniew Jędrzejewski-Szmek 62fe94
                 return 0;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
         r = get_child_nodes(bus, m->path, n, &s, &error);
Zbigniew Jędrzejewski-Szmek 62fe94
@@ -1167,42 +1159,13 @@ static int process_get_managed_objects(
Zbigniew Jędrzejewski-Szmek 62fe94
         if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
                 return r;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
-        empty = set_isempty(s);
Zbigniew Jędrzejewski-Szmek 62fe94
-        if (empty) {
Zbigniew Jędrzejewski-Szmek 62fe94
-                struct node_vtable *c;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                /* Hmm, so we have no children? Then let's check
Zbigniew Jędrzejewski-Szmek 62fe94
-                 * whether we exist at all, i.e. whether at least one
Zbigniew Jędrzejewski-Szmek 62fe94
-                 * vtable exists. */
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                LIST_FOREACH(vtables, c, n->vtables) {
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                        if (require_fallback && !c->is_fallback)
Zbigniew Jędrzejewski-Szmek 62fe94
-                                continue;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                        if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
-                                return r;
Zbigniew Jędrzejewski-Szmek 62fe94
-                        if (r == 0)
Zbigniew Jędrzejewski-Szmek 62fe94
-                                continue;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                        empty = false;
Zbigniew Jędrzejewski-Szmek 62fe94
-                        break;
Zbigniew Jędrzejewski-Szmek 62fe94
-                }
Zbigniew Jędrzejewski-Szmek 62fe94
+        SET_FOREACH(path, s, i) {
Zbigniew Jędrzejewski-Szmek 62fe94
+                r = object_manager_serialize_path_and_fallbacks(bus, reply, path, &error);
Zbigniew Jędrzejewski-Szmek 62fe94
+                if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
+                        return r;
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
-                if (empty)
Zbigniew Jędrzejewski-Szmek 62fe94
+                if (bus->nodes_modified)
Zbigniew Jędrzejewski-Szmek 62fe94
                         return 0;
Zbigniew Jędrzejewski-Szmek 62fe94
-        } else {
Zbigniew Jędrzejewski-Szmek 62fe94
-                Iterator i;
Zbigniew Jędrzejewski-Szmek 62fe94
-                char *path;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                SET_FOREACH(path, s, i) {
Zbigniew Jędrzejewski-Szmek 62fe94
-                        r = object_manager_serialize_path_and_fallbacks(bus, reply, path, &error);
Zbigniew Jędrzejewski-Szmek 62fe94
-                        if (r < 0)
Zbigniew Jędrzejewski-Szmek 62fe94
-                                return r;
Zbigniew Jędrzejewski-Szmek 62fe94
-
Zbigniew Jędrzejewski-Szmek 62fe94
-                        if (bus->nodes_modified)
Zbigniew Jędrzejewski-Szmek 62fe94
-                                return 0;
Zbigniew Jędrzejewski-Szmek 62fe94
-                }
Zbigniew Jędrzejewski-Szmek 62fe94
         }
Zbigniew Jędrzejewski-Szmek 62fe94
 
Zbigniew Jędrzejewski-Szmek 62fe94
         r = sd_bus_message_close_container(reply);