kdbus: restructure name-registry to follow dbus-spec

The DBus Specification [1] is pretty clear about how name-acquisition,
queueing and releasing must work. Most of it's peculiarities nobody
relies on, but we better comply to them to at least allow proper
backwards compatibility via bus-proxy.

In particular, this means we don't implement the following details:
 * NameAcquire must update the stored flags of a name-owner if it is
   already queued or owns the name.
 * Only KDBUS_NAME_QUEUE and KDBUS_NAME_ALLOW_REPLACEMENT are stored
   flags of name owners. Everything else is only used during command
   execution and/or as reply flags.
 * NameAcquire on an already queued owner must not change the position in
   the queue.
 * KDBUS_NAME_REPLACE_EXISTING for already queued ownes *jumps* the queue
   if the primary owner has ALLOW_REPLACEMENT set.
 * If a primary owner is replaced by someone else, they must retain their
   stored name-flags.
 * If a primary owner is replaced by someone else, they must be placed at
   second position in the queue, if queuing is requested.

In dbus-daemon the name-owner queue is a single queue. That is, the
primary owner of a name is not special, instead, it simply is the first
queued owner. This explains most of the peculiarities of the NameAcquire
behavior and makes it much easier to implement them.

Hence, this patch rewrites the name-registry to follow the lead:
 * *ANY* name owner is now represented by a "struct kdbus_name_owner"
   objects, regardless whether they're queued, activators or primary.
 * All name-ownerships are linked in a *single* list on each connection.
   This gets rid of redundant conn->queued_names_list and
   conn->activator_of.
 * Limits and control functions always apply to 'struct kdbus_name_owner'
   now, instead  of 'struct kdbus_name_entry'. This solves some issues
   where name-ownership was properly limited, but name-queueing was not.
 * Activators are now correctly updated regarding KDBUS_NAME_IN_QUEUE
   depending whether they own the name or not.
 * owner->flags is now kept clean. Only real requested flags are stored,
   any operation-flags are cleared.
 * Special handling of activators is kept to a minimum. This really
   allows us to treat them more like real queued owners that allow
   replacement, than something special.

With this in place, we follow the dbus-spec regarding behavior of
name-acquisition perfectly. Hence, the bus-proxy can properly implement
backwards-compatibility to dbus1.

[1] http://dbus.freedesktop.org/doc/dbus-specification.html

Reviewed-by: Daniel Mack <daniel@zonque.org>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

ipc/kdbus/connection.c

@@ -123,9 +123,7 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep, bool privileged,
 #endif
 	mutex_init(&conn->lock);
 	INIT_LIST_HEAD(&conn->names_list);
-	INIT_LIST_HEAD(&conn->names_queue_list);
 	INIT_LIST_HEAD(&conn->reply_list);
-	atomic_set(&conn->name_count, 0);
 	atomic_set(&conn->request_count, 0);
 	atomic_set(&conn->lost_count, 0);
 	INIT_DELAYED_WORK(&conn->work, kdbus_reply_list_scan_work);
@@ -267,7 +265,6 @@ static void __kdbus_conn_free(struct kref *kref)
 	WARN_ON(delayed_work_pending(&conn->work));
 	WARN_ON(!list_empty(&conn->queue.msg_list));
 	WARN_ON(!list_empty(&conn->names_list));
-	WARN_ON(!list_empty(&conn->names_queue_list));
 	WARN_ON(!list_empty(&conn->reply_list));
 
 	if (conn->user) {
@@ -601,12 +598,13 @@ int kdbus_conn_disconnect(struct kdbus_conn *conn, bool ensure_queue_empty)
  */
 bool kdbus_conn_has_name(struct kdbus_conn *conn, const char *name)
 {
-	struct kdbus_name_entry *e;
+	struct kdbus_name_owner *owner;
 
 	lockdep_assert_held(&conn->ep->bus->name_registry->rwlock);
 
-	list_for_each_entry(e, &conn->names_list, conn_entry)
-		if (strcmp(e->name, name) == 0)
+	list_for_each_entry(owner, &conn->names_list, conn_entry)
+		if (!(owner->flags & KDBUS_NAME_IN_QUEUE) &&
+		    !strcmp(name, owner->name->name))
 			return true;
 
 	return false;
@@ -1035,6 +1033,7 @@ static int kdbus_pin_dst(struct kdbus_bus *bus,
 			 struct kdbus_conn **out_dst)
 {
 	const struct kdbus_msg *msg = staging->msg;
+	struct kdbus_name_owner *owner = NULL;
 	struct kdbus_name_entry *name = NULL;
 	struct kdbus_conn *dst = NULL;
 	int ret;
@@ -1053,7 +1052,9 @@ static int kdbus_pin_dst(struct kdbus_bus *bus,
 	} else {
 		name = kdbus_name_lookup_unlocked(bus->name_registry,
 						  staging->dst_name);
-		if (!name)
+		if (name)
+			owner = kdbus_name_get_owner(name);
+		if (!owner)
 			return -ESRCH;
 
 		/*
@@ -1065,19 +1066,14 @@ static int kdbus_pin_dst(struct kdbus_bus *bus,
 		 * owns the given name.
 		 */
 		if (msg->dst_id != KDBUS_DST_ID_NAME &&
-		    msg->dst_id != name->conn->id)
+		    msg->dst_id != owner->conn->id)
 			return -EREMCHG;
 
-		if (!name->conn && name->activator)
-			dst = kdbus_conn_ref(name->activator);
-		else
-			dst = kdbus_conn_ref(name->conn);
-
 		if ((msg->flags & KDBUS_MSG_NO_AUTO_START) &&
-		    kdbus_conn_is_activator(dst)) {
-			ret = -EADDRNOTAVAIL;
-			goto error;
-		}
+		    kdbus_conn_is_activator(owner->conn))
+			return -EADDRNOTAVAIL;
+
+		dst = kdbus_conn_ref(owner->conn);
 	}
 
 	*out_name = name;
@@ -1366,7 +1362,7 @@ static bool kdbus_conn_policy_query_all(struct kdbus_conn *conn,
 					struct kdbus_conn *whom,
 					unsigned int access)
 {
-	struct kdbus_name_entry *ne;
+	struct kdbus_name_owner *owner;
 	bool pass = false;
 	int res;
 
@@ -1375,10 +1371,14 @@ static bool kdbus_conn_policy_query_all(struct kdbus_conn *conn,
 	down_read(&db->entries_rwlock);
 	mutex_lock(&whom->lock);
 
-	list_for_each_entry(ne, &whom->names_list, conn_entry) {
-		res = kdbus_policy_query_unlocked(db, conn_creds ? : conn->cred,
-						  ne->name,
-						  kdbus_strhash(ne->name));
+	list_for_each_entry(owner, &whom->names_list, conn_entry) {
+		if (owner->flags & KDBUS_NAME_IN_QUEUE)
+			continue;
+
+		res = kdbus_policy_query_unlocked(db,
+					conn_creds ? : conn->cred,
+					owner->name->name,
+					kdbus_strhash(owner->name->name));
 		if (res >= (int)access) {
 			pass = true;
 			break;
@@ -1696,6 +1696,7 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
 	struct kdbus_meta_conn *conn_meta = NULL;
 	struct kdbus_pool_slice *slice = NULL;
 	struct kdbus_name_entry *entry = NULL;
+	struct kdbus_name_owner *owner = NULL;
 	struct kdbus_conn *owner_conn = NULL;
 	struct kdbus_item *meta_items = NULL;
 	struct kdbus_info info = {};
@@ -1732,15 +1733,17 @@ int kdbus_cmd_conn_info(struct kdbus_conn *conn, void __user *argp)
 
 	if (name) {
 		entry = kdbus_name_lookup_unlocked(bus->name_registry, name);
-		if (!entry || !entry->conn ||
+		if (entry)
+			owner = kdbus_name_get_owner(entry);
+		if (!owner ||
 		    !kdbus_conn_policy_see_name(conn, current_cred(), name) ||
-		    (cmd->id != 0 && entry->conn->id != cmd->id)) {
+		    (cmd->id != 0 && owner->conn->id != cmd->id)) {
 			/* pretend a name doesn't exist if you cannot see it */
 			ret = -ESRCH;
 			goto exit;
 		}
 
-		owner_conn = kdbus_conn_ref(entry->conn);
+		owner_conn = kdbus_conn_ref(owner->conn);
 	} else if (cmd->id > 0) {
 		owner_conn = kdbus_bus_find_conn_by_id(bus, cmd->id);
 		if (!owner_conn || !kdbus_conn_policy_see(conn, current_cred(),

ipc/kdbus/connection.h

@@ -30,6 +30,7 @@
 					 KDBUS_HELLO_POLICY_HOLDER | \
 					 KDBUS_HELLO_MONITOR)
 
+struct kdbus_name_entry;
 struct kdbus_quota;
 struct kdbus_staging;
 
@@ -61,7 +62,6 @@ struct kdbus_staging;
  * @cred:		The credentials of the connection at creation time
  * @pid:		Pid at creation time
  * @root_path:		Root path at creation time
- * @name_count:		Number of owned well-known names
  * @request_count:	Number of pending requests issued by this
  *			connection that are waiting for replies from
  *			other peers
@@ -70,9 +70,8 @@ struct kdbus_staging;
  * @queue:		The message queue associated with this connection
  * @quota:		Array of per-user quota indexed by user->id
  * @n_quota:		Number of elements in quota array
- * @activator_of:	Well-known name entry this connection acts as an
  * @names_list:		List of well-known names
- * @names_queue_list:	Well-known names this connection waits for
+ * @name_count:		Number of owned well-known names
  * @privileged:		Whether this connection is privileged on the bus
  */
 struct kdbus_conn {
@@ -101,7 +100,6 @@ struct kdbus_conn {
 	const struct cred *cred;
 	struct pid *pid;
 	struct path root_path;
-	atomic_t name_count;
 	atomic_t request_count;
 	atomic_t lost_count;
 	wait_queue_head_t wait;
@@ -111,9 +109,8 @@ struct kdbus_conn {
 	unsigned int n_quota;
 
 	/* protected by registry->rwlock */
-	struct kdbus_name_entry *activator_of;
 	struct list_head names_list;
-	struct list_head names_queue_list;
+	unsigned int name_count;
 
 	bool privileged:1;
 };

ipc/kdbus/metadata.c

@@ -603,17 +603,19 @@ static void kdbus_meta_conn_collect_timestamp(struct kdbus_meta_conn *mc,
 static int kdbus_meta_conn_collect_names(struct kdbus_meta_conn *mc,
 					 struct kdbus_conn *conn)
 {
-	const struct kdbus_name_entry *e;
+	const struct kdbus_name_owner *owner;
 	struct kdbus_item *item;
 	size_t slen, size;
 
 	lockdep_assert_held(&conn->ep->bus->name_registry->rwlock);
 
 	size = 0;
 	/* open-code length calculation to avoid final padding */
-	list_for_each_entry(e, &conn->names_list, conn_entry)
-		size = KDBUS_ALIGN8(size) + KDBUS_ITEM_HEADER_SIZE +
-			sizeof(struct kdbus_name) + strlen(e->name) + 1;
+	list_for_each_entry(owner, &conn->names_list, conn_entry)
+		if (!(owner->flags & KDBUS_NAME_IN_QUEUE))
+			size = KDBUS_ALIGN8(size) + KDBUS_ITEM_HEADER_SIZE +
+				sizeof(struct kdbus_name) +
+				strlen(owner->name->name) + 1;
 
 	if (!size)
 		return 0;
@@ -626,12 +628,15 @@ static int kdbus_meta_conn_collect_names(struct kdbus_meta_conn *mc,
 	mc->owned_names_items = item;
 	mc->owned_names_size = size;
 
-	list_for_each_entry(e, &conn->names_list, conn_entry) {
-		slen = strlen(e->name) + 1;
+	list_for_each_entry(owner, &conn->names_list, conn_entry) {
+		if (owner->flags & KDBUS_NAME_IN_QUEUE)
+			continue;
+
+		slen = strlen(owner->name->name) + 1;
 		kdbus_item_set(item, KDBUS_ITEM_OWNED_NAME, NULL,
 			       sizeof(struct kdbus_name) + slen);
-		item->name.flags = e->flags;
-		memcpy(item->name.name, e->name, slen);
+		item->name.flags = owner->flags;
+		memcpy(item->name.name, owner->name->name, slen);
 		item = KDBUS_ITEM_NEXT(item);
 	}