[PATCH 1/4] kdbus: restructure name-registry to follow dbus-spec

From: Daniel Mack
Date: Fri Aug 07 2015 - 10:37:41 EST

From: David Herrmann <dh.herrmann@xxxxxxxxx>

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@xxxxxxxxxx>
Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx>
---
ipc/kdbus/connection.c | 53 ++--
ipc/kdbus/connection.h | 9 +-
ipc/kdbus/metadata.c | 21 +-
ipc/kdbus/names.c | 749 +++++++++++++++++++++++++++----------------------
ipc/kdbus/names.h | 55 +++-
5 files changed, 496 insertions(+), 391 deletions(-)

diff --git a/ipc/kdbus/connection.c b/ipc/kdbus/connection.c
index aa3296e..ef63d65 100644
--- a/ipc/kdbus/connection.c
+++ b/ipc/kdbus/connection.c
@@ -129,9 +129,7 @@ static struct kdbus_conn *kdbus_conn_new(struct kdbus_ep *ep,
#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);
@@ -284,7 +282,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) {
@@ -618,12 +615,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;
@@ -1052,6 +1050,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;
@@ -1070,7 +1069,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;

/*
@@ -1082,19 +1083,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;
@@ -1383,7 +1379,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;

@@ -1392,10 +1388,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;
@@ -1713,6 +1713,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 = {};
@@ -1749,15 +1750,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(),
diff --git a/ipc/kdbus/connection.h b/ipc/kdbus/connection.h
index 8e0180a..1ad0820 100644
--- a/ipc/kdbus/connection.h
+++ b/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 domain
* @owner: Owned by the same user as the bus owner
*/
@@ -102,7 +101,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;
@@ -112,9 +110,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;
bool owner:1;
diff --git a/ipc/kdbus/metadata.c b/ipc/kdbus/metadata.c
index d4973a9..71ca475 100644
--- a/ipc/kdbus/metadata.c
+++ b/ipc/kdbus/metadata.c
@@ -603,7 +603,7 @@ 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;

@@ -611,9 +611,11 @@ static int kdbus_meta_conn_collect_names(struct kdbus_meta_conn *mc,

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);
}

diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c
index 057f806..7a6e61c 100644
--- a/ipc/kdbus/names.c
+++ b/ipc/kdbus/names.c
@@ -34,167 +34,128 @@
#include "notify.h"
#include "policy.h"

-struct kdbus_name_pending {
- u64 flags;
- struct kdbus_conn *conn;
- struct kdbus_name_entry *name;
- struct list_head conn_entry;
- struct list_head name_entry;
-};
+#define KDBUS_NAME_SAVED_MASK (KDBUS_NAME_ALLOW_REPLACEMENT | \
+ KDBUS_NAME_QUEUE)

-static int kdbus_name_pending_new(struct kdbus_name_entry *e,
- struct kdbus_conn *conn, u64 flags)
+static bool kdbus_name_owner_is_used(struct kdbus_name_owner *owner)
{
- struct kdbus_name_pending *p;
-
- kdbus_conn_assert_active(conn);
-
- p = kmalloc(sizeof(*p), GFP_KERNEL);
- if (!p)
- return -ENOMEM;
-
- p->flags = flags;
- p->conn = conn;
- p->name = e;
- list_add_tail(&p->conn_entry, &conn->names_queue_list);
- list_add_tail(&p->name_entry, &e->queue);
-
- return 0;
+ return !list_empty(&owner->name_entry) ||
+ owner == owner->name->activator;
}

-static void kdbus_name_pending_free(struct kdbus_name_pending *p)
+static struct kdbus_name_owner *
+kdbus_name_owner_new(struct kdbus_conn *conn, struct kdbus_name_entry *name,
+ u64 flags)
{
- if (!p)
- return;
+ struct kdbus_name_owner *owner;

- list_del(&p->name_entry);
- list_del(&p->conn_entry);
- kfree(p);
-}
-
-static struct kdbus_name_entry *
-kdbus_name_entry_new(struct kdbus_name_registry *r, u32 hash, const char *name)
-{
- struct kdbus_name_entry *e;
- size_t namelen;
+ kdbus_conn_assert_active(conn);

- namelen = strlen(name);
+ if (conn->name_count >= KDBUS_CONN_MAX_NAMES)
+ return ERR_PTR(-E2BIG);

- e = kmalloc(sizeof(*e) + namelen + 1, GFP_KERNEL);
- if (!e)
+ owner = kmalloc(sizeof(*owner), GFP_KERNEL);
+ if (!owner)
return ERR_PTR(-ENOMEM);

- e->name_id = ++r->name_seq_last;
- e->flags = 0;
- e->conn = NULL;
- e->activator = NULL;
- INIT_LIST_HEAD(&e->queue);
- INIT_LIST_HEAD(&e->conn_entry);
- hash_add(r->entries_hash, &e->hentry, hash);
- memcpy(e->name, name, namelen + 1);
+ owner->flags = flags & KDBUS_NAME_SAVED_MASK;
+ owner->conn = conn;
+ owner->name = name;
+ list_add_tail(&owner->conn_entry, &conn->names_list);
+ INIT_LIST_HEAD(&owner->name_entry);

- return e;
+ ++conn->name_count;
+ return owner;
}

-static void kdbus_name_entry_free(struct kdbus_name_entry *e)
+static void kdbus_name_owner_free(struct kdbus_name_owner *owner)
{
- if (!e)
+ if (!owner)
return;

- WARN_ON(!list_empty(&e->conn_entry));
- WARN_ON(!list_empty(&e->queue));
- WARN_ON(e->activator);
- WARN_ON(e->conn);
-
- hash_del(&e->hentry);
- kfree(e);
+ WARN_ON(kdbus_name_owner_is_used(owner));
+ --owner->conn->name_count;
+ list_del(&owner->conn_entry);
+ kfree(owner);
}

-static void kdbus_name_entry_set_owner(struct kdbus_name_entry *e,
- struct kdbus_conn *conn, u64 flags)
+static struct kdbus_name_owner *
+kdbus_name_owner_find(struct kdbus_name_entry *name, struct kdbus_conn *conn)
{
- WARN_ON(e->conn);
+ struct kdbus_name_owner *owner;

- e->conn = kdbus_conn_ref(conn);
- e->flags = flags;
- atomic_inc(&conn->name_count);
- list_add_tail(&e->conn_entry, &e->conn->names_list);
+ /*
+ * Use conn->names_list over name->queue to make sure boundaries of
+ * this linear search are controlled by the connection itself.
+ * Furthermore, this will find normal owners as well as activators
+ * without any additional code.
+ */
+ list_for_each_entry(owner, &conn->names_list, conn_entry)
+ if (owner->name == name)
+ return owner;
+
+ return NULL;
}

-static void kdbus_name_entry_remove_owner(struct kdbus_name_entry *e)
+static bool kdbus_name_entry_is_used(struct kdbus_name_entry *name)
{
- WARN_ON(!e->conn);
-
- list_del_init(&e->conn_entry);
- atomic_dec(&e->conn->name_count);
- e->flags = 0;
- e->conn = kdbus_conn_unref(e->conn);
+ return !list_empty(&name->queue) || name->activator;
}

-static void kdbus_name_entry_replace_owner(struct kdbus_name_entry *e,
- struct kdbus_conn *conn, u64 flags)
+static struct kdbus_name_owner *
+kdbus_name_entry_first(struct kdbus_name_entry *name)
{
- if (WARN_ON(!e->conn) || WARN_ON(conn == e->conn))
- return;
-
- kdbus_notify_name_change(conn->ep->bus, KDBUS_ITEM_NAME_CHANGE,
- e->conn->id, conn->id,
- e->flags, flags, e->name);
- kdbus_name_entry_remove_owner(e);
- kdbus_name_entry_set_owner(e, conn, flags);
+ return list_first_entry_or_null(&name->queue, struct kdbus_name_owner,
+ name_entry);
}

-/**
- * kdbus_name_is_valid() - check if a name is valid
- * @p: The name to check
- * @allow_wildcard: Whether or not to allow a wildcard name
- *
- * A name is valid if all of the following criterias are met:
- *
- * - The name has two or more elements separated by a period ('.') character.
- * - All elements must contain at least one character.
- * - Each element must only contain the ASCII characters "[A-Z][a-z][0-9]_-"
- * and must not begin with a digit.
- * - The name must not exceed KDBUS_NAME_MAX_LEN.
- * - If @allow_wildcard is true, the name may end on '.*'
- */
-bool kdbus_name_is_valid(const char *p, bool allow_wildcard)
+static struct kdbus_name_entry *
+kdbus_name_entry_new(struct kdbus_name_registry *r, u32 hash,
+ const char *name_str)
{
- bool dot, found_dot = false;
- const char *q;
+ struct kdbus_name_entry *name;
+ size_t namelen;

- for (dot = true, q = p; *q; q++) {
- if (*q == '.') {
- if (dot)
- return false;
+ lockdep_assert_held(&r->rwlock);

- found_dot = true;
- dot = true;
- } else {
- bool good;
+ namelen = strlen(name_str);

- good = isalpha(*q) || (!dot && isdigit(*q)) ||
- *q == '_' || *q == '-' ||
- (allow_wildcard && dot &&
- *q == '*' && *(q + 1) == '\0');
+ name = kmalloc(sizeof(*name) + namelen + 1, GFP_KERNEL);
+ if (!name)
+ return ERR_PTR(-ENOMEM);

- if (!good)
- return false;
+ name->name_id = ++r->name_seq_last;
+ name->activator = NULL;
+ INIT_LIST_HEAD(&name->queue);
+ hash_add(r->entries_hash, &name->hentry, hash);
+ memcpy(name->name, name_str, namelen + 1);

- dot = false;
- }
- }
+ return name;
+}

- if (q - p > KDBUS_NAME_MAX_LEN)
- return false;
+static void kdbus_name_entry_free(struct kdbus_name_entry *name)
+{
+ if (!name)
+ return;

- if (dot)
- return false;
+ WARN_ON(kdbus_name_entry_is_used(name));
+ hash_del(&name->hentry);
+ kfree(name);
+}

- if (!found_dot)
- return false;
+static struct kdbus_name_entry *
+kdbus_name_entry_find(struct kdbus_name_registry *r, u32 hash,
+ const char *name_str)
+{
+ struct kdbus_name_entry *name;

- return true;
+ lockdep_assert_held(&r->rwlock);
+
+ hash_for_each_possible(r->entries_hash, name, hentry, hash)
+ if (!strcmp(name->name, name_str))
+ return name;
+
+ return NULL;
}

/**
@@ -218,32 +179,19 @@ struct kdbus_name_registry *kdbus_name_registry_new(void)
}

/**
- * kdbus_name_registry_free() - drop a name reg's reference
- * @reg: The name registry, may be %NULL
+ * kdbus_name_registry_free() - free name registry
+ * @r: name registry to free, or NULL
*
- * Cleanup the name registry's internal structures.
+ * Free a name registry and cleanup all internal objects. This is a no-op if
+ * you pass NULL as registry.
*/
-void kdbus_name_registry_free(struct kdbus_name_registry *reg)
+void kdbus_name_registry_free(struct kdbus_name_registry *r)
{
- if (!reg)
+ if (!r)
return;

- WARN_ON(!hash_empty(reg->entries_hash));
- kfree(reg);
-}
-
-static struct kdbus_name_entry *
-kdbus_name_find(struct kdbus_name_registry *reg, u32 hash, const char *name)
-{
- struct kdbus_name_entry *e;
-
- lockdep_assert_held(&reg->rwlock);
-
- hash_for_each_possible(reg->entries_hash, e, hentry, hash)
- if (strcmp(e->name, name) == 0)
- return e;
-
- return NULL;
+ WARN_ON(!hash_empty(r->entries_hash));
+ kfree(r);
}

/**
@@ -260,169 +208,271 @@ kdbus_name_find(struct kdbus_name_registry *reg, u32 hash, const char *name)
struct kdbus_name_entry *
kdbus_name_lookup_unlocked(struct kdbus_name_registry *reg, const char *name)
{
- return kdbus_name_find(reg, kdbus_strhash(name), name);
+ return kdbus_name_entry_find(reg, kdbus_strhash(name), name);
}

-/**
- * kdbus_name_acquire() - acquire a name
- * @reg: The name registry
- * @conn: The connection to pin this entry to
- * @name: The name to acquire
- * @flags: Acquisition flags (KDBUS_NAME_*)
- * @return_flags: Pointer to return flags for the acquired name
- * (KDBUS_NAME_*), may be %NULL
- *
- * Callers must ensure that @conn is either a privileged bus user or has
- * sufficient privileges in the policy-db to own the well-known name @name.
- *
- * Return: 0 success, negative error number on failure.
- */
-int kdbus_name_acquire(struct kdbus_name_registry *reg,
- struct kdbus_conn *conn, const char *name,
- u64 flags, u64 *return_flags)
+static int kdbus_name_become_activator(struct kdbus_name_owner *owner)
{
- struct kdbus_name_entry *e;
- u64 rflags = 0;
- int ret = 0;
- u32 hash;
-
- kdbus_conn_assert_active(conn);
+ if (kdbus_name_owner_is_used(owner))
+ return -EALREADY;
+ if (owner->name->activator)
+ return -EEXIST;
+
+ owner->name->activator = owner;
+ owner->flags |= KDBUS_NAME_ACTIVATOR;
+
+ if (kdbus_name_entry_first(owner->name))
+ owner->flags |= KDBUS_NAME_IN_QUEUE;
+ else
+ kdbus_notify_name_change(owner->conn->ep->bus,
+ KDBUS_ITEM_NAME_ADD,
+ 0, owner->conn->id,
+ 0, owner->flags,
+ owner->name->name);

- down_write(&reg->rwlock);
-
- if (!kdbus_conn_policy_own_name(conn, current_cred(), name)) {
- ret = -EPERM;
- goto exit_unlock;
- }
-
- hash = kdbus_strhash(name);
- e = kdbus_name_find(reg, hash, name);
- if (!e) {
- /* claim new name */
+ return 0;
+}

- if (conn->activator_of) {
- ret = -EINVAL;
- goto exit_unlock;
- }
+static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags)
+{
+ struct kdbus_name_owner *primary, *activator;
+ struct kdbus_name_entry *name;
+ struct kdbus_bus *bus;
+ int ret = 0;

- e = kdbus_name_entry_new(reg, hash, name);
- if (IS_ERR(e)) {
- ret = PTR_ERR(e);
- goto exit_unlock;
+ name = owner->name;
+ bus = owner->conn->ep->bus;
+ primary = kdbus_name_entry_first(name);
+ activator = name->activator;
+
+ /* cannot be activator and acquire a name */
+ if (owner == activator)
+ return -EUCLEAN;
+
+ /* update saved flags */
+ owner->flags = flags & KDBUS_NAME_SAVED_MASK;
+
+ if (!primary) {
+ /*
+ * No primary owner (but maybe an activator). Take over the
+ * name.
+ */
+
+ list_add(&owner->name_entry, &name->queue);
+
+ /* move messages to new owner on activation */
+ if (activator) {
+ kdbus_conn_move_messages(owner->conn, activator->conn,
+ name->name_id);
+ kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_CHANGE,
+ activator->conn->id, owner->conn->id,
+ activator->flags, owner->flags,
+ name->name);
+ activator->flags |= KDBUS_NAME_IN_QUEUE;
+ } else {
+ kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_ADD,
+ 0, owner->conn->id,
+ 0, owner->flags,
+ name->name);
}

- if (kdbus_conn_is_activator(conn)) {
- e->activator = kdbus_conn_ref(conn);
- conn->activator_of = e;
- }
+ } else if (owner == primary) {
+ /*
+ * Already the primary owner of the name, flags were already
+ * updated. Nothing to do.
+ * For compatibility, we have to return -EALREADY.
+ */

- kdbus_name_entry_set_owner(e, conn, flags);
- kdbus_notify_name_change(e->conn->ep->bus, KDBUS_ITEM_NAME_ADD,
- 0, e->conn->id, 0, e->flags, e->name);
- } else if (e->conn == conn || e == conn->activator_of) {
- /* connection already owns that name */
ret = -EALREADY;
- } else if (kdbus_conn_is_activator(conn)) {
- /* activator claims existing name */

- if (conn->activator_of) {
- ret = -EINVAL; /* multiple names not allowed */
- } else if (e->activator) {
- ret = -EEXIST; /* only one activator per name */
+ } else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) &&
+ (flags & KDBUS_NAME_REPLACE_EXISTING)) {
+ /*
+ * We're not the primary owner but can replace it. Move us
+ * ahead of the primary owner and acquire the name (possibly
+ * skipping queued owners ahead of us).
+ */
+
+ list_del_init(&owner->name_entry);
+ list_add(&owner->name_entry, &name->queue);
+
+ kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_CHANGE,
+ primary->conn->id, owner->conn->id,
+ primary->flags, owner->flags,
+ name->name);
+
+ /* requeue old primary, or drop if queueing not wanted */
+ if (primary->flags & KDBUS_NAME_QUEUE) {
+ primary->flags |= KDBUS_NAME_IN_QUEUE;
} else {
- e->activator = kdbus_conn_ref(conn);
- conn->activator_of = e;
- }
- } else if (e->flags & KDBUS_NAME_ACTIVATOR) {
- /* claim name of an activator */
-
- kdbus_conn_move_messages(conn, e->activator, 0);
- kdbus_name_entry_replace_owner(e, conn, flags);
- } else if ((flags & KDBUS_NAME_REPLACE_EXISTING) &&
- (e->flags & KDBUS_NAME_ALLOW_REPLACEMENT)) {
- /* claim name of a previous owner */
-
- if (e->flags & KDBUS_NAME_QUEUE) {
- /* move owner back to queue if they asked for it */
- ret = kdbus_name_pending_new(e, e->conn, e->flags);
- if (ret < 0)
- goto exit_unlock;
+ list_del_init(&primary->name_entry);
+ kdbus_name_owner_free(primary);
}

- kdbus_name_entry_replace_owner(e, conn, flags);
} else if (flags & KDBUS_NAME_QUEUE) {
- /* add to waiting-queue of the name */
-
- ret = kdbus_name_pending_new(e, conn, flags);
- if (ret >= 0)
- /* tell the caller that we queued it */
- rflags |= KDBUS_NAME_IN_QUEUE;
+ /*
+ * Name is already occupied and we cannot take it over, but
+ * queuing is allowed. Put us silently on the queue, if not
+ * already there.
+ */
+
+ owner->flags |= KDBUS_NAME_IN_QUEUE;
+ if (!kdbus_name_owner_is_used(owner))
+ list_add_tail(&owner->name_entry, &name->queue);
+ } else if (kdbus_name_owner_is_used(owner)) {
+ /*
+ * Already queued on name, but re-queueing was not requested.
+ * Make sure to unlink it from the name, the caller is
+ * responsible for releasing it.
+ */
+
+ list_del_init(&owner->name_entry);
+ ret = -EEXIST;
} else {
- /* the name is busy, return a failure */
+ /*
+ * Name is already claimed and queueing is not requested.
+ * Return error to the caller.
+ */
+
ret = -EEXIST;
}

- if (ret == 0 && return_flags)
- *return_flags = rflags;
+ return ret;
+}

-exit_unlock:
+int kdbus_name_acquire(struct kdbus_name_registry *reg,
+ struct kdbus_conn *conn, const char *name_str,
+ u64 flags, u64 *return_flags)
+{
+ struct kdbus_name_entry *name = NULL;
+ struct kdbus_name_owner *owner = NULL;
+ u32 hash;
+ int ret;
+
+ kdbus_conn_assert_active(conn);
+
+ down_write(&reg->rwlock);
+
+ /*
+ * Verify the connection has access to the name. Do this before testing
+ * for double-acquisitions and other errors to make sure we do not leak
+ * information about this name through possible custom endpoints.
+ */
+ if (!kdbus_conn_policy_own_name(conn, current_cred(), name_str)) {
+ ret = -EPERM;
+ goto exit;
+ }
+
+ /*
+ * Lookup the name entry. If it already exists, search for an owner
+ * entry as we might already own that name. If either does not exist,
+ * we will allocate a fresh one.
+ */
+ hash = kdbus_strhash(name_str);
+ name = kdbus_name_entry_find(reg, hash, name_str);
+ if (name) {
+ owner = kdbus_name_owner_find(name, conn);
+ } else {
+ name = kdbus_name_entry_new(reg, hash, name_str);
+ if (IS_ERR(name)) {
+ ret = PTR_ERR(name);
+ name = NULL;
+ goto exit;
+ }
+ }
+
+ /* create name owner object if not already queued */
+ if (!owner) {
+ owner = kdbus_name_owner_new(conn, name, flags);
+ if (IS_ERR(owner)) {
+ ret = PTR_ERR(owner);
+ owner = NULL;
+ goto exit;
+ }
+ }
+
+ if (flags & KDBUS_NAME_ACTIVATOR)
+ ret = kdbus_name_become_activator(owner);
+ else
+ ret = kdbus_name_update(owner, flags);
+ if (ret < 0)
+ goto exit;
+
+ if (return_flags)
+ *return_flags = owner->flags;
+
+exit:
+ if (owner && !kdbus_name_owner_is_used(owner))
+ kdbus_name_owner_free(owner);
+ if (name && !kdbus_name_entry_is_used(name))
+ kdbus_name_entry_free(name);
up_write(&reg->rwlock);
kdbus_notify_flush(conn->ep->bus);
return ret;
}

-static void kdbus_name_release_unlocked(struct kdbus_name_registry *reg,
- struct kdbus_name_entry *e)
+static void kdbus_name_release_unlocked(struct kdbus_name_owner *owner)
{
- struct kdbus_name_pending *p;
-
- lockdep_assert_held(&reg->rwlock);
-
- p = list_first_entry_or_null(&e->queue, struct kdbus_name_pending,
- name_entry);
-
- if (p) {
- /* give it to first active waiter in the queue */
- kdbus_name_entry_replace_owner(e, p->conn, p->flags);
- kdbus_name_pending_free(p);
- } else if (e->activator && e->activator != e->conn) {
- /* hand it back to an active activator connection */
- kdbus_conn_move_messages(e->activator, e->conn, e->name_id);
- kdbus_name_entry_replace_owner(e, e->activator,
- KDBUS_NAME_ACTIVATOR);
- } else {
- /* release the name */
- kdbus_notify_name_change(e->conn->ep->bus,
- KDBUS_ITEM_NAME_REMOVE,
- e->conn->id, 0, e->flags, 0, e->name);
- kdbus_name_entry_remove_owner(e);
- kdbus_name_entry_free(e);
+ struct kdbus_name_owner *primary, *next;
+ struct kdbus_name_entry *name;
+
+ name = owner->name;
+ primary = kdbus_name_entry_first(name);
+
+ list_del_init(&owner->name_entry);
+ if (owner == name->activator)
+ name->activator = NULL;
+
+ if (!primary || owner == primary) {
+ next = kdbus_name_entry_first(name);
+ if (!next)
+ next = name->activator;
+
+ if (next) {
+ /* hand to next in queue */
+ next->flags &= ~KDBUS_NAME_IN_QUEUE;
+ if (next == name->activator)
+ kdbus_conn_move_messages(next->conn,
+ owner->conn,
+ name->name_id);
+
+ kdbus_notify_name_change(owner->conn->ep->bus,
+ KDBUS_ITEM_NAME_CHANGE,
+ owner->conn->id, next->conn->id,
+ owner->flags, next->flags,
+ name->name);
+ } else {
+ kdbus_notify_name_change(owner->conn->ep->bus,
+ KDBUS_ITEM_NAME_REMOVE,
+ owner->conn->id, 0,
+ owner->flags, 0,
+ name->name);
+ }
}
+
+ kdbus_name_owner_free(owner);
+ if (!kdbus_name_entry_is_used(name))
+ kdbus_name_entry_free(name);
}

static int kdbus_name_release(struct kdbus_name_registry *reg,
struct kdbus_conn *conn,
- const char *name)
+ const char *name_str)
{
- struct kdbus_name_pending *p;
- struct kdbus_name_entry *e;
+ struct kdbus_name_owner *owner;
+ struct kdbus_name_entry *name;
int ret = 0;

down_write(&reg->rwlock);
- e = kdbus_name_find(reg, kdbus_strhash(name), name);
- if (!e) {
- ret = -ESRCH;
- } else if (e->conn == conn) {
- kdbus_name_release_unlocked(reg, e);
+ name = kdbus_name_entry_find(reg, kdbus_strhash(name_str), name_str);
+ if (name) {
+ owner = kdbus_name_owner_find(name, conn);
+ if (owner)
+ kdbus_name_release_unlocked(owner);
+ else
+ ret = -EADDRINUSE;
} else {
- ret = -EADDRINUSE;
- list_for_each_entry(p, &e->queue, name_entry) {
- if (p->conn == conn) {
- kdbus_name_pending_free(p);
- ret = 0;
- break;
- }
- }
+ ret = -ESRCH;
}
up_write(&reg->rwlock);

@@ -438,33 +488,74 @@ static int kdbus_name_release(struct kdbus_name_registry *reg,
void kdbus_name_release_all(struct kdbus_name_registry *reg,
struct kdbus_conn *conn)
{
- struct kdbus_name_pending *p;
- struct kdbus_conn *activator = NULL;
- struct kdbus_name_entry *e;
+ struct kdbus_name_owner *owner;

down_write(&reg->rwlock);

- if (conn->activator_of) {
- activator = conn->activator_of->activator;
- conn->activator_of->activator = NULL;
- }
-
- while ((p = list_first_entry_or_null(&conn->names_queue_list,
- struct kdbus_name_pending,
- conn_entry)))
- kdbus_name_pending_free(p);
- while ((e = list_first_entry_or_null(&conn->names_list,
- struct kdbus_name_entry,
- conn_entry)))
- kdbus_name_release_unlocked(reg, e);
+ while ((owner = list_first_entry_or_null(&conn->names_list,
+ struct kdbus_name_owner,
+ conn_entry)))
+ kdbus_name_release_unlocked(owner);

up_write(&reg->rwlock);

- kdbus_conn_unref(activator);
kdbus_notify_flush(conn->ep->bus);
}

/**
+ * kdbus_name_is_valid() - check if a name is valid
+ * @p: The name to check
+ * @allow_wildcard: Whether or not to allow a wildcard name
+ *
+ * A name is valid if all of the following criterias are met:
+ *
+ * - The name has two or more elements separated by a period ('.') character.
+ * - All elements must contain at least one character.
+ * - Each element must only contain the ASCII characters "[A-Z][a-z][0-9]_-"
+ * and must not begin with a digit.
+ * - The name must not exceed KDBUS_NAME_MAX_LEN.
+ * - If @allow_wildcard is true, the name may end on '.*'
+ */
+bool kdbus_name_is_valid(const char *p, bool allow_wildcard)
+{
+ bool dot, found_dot = false;
+ const char *q;
+
+ for (dot = true, q = p; *q; q++) {
+ if (*q == '.') {
+ if (dot)
+ return false;
+
+ found_dot = true;
+ dot = true;
+ } else {
+ bool good;
+
+ good = isalpha(*q) || (!dot && isdigit(*q)) ||
+ *q == '_' || *q == '-' ||
+ (allow_wildcard && dot &&
+ *q == '*' && *(q + 1) == '\0');
+
+ if (!good)
+ return false;
+
+ dot = false;
+ }
+ }
+
+ if (q - p > KDBUS_NAME_MAX_LEN)
+ return false;
+
+ if (dot)
+ return false;
+
+ if (!found_dot)
+ return false;
+
+ return true;
+}
+
+/**
* kdbus_cmd_name_acquire() - handle KDBUS_CMD_NAME_ACQUIRE
* @conn: connection to operate on
* @argp: command payload
@@ -503,20 +594,9 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp)
goto exit;
}

- /*
- * Do atomic_inc_return here to reserve our slot, then decrement
- * it before returning.
- */
- if (atomic_inc_return(&conn->name_count) > KDBUS_CONN_MAX_NAMES) {
- ret = -E2BIG;
- goto exit_dec;
- }
-
ret = kdbus_name_acquire(conn->ep->bus->name_registry, conn, item_name,
cmd->flags, &cmd->return_flags);

-exit_dec:
- atomic_dec(&conn->name_count);
exit:
return kdbus_args_clear(&args, ret);
}
@@ -559,7 +639,7 @@ static int kdbus_list_write(struct kdbus_conn *conn,
struct kdbus_conn *c,
struct kdbus_pool_slice *slice,
size_t *pos,
- struct kdbus_name_entry *e,
+ struct kdbus_name_owner *o,
bool write)
{
struct kvec kvec[4];
@@ -580,22 +660,22 @@ static int kdbus_list_write(struct kdbus_conn *conn,
u64 flags;
} h = {};

- if (e && !kdbus_conn_policy_see_name_unlocked(conn, current_cred(),
- e->name))
+ if (o && !kdbus_conn_policy_see_name_unlocked(conn, current_cred(),
+ o->name->name))
return 0;

kdbus_kvec_set(&kvec[cnt++], &info, sizeof(info), &info.size);

/* append name */
- if (e) {
- size_t slen = strlen(e->name) + 1;
+ if (o) {
+ size_t slen = strlen(o->name->name) + 1;

h.size = offsetof(struct kdbus_item, name.name) + slen;
h.type = KDBUS_ITEM_OWNED_NAME;
- h.flags = e->flags;
+ h.flags = o->flags;

kdbus_kvec_set(&kvec[cnt++], &h, sizeof(h), &info.size);
- kdbus_kvec_set(&kvec[cnt++], e->name, slen, &info.size);
+ kdbus_kvec_set(&kvec[cnt++], o->name->name, slen, &info.size);
cnt += !!kdbus_kvec_pad(&kvec[cnt], &info.size);
}

@@ -625,63 +705,52 @@ static int kdbus_list_all(struct kdbus_conn *conn, u64 flags,
if (kdbus_conn_is_monitor(c))
continue;

- /* skip activators */
- if (!(flags & KDBUS_LIST_ACTIVATORS) &&
- kdbus_conn_is_activator(c))
- continue;
-
/* all names the connection owns */
- if (flags & (KDBUS_LIST_NAMES | KDBUS_LIST_ACTIVATORS)) {
- struct kdbus_name_entry *e;
+ if (flags & (KDBUS_LIST_NAMES |
+ KDBUS_LIST_ACTIVATORS |
+ KDBUS_LIST_QUEUED)) {
+ struct kdbus_name_owner *o;

- list_for_each_entry(e, &c->names_list, conn_entry) {
- struct kdbus_conn *a = e->activator;
+ list_for_each_entry(o, &c->names_list, conn_entry) {
+ if (o->flags & KDBUS_NAME_ACTIVATOR) {
+ if (!(flags & KDBUS_LIST_ACTIVATORS))
+ continue;

- if ((flags & KDBUS_LIST_ACTIVATORS) &&
- a && a != c) {
- ret = kdbus_list_write(conn, a, slice,
- &p, e, write);
+ ret = kdbus_list_write(conn, c, slice,
+ &p, o, write);
if (ret < 0) {
mutex_unlock(&c->lock);
return ret;
}

added = true;
- }
+ } else if (o->flags & KDBUS_NAME_IN_QUEUE) {
+ if (!(flags & KDBUS_LIST_QUEUED))
+ continue;

- if (flags & KDBUS_LIST_NAMES ||
- kdbus_conn_is_activator(c)) {
ret = kdbus_list_write(conn, c, slice,
- &p, e, write);
+ &p, o, write);
if (ret < 0) {
mutex_unlock(&c->lock);
return ret;
}

added = true;
- }
- }
- }
+ } else if (flags & KDBUS_LIST_NAMES) {
+ ret = kdbus_list_write(conn, c, slice,
+ &p, o, write);
+ if (ret < 0) {
+ mutex_unlock(&c->lock);
+ return ret;
+ }

- /* queue of names the connection is currently waiting for */
- if (flags & KDBUS_LIST_QUEUED) {
- struct kdbus_name_pending *q;
-
- list_for_each_entry(q, &c->names_queue_list,
- conn_entry) {
- ret = kdbus_list_write(conn, c, slice, &p,
- q->name, write);
- if (ret < 0) {
- mutex_unlock(&c->lock);
- return ret;
+ added = true;
}
-
- added = true;
}
}

/* nothing added so far, just add the unique ID */
- if (!added && flags & KDBUS_LIST_UNIQUE) {
+ if (!added && (flags & KDBUS_LIST_UNIQUE)) {
ret = kdbus_list_write(conn, c, slice, &p, NULL, write);
if (ret < 0)
return ret;
diff --git a/ipc/kdbus/names.h b/ipc/kdbus/names.h
index 3dd2589..edac59d 100644
--- a/ipc/kdbus/names.h
+++ b/ipc/kdbus/names.h
@@ -18,6 +18,10 @@
#include <linux/hashtable.h>
#include <linux/rwsem.h>

+struct kdbus_name_entry;
+struct kdbus_name_owner;
+struct kdbus_name_registry;
+
/**
* struct kdbus_name_registry - names registered for a bus
* @entries_hash: Map of entries
@@ -32,27 +36,37 @@ struct kdbus_name_registry {

/**
* struct kdbus_name_entry - well-know name entry
- * @name_id: Sequence number of name entry to be able to uniquely
+ * @name_id: sequence number of name entry to be able to uniquely
* identify a name over its registration lifetime
- * @flags: KDBUS_NAME_* flags
- * @conn: Connection owning the name
- * @activator: Connection of the activator queuing incoming messages
- * @queue: List of queued connections
- * @conn_entry: Entry in connection
- * @hentry: Entry in registry map
- * @name: The well-known name
+ * @activator: activator of this name, or NULL
+ * @queue: list of queued owners
+ * @hentry: entry in registry map
+ * @name: well-known name
*/
struct kdbus_name_entry {
u64 name_id;
- u64 flags;
- struct kdbus_conn *conn;
- struct kdbus_conn *activator;
+ struct kdbus_name_owner *activator;
struct list_head queue;
- struct list_head conn_entry;
struct hlist_node hentry;
char name[];
};

+/**
+ * struct kdbus_name_owner - owner of a well-known name
+ * @flags: KDBUS_NAME_* flags of this owner
+ * @conn: connection owning the name
+ * @name: name that is owned
+ * @conn_entry: link into @conn
+ * @name_entry: link into @name
+ */
+struct kdbus_name_owner {
+ u64 flags;
+ struct kdbus_conn *conn;
+ struct kdbus_name_entry *name;
+ struct list_head conn_entry;
+ struct list_head name_entry;
+};
+
bool kdbus_name_is_valid(const char *p, bool allow_wildcard);

struct kdbus_name_registry *kdbus_name_registry_new(void);
@@ -71,4 +85,21 @@ int kdbus_cmd_name_acquire(struct kdbus_conn *conn, void __user *argp);
int kdbus_cmd_name_release(struct kdbus_conn *conn, void __user *argp);
int kdbus_cmd_list(struct kdbus_conn *conn, void __user *argp);

+/**
+ * kdbus_name_get_owner() - get current owner of a name
+ * @name: name to get current owner of
+ *
+ * This returns a pointer to the current owner of a name (or its activator if
+ * there is no owner). The caller must make sure @name is valid and does not
+ * vanish.
+ *
+ * Return: Pointer to current owner or NULL if there is none.
+ */
+static inline struct kdbus_name_owner *
+kdbus_name_get_owner(struct kdbus_name_entry *name)
+{
+ return list_first_entry_or_null(&name->queue, struct kdbus_name_owner,
+ name_entry) ? : name->activator;
+}
+
#endif
--
2.4.3


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/