[systemd-devel] FW: [PATCH] names: clear e->activator when activator has disconnected

[Daniel Mack]

On 02/24/2014 12:30 PM, Radoslaw Pajak wrote:
> Your patch works if the activator connection is still the owner of
> the name, but it doesn't work if activator has lost the ownership. If
> regular connection takes over the name than kdbus_name_replace_owner
> calls kdbus_name_entry_remove_owner which removes e->conn_entry from
> the names list of the activator connection, which you wanted to use.

Yeah, you're right of course.

> If activator connection has closed than, there is stil e->activator
> pointer to this non existing connection which is bug by itself and
> new activator connection cannot acquire that name. Without our patch
> there is no link between the activator connection and name_entry.

Yes, but there's one more pointer that can dangle :)

As we're not on a fast path here, I think we can really walk the hash of
all names in the registry of the bus, and clean up the ->activator
pointer. Which should also fix the bug you mentioned above.

What about this one instead?


Thanks,
Daniel


diff --git a/names.c b/names.c
index 341f9e9..b430cf8 100644
--- a/names.c
+++ b/names.c
@@ -243,6 +243,7 @@ void kdbus_name_remove_by_conn(struct
kdbus_name_registry *reg,
        LIST_HEAD(names_queue_list);
        LIST_HEAD(notify_list);
        LIST_HEAD(names_list);
+       int i;

        mutex_lock(&conn->lock);
        list_splice_init(&conn->names_list, &names_list);
@@ -251,10 +252,17 @@ void kdbus_name_remove_by_conn(struct
kdbus_name_registry *reg,

        mutex_lock(&conn->bus->lock);
        mutex_lock(&reg->entries_lock);
+
        list_for_each_entry_safe(q, q_tmp, &names_queue_list, conn_entry)
                kdbus_name_queue_item_free(q);
+
        list_for_each_entry_safe(e, e_tmp, &names_list, conn_entry)
                kdbus_name_entry_release(e, &notify_list);
+
+       hash_for_each(reg->entries_hash, i, e, hentry)
+               if (conn == e->activator)
+                       e->activator = kdbus_conn_unref(conn);
+
        mutex_unlock(&reg->entries_lock);
        mutex_unlock(&conn->bus->lock);