names: fix kdbus_name_lookup() race condition

We cannot return pointers to the internal name registry from
kdbus_name_lookup(). No-one guarantees that these entries will stay valid.
Therefore, return the requested information directly with proper
references. This is protected by locks in kdbus_name_lookup() itself and
thus is safe.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

diff --git a/connection.c b/connection.c
index 7899a02f69449cb5cfbd39872e9430ffcd79f884..cffc02921f58d6acc1818b6174c790d6183cb3a6 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -780,25 +780,24 @@ static int kdbus_conn_get_conn_dst(struct kdbus_bus *bus,
                                   struct kdbus_conn **conn)
 {
        const struct kdbus_msg *msg = &kmsg->msg;
-       struct kdbus_conn *c;
+       struct kdbus_conn *c, *activator;
        u64 name_id = 0;
        int ret = 0;
 
        if (msg->dst_id == KDBUS_DST_ID_NAME) {
-               const struct kdbus_name_entry *name_entry;
-
                BUG_ON(!kmsg->dst_name);
-               name_entry = kdbus_name_lookup(bus->name_registry,
-                                              kmsg->dst_name);
-               if (!name_entry)
-                       return -ESRCH;
-
-               name_id = name_entry->name_id;
+               ret = kdbus_name_lookup(bus->name_registry,
+                                       kmsg->dst_name,
+                                       &c,
+                                       &activator,
+                                       &name_id);
+               if (ret < 0)
+                       return ret;
 
-               if (!name_entry->conn && name_entry->activator)
-                       c = kdbus_conn_ref(name_entry->activator);
+               if (!c && activator)
+                       c = activator;
                else
-                       c = kdbus_conn_ref(name_entry->conn);
+                       kdbus_conn_unref(activator);
 
                if ((msg->flags & KDBUS_MSG_FLAGS_NO_AUTO_START) &&
                    (c->flags & KDBUS_HELLO_ACTIVATOR)) {
@@ -1785,21 +1784,18 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn,
         * was already set above.
         */
        if (name) {
-               struct kdbus_name_entry *e;
-
                if (!kdbus_check_strlen(cmd_info, name)) {
                        ret = -EINVAL;
                        goto exit;
                }
 
-               e = kdbus_name_lookup(conn->bus->name_registry, name);
-               if (!e) {
-                       ret = -ESRCH;
+               ret = kdbus_name_lookup(conn->bus->name_registry,
+                                       name,
+                                       &owner_conn,
+                                       NULL,
+                                       NULL);
+               if (ret < 0)
                        goto exit;
-               }
-
-               if (e->conn)
-                       owner_conn = kdbus_conn_ref(e->conn);
        }
 
        if (!owner_conn) {

diff --git a/names.c b/names.c
index f4f56d898dd54e3b08def62c424b448108922813..1981efbf4234d9308a6e3186126356457adc38b6 100644 (file)
--- a/names.c
+++ b/names.c
@@ -313,20 +313,38 @@ void kdbus_name_remove_by_conn(struct kdbus_name_registry *reg,
  * kdbus_name_lookup() - look up a name in a name registry
  * @reg:               The name registry
  * @name:              The name to look up
+ * @conn:              Output for the connection owning the name or NULL
+ * @activator:         Output for the activator owning the name or NULL
+ * @name_id:           Output for the name-id or NULL
  *
- * Return: name entry if found, otherwise NULL.
+ * Search for a name in a given name registry. The found connections are stored
+ * with a new reference in the output stores. If either is not found, they're
+ * set to NULL.
+ *
+ * Return: 0 if either @conn or @activator was found, -ESRCH if nothing found.
  */
-struct kdbus_name_entry *kdbus_name_lookup(struct kdbus_name_registry *reg,
-                                          const char *name)
+int kdbus_name_lookup(struct kdbus_name_registry *reg,
+                     const char *name,
+                     struct kdbus_conn **conn,
+                     struct kdbus_conn **activator,
+                     u64 *name_id)
 {
        struct kdbus_name_entry *e = NULL;
        u32 hash = kdbus_str_hash(name);
 
        mutex_lock(&reg->lock);
        e = __kdbus_name_lookup(reg, hash, name);
+       if (e) {
+               if (conn)
+                       *conn = kdbus_conn_ref(e->conn);
+               if (activator)
+                       *activator = kdbus_conn_ref(e->activator);
+               if (name_id)
+                       *name_id = e->name_id;
+       }
        mutex_unlock(&reg->lock);
 
-       return e;
+       return e ? 0 : -ESRCH;
 }
 
 static int kdbus_name_queue_conn(struct kdbus_conn *conn, u64 flags,

diff --git a/names.h b/names.h
index 6e01cd1b693043d99f4d45a6acbf29c15653cf92..1d28f37d016863125da62b9f666556f44437abae 100644 (file)
--- a/names.h
+++ b/names.h
@@ -67,8 +67,11 @@ int kdbus_cmd_name_list(struct kdbus_name_registry *reg,
                        struct kdbus_conn *conn,
                        struct kdbus_cmd_name_list *cmd);
 
-struct kdbus_name_entry *kdbus_name_lookup(struct kdbus_name_registry *reg,
-                                          const char *name);
+int kdbus_name_lookup(struct kdbus_name_registry *reg,
+                     const char *name,
+                     struct kdbus_conn **conn,
+                     struct kdbus_conn **activator,
+                     u64 *name_id);
 void kdbus_name_remove_by_conn(struct kdbus_name_registry *reg,
                               struct kdbus_conn *conn);