From 449d312f1540c18daa5ec3bd212a65254a8f7a5d Mon Sep 17 00:00:00 2001 From: Josh Boyer Date: Sat, 8 Aug 2015 18:21:51 -0400 Subject: [PATCH] Update kdbus to latest upstream. Commit a36324913ff2 --- kdbus.patch | 1882 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 1882 insertions(+) diff --git a/kdbus.patch b/kdbus.patch index 307d2fcbd..896e78169 100644 --- a/kdbus.patch +++ b/kdbus.patch @@ -49952,3 +49952,1885 @@ index 66ebb47370eb..fd4ac5adc6d2 100644 -- 2.4.3 +From 0e807c0b41b9e5a434f803a3ea0ba12ed316cbb7 Mon Sep 17 00:00:00 2001 +From: David Herrmann +Date: Fri, 7 Aug 2015 16:36:33 +0200 +Subject: [PATCH 1/4] 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 +Signed-off-by: David Herrmann +Signed-off-by: Greg Kroah-Hartman +--- + 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 aa3296ea4f93..ef63d6533273 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 8e0180ace3f6..1ad082014faa 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 d4973a90a81e..71ca475a80d5 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 057f8061c20f..7a6e61c35ebf 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(®->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(®->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(®->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(®->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(®->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(®->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(®->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(®->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(®->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 3dd2589293e0..edac59ddd8ee 100644 +--- a/ipc/kdbus/names.h ++++ b/ipc/kdbus/names.h +@@ -18,6 +18,10 @@ + #include + #include + ++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 + + +From 0486b859f05aa75d59e2c1a4f62053c1a1151d35 Mon Sep 17 00:00:00 2001 +From: David Herrmann +Date: Fri, 7 Aug 2015 16:36:34 +0200 +Subject: [PATCH 2/4] kdbus: inform caller about exact updates on NAME_ACQUIRE + +This adds two new return flags for KDBUS_CMD_NAME_ACQUIRE: + + * The KDBUS_NAME_PRIMARY flag is set for a name if, and only if, the + connection is currently the primary owner of a name. It is thus the + negation of KDBUS_NAME_IN_QUEUE, but is required to distinguish the + case from the situation where the connection is neither queued nor the + primary owner of a name. + Additionally, this flag is included in name listings and events. + + * The KDBUS_NAME_ACQUIRED flag is exclusively used as return flag for + KDBUS_CMD_NAME_ACQUIRE to let the caller know whether _this_ exact + call actually queued the connection on the name. If the flag is not + set, the connection was either already queued and only the flags were + updated, or the connection is not queued at all. + +This information was previously available to the caller via error-codes +from KDBUS_CMD_NAME_ACQUIRE. However, in some situations we actually +modify kernel state but return an error. This is considered bad style and +we really need to avoid this. Hence, these two new flags allow us to +avoid returning errors, but still inform the caller about the exact +conditions of the execution. + +Reviewed-by: Daniel Mack +Signed-off-by: David Herrmann +Signed-off-by: Greg Kroah-Hartman +--- + include/uapi/linux/kdbus.h | 4 ++++ + ipc/kdbus/names.c | 38 ++++++++++++++++++++++++++++---------- + 2 files changed, 32 insertions(+), 10 deletions(-) + +diff --git a/include/uapi/linux/kdbus.h b/include/uapi/linux/kdbus.h +index ecffc6b13c3e..4fc44cb1d4a8 100644 +--- a/include/uapi/linux/kdbus.h ++++ b/include/uapi/linux/kdbus.h +@@ -854,6 +854,8 @@ enum kdbus_make_flags { + * @KDBUS_NAME_QUEUE: Name should be queued if busy + * @KDBUS_NAME_IN_QUEUE: Name is queued + * @KDBUS_NAME_ACTIVATOR: Name is owned by a activator connection ++ * @KDBUS_NAME_PRIMARY: Primary owner of the name ++ * @KDBUS_NAME_ACQUIRED: Name was acquired/queued _now_ + */ + enum kdbus_name_flags { + KDBUS_NAME_REPLACE_EXISTING = 1ULL << 0, +@@ -861,6 +863,8 @@ enum kdbus_name_flags { + KDBUS_NAME_QUEUE = 1ULL << 2, + KDBUS_NAME_IN_QUEUE = 1ULL << 3, + KDBUS_NAME_ACTIVATOR = 1ULL << 4, ++ KDBUS_NAME_PRIMARY = 1ULL << 5, ++ KDBUS_NAME_ACQUIRED = 1ULL << 6, + }; + + /** +diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c +index 7a6e61c35ebf..a47ee5452158 100644 +--- a/ipc/kdbus/names.c ++++ b/ipc/kdbus/names.c +@@ -211,7 +211,8 @@ kdbus_name_lookup_unlocked(struct kdbus_name_registry *reg, const char *name) + return kdbus_name_entry_find(reg, kdbus_strhash(name), name); + } + +-static int kdbus_name_become_activator(struct kdbus_name_owner *owner) ++static int kdbus_name_become_activator(struct kdbus_name_owner *owner, ++ u64 *return_flags) + { + if (kdbus_name_owner_is_used(owner)) + return -EALREADY; +@@ -221,23 +222,30 @@ static int kdbus_name_become_activator(struct kdbus_name_owner *owner) + owner->name->activator = owner; + owner->flags |= KDBUS_NAME_ACTIVATOR; + +- if (kdbus_name_entry_first(owner->name)) ++ if (kdbus_name_entry_first(owner->name)) { + owner->flags |= KDBUS_NAME_IN_QUEUE; +- else ++ } else { ++ owner->flags |= KDBUS_NAME_PRIMARY; + kdbus_notify_name_change(owner->conn->ep->bus, + KDBUS_ITEM_NAME_ADD, + 0, owner->conn->id, + 0, owner->flags, + owner->name->name); ++ } ++ ++ if (return_flags) ++ *return_flags = owner->flags | KDBUS_NAME_ACQUIRED; + + return 0; + } + +-static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) ++static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags, ++ u64 *return_flags) + { + struct kdbus_name_owner *primary, *activator; + struct kdbus_name_entry *name; + struct kdbus_bus *bus; ++ u64 nflags = 0; + int ret = 0; + + name = owner->name; +@@ -259,6 +267,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + */ + + list_add(&owner->name_entry, &name->queue); ++ owner->flags |= KDBUS_NAME_PRIMARY; ++ nflags |= KDBUS_NAME_ACQUIRED; + + /* move messages to new owner on activation */ + if (activator) { +@@ -268,6 +278,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + activator->conn->id, owner->conn->id, + activator->flags, owner->flags, + name->name); ++ activator->flags &= ~KDBUS_NAME_PRIMARY; + activator->flags |= KDBUS_NAME_IN_QUEUE; + } else { + kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_ADD, +@@ -283,6 +294,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + * For compatibility, we have to return -EALREADY. + */ + ++ owner->flags |= KDBUS_NAME_PRIMARY; + ret = -EALREADY; + + } else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) && +@@ -295,6 +307,8 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + + list_del_init(&owner->name_entry); + list_add(&owner->name_entry, &name->queue); ++ owner->flags |= KDBUS_NAME_PRIMARY; ++ nflags |= KDBUS_NAME_ACQUIRED; + + kdbus_notify_name_change(bus, KDBUS_ITEM_NAME_CHANGE, + primary->conn->id, owner->conn->id, +@@ -303,6 +317,7 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + + /* requeue old primary, or drop if queueing not wanted */ + if (primary->flags & KDBUS_NAME_QUEUE) { ++ primary->flags &= ~KDBUS_NAME_PRIMARY; + primary->flags |= KDBUS_NAME_IN_QUEUE; + } else { + list_del_init(&primary->name_entry); +@@ -317,8 +332,10 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + */ + + owner->flags |= KDBUS_NAME_IN_QUEUE; +- if (!kdbus_name_owner_is_used(owner)) ++ if (!kdbus_name_owner_is_used(owner)) { + list_add_tail(&owner->name_entry, &name->queue); ++ nflags |= KDBUS_NAME_ACQUIRED; ++ } + } else if (kdbus_name_owner_is_used(owner)) { + /* + * Already queued on name, but re-queueing was not requested. +@@ -337,6 +354,9 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags) + ret = -EEXIST; + } + ++ if (return_flags) ++ *return_flags = owner->flags | nflags; ++ + return ret; + } + +@@ -392,15 +412,12 @@ int kdbus_name_acquire(struct kdbus_name_registry *reg, + } + + if (flags & KDBUS_NAME_ACTIVATOR) +- ret = kdbus_name_become_activator(owner); ++ ret = kdbus_name_become_activator(owner, return_flags); + else +- ret = kdbus_name_update(owner, flags); ++ ret = kdbus_name_update(owner, flags, return_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); +@@ -431,6 +448,7 @@ static void kdbus_name_release_unlocked(struct kdbus_name_owner *owner) + if (next) { + /* hand to next in queue */ + next->flags &= ~KDBUS_NAME_IN_QUEUE; ++ next->flags |= KDBUS_NAME_PRIMARY; + if (next == name->activator) + kdbus_conn_move_messages(next->conn, + owner->conn, +-- +2.4.3 + + +From 39b055e664cd7dd31a6de860f87f4728b4138590 Mon Sep 17 00:00:00 2001 +From: David Herrmann +Date: Fri, 7 Aug 2015 16:36:35 +0200 +Subject: [PATCH 3/4] kdbus: never return <0 from ioctls if we changed state + +If an ioctl() returns <0, user-space should be safe to assume it had no +effect on the state of any object. This might not be always possible, but +in kdbus we adhered to this rule. But there's one exception, namely +KDBUS_CMD_NAME_ACQUIRE. This call used to fail with -EALREADY if we owned +a name and tried to acquire it again. However, regardless whether the +name was already owned, the name-flags are updated according to the newly +provided flags. Hence, we change the state of name-ownership, but might +still return an error. + +This patch changes behavior and now returns 0 in those cases. User-space +still gets the same information (via return_flags), but will no longer be +told that the call failed. The tests reflect that and simply check for +KDBUS_NAME_ACQUIRED in 'return_flags'. + +Reviewed-by: Daniel Mack +Signed-off-by: David Herrmann +Signed-off-by: Greg Kroah-Hartman +--- + ipc/kdbus/names.c | 3 --- + tools/testing/selftests/kdbus/test-chat.c | 6 ++++-- + tools/testing/selftests/kdbus/test-names.c | 4 ---- + 3 files changed, 4 insertions(+), 9 deletions(-) + +diff --git a/ipc/kdbus/names.c b/ipc/kdbus/names.c +index a47ee5452158..bf44ca3f12b6 100644 +--- a/ipc/kdbus/names.c ++++ b/ipc/kdbus/names.c +@@ -291,11 +291,9 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags, + /* + * Already the primary owner of the name, flags were already + * updated. Nothing to do. +- * For compatibility, we have to return -EALREADY. + */ + + owner->flags |= KDBUS_NAME_PRIMARY; +- ret = -EALREADY; + + } else if ((primary->flags & KDBUS_NAME_ALLOW_REPLACEMENT) && + (flags & KDBUS_NAME_REPLACE_EXISTING)) { +@@ -344,7 +342,6 @@ static int kdbus_name_update(struct kdbus_name_owner *owner, u64 flags, + */ + + list_del_init(&owner->name_entry); +- ret = -EEXIST; + } else { + /* + * Name is already claimed and queueing is not requested. +diff --git a/tools/testing/selftests/kdbus/test-chat.c b/tools/testing/selftests/kdbus/test-chat.c +index 71a92d8b7c85..41e5b53fe0cc 100644 +--- a/tools/testing/selftests/kdbus/test-chat.c ++++ b/tools/testing/selftests/kdbus/test-chat.c +@@ -41,8 +41,10 @@ int kdbus_test_chat(struct kdbus_test_env *env) + ret = kdbus_name_acquire(conn_a, "foo.bar.double", NULL); + ASSERT_RETURN(ret == 0); + +- ret = kdbus_name_acquire(conn_a, "foo.bar.double", NULL); +- ASSERT_RETURN(ret == -EALREADY); ++ flags = 0; ++ ret = kdbus_name_acquire(conn_a, "foo.bar.double", &flags); ++ ASSERT_RETURN(ret == 0); ++ ASSERT_RETURN(!(flags & KDBUS_NAME_ACQUIRED)); + + ret = kdbus_name_release(conn_a, "foo.bar.double"); + ASSERT_RETURN(ret == 0); +diff --git a/tools/testing/selftests/kdbus/test-names.c b/tools/testing/selftests/kdbus/test-names.c +index fd4ac5adc6d2..9217465f3ff1 100644 +--- a/tools/testing/selftests/kdbus/test-names.c ++++ b/tools/testing/selftests/kdbus/test-names.c +@@ -143,10 +143,6 @@ int kdbus_test_name_conflict(struct kdbus_test_env *env) + ret = conn_is_name_owner(env->conn, name); + ASSERT_RETURN(ret == 0); + +- /* check that we can't acquire it again from the 1st connection */ +- ret = kdbus_name_acquire(env->conn, name, NULL); +- ASSERT_RETURN(ret == -EALREADY); +- + /* check that we also can't acquire it again from the 2nd connection */ + ret = kdbus_name_acquire(conn, name, NULL); + ASSERT_RETURN(ret == -EEXIST); +-- +2.4.3 + + +From a36324913ff21d7a0989c52b7208e8d738e17d64 Mon Sep 17 00:00:00 2001 +From: Daniel Mack +Date: Fri, 7 Aug 2015 16:36:36 +0200 +Subject: [PATCH 4/4] kdbus: selftests: add more name registry tests + +Add some more code for testing the name registry state. This can now be used +to track the state of queued names and per-name queing settings. + +Also add new tests to check the newly added KDBUS_NAME_PRIMARY and +KDBUS_NAME_ACQUIRED flags and name takeovers. + +Signed-off-by: Daniel Mack +Signed-off-by: Greg Kroah-Hartman +--- + tools/testing/selftests/kdbus/kdbus-test.c | 6 ++ + tools/testing/selftests/kdbus/kdbus-test.h | 1 + + tools/testing/selftests/kdbus/test-names.c | 133 +++++++++++++++++++++++------ + 3 files changed, 112 insertions(+), 28 deletions(-) + +diff --git a/tools/testing/selftests/kdbus/kdbus-test.c b/tools/testing/selftests/kdbus/kdbus-test.c +index db732e59650a..db57381570fa 100644 +--- a/tools/testing/selftests/kdbus/kdbus-test.c ++++ b/tools/testing/selftests/kdbus/kdbus-test.c +@@ -118,6 +118,12 @@ static const struct kdbus_test tests[] = { + .flags = TEST_CREATE_BUS | TEST_CREATE_CONN, + }, + { ++ .name = "name-takeover", ++ .desc = "takeover of names", ++ .func = kdbus_test_name_takeover, ++ .flags = TEST_CREATE_BUS | TEST_CREATE_CONN, ++ }, ++ { + .name = "message-basic", + .desc = "basic message handling", + .func = kdbus_test_message_basic, +diff --git a/tools/testing/selftests/kdbus/kdbus-test.h b/tools/testing/selftests/kdbus/kdbus-test.h +index a5c6ae81b81b..ee937f9a84dc 100644 +--- a/tools/testing/selftests/kdbus/kdbus-test.h ++++ b/tools/testing/selftests/kdbus/kdbus-test.h +@@ -72,6 +72,7 @@ int kdbus_test_monitor(struct kdbus_test_env *env); + int kdbus_test_name_basic(struct kdbus_test_env *env); + int kdbus_test_name_conflict(struct kdbus_test_env *env); + int kdbus_test_name_queue(struct kdbus_test_env *env); ++int kdbus_test_name_takeover(struct kdbus_test_env *env); + int kdbus_test_policy(struct kdbus_test_env *env); + int kdbus_test_policy_ns(struct kdbus_test_env *env); + int kdbus_test_policy_priv(struct kdbus_test_env *env); +diff --git a/tools/testing/selftests/kdbus/test-names.c b/tools/testing/selftests/kdbus/test-names.c +index 9217465f3ff1..e400dc86a2f5 100644 +--- a/tools/testing/selftests/kdbus/test-names.c ++++ b/tools/testing/selftests/kdbus/test-names.c +@@ -17,44 +17,68 @@ + #include "kdbus-enum.h" + #include "kdbus-test.h" + +-static int conn_is_name_owner(const struct kdbus_conn *conn, +- const char *needle) ++struct test_name { ++ const char *name; ++ __u64 owner_id; ++ __u64 flags; ++}; ++ ++static bool conn_test_names(const struct kdbus_conn *conn, ++ const struct test_name *tests, ++ unsigned int n_tests) + { +- struct kdbus_cmd_list cmd_list = { .size = sizeof(cmd_list) }; ++ struct kdbus_cmd_list cmd_list = {}; + struct kdbus_info *name, *list; +- bool found = false; ++ unsigned int i; + int ret; + +- cmd_list.flags = KDBUS_LIST_NAMES; ++ cmd_list.size = sizeof(cmd_list); ++ cmd_list.flags = KDBUS_LIST_NAMES | ++ KDBUS_LIST_ACTIVATORS | ++ KDBUS_LIST_QUEUED; + + ret = kdbus_cmd_list(conn->fd, &cmd_list); + ASSERT_RETURN(ret == 0); + + list = (struct kdbus_info *)(conn->buf + cmd_list.offset); +- KDBUS_FOREACH(name, list, cmd_list.list_size) { +- struct kdbus_item *item; +- const char *n = NULL; + +- KDBUS_ITEM_FOREACH(item, name, items) { +- if (item->type == KDBUS_ITEM_OWNED_NAME) { +- n = item->name.name; ++ for (i = 0; i < n_tests; i++) { ++ const struct test_name *t = tests + i; ++ bool found = false; ++ ++ KDBUS_FOREACH(name, list, cmd_list.list_size) { ++ struct kdbus_item *item; + +- if (name->id == conn->id && +- n && strcmp(needle, n) == 0) { ++ KDBUS_ITEM_FOREACH(item, name, items) { ++ if (item->type != KDBUS_ITEM_OWNED_NAME || ++ strcmp(item->name.name, t->name) != 0) ++ continue; ++ ++ if (t->owner_id == name->id && ++ t->flags == item->name.flags) { + found = true; + break; + } + } + } + +- if (found) +- break; ++ if (!found) ++ return false; + } + +- ret = kdbus_free(conn, cmd_list.offset); +- ASSERT_RETURN(ret == 0); ++ return true; ++} ++ ++static bool conn_is_name_primary_owner(const struct kdbus_conn *conn, ++ const char *needle) ++{ ++ struct test_name t = { ++ .name = needle, ++ .owner_id = conn->id, ++ .flags = KDBUS_NAME_PRIMARY, ++ }; + +- return found ? 0 : -1; ++ return conn_test_names(conn, &t, 1); + } + + int kdbus_test_name_basic(struct kdbus_test_env *env) +@@ -90,15 +114,15 @@ int kdbus_test_name_basic(struct kdbus_test_env *env) + ret = kdbus_name_acquire(env->conn, name, NULL); + ASSERT_RETURN(ret == 0); + +- ret = conn_is_name_owner(env->conn, name); +- ASSERT_RETURN(ret == 0); ++ ret = conn_is_name_primary_owner(env->conn, name); ++ ASSERT_RETURN(ret == true); + + /* ... and release it again */ + ret = kdbus_name_release(env->conn, name); + ASSERT_RETURN(ret == 0); + +- ret = conn_is_name_owner(env->conn, name); +- ASSERT_RETURN(ret != 0); ++ ret = conn_is_name_primary_owner(env->conn, name); ++ ASSERT_RETURN(ret == false); + + /* check that we can't release it again */ + ret = kdbus_name_release(env->conn, name); +@@ -140,8 +164,8 @@ int kdbus_test_name_conflict(struct kdbus_test_env *env) + ret = kdbus_name_acquire(env->conn, name, NULL); + ASSERT_RETURN(ret == 0); + +- ret = conn_is_name_owner(env->conn, name); +- ASSERT_RETURN(ret == 0); ++ ret = conn_is_name_primary_owner(env->conn, name); ++ ASSERT_RETURN(ret == true); + + /* check that we also can't acquire it again from the 2nd connection */ + ret = kdbus_name_acquire(conn, name, NULL); +@@ -155,13 +179,14 @@ int kdbus_test_name_conflict(struct kdbus_test_env *env) + int kdbus_test_name_queue(struct kdbus_test_env *env) + { + struct kdbus_conn *conn; ++ struct test_name t[2]; + const char *name; + uint64_t flags; + int ret; + + name = "foo.bla.blaz"; + +- flags = KDBUS_NAME_ALLOW_REPLACEMENT; ++ flags = 0; + + /* create a 2nd connection */ + conn = kdbus_hello(env->buspath, 0, NULL, 0); +@@ -172,8 +197,8 @@ int kdbus_test_name_queue(struct kdbus_test_env *env) + ret = kdbus_name_acquire(env->conn, name, &flags); + ASSERT_RETURN(ret == 0); + +- ret = conn_is_name_owner(env->conn, name); +- ASSERT_RETURN(ret == 0); ++ ret = conn_is_name_primary_owner(env->conn, name); ++ ASSERT_RETURN(ret == true); + + /* queue the 2nd connection as waiting owner */ + flags = KDBUS_NAME_QUEUE; +@@ -181,14 +206,66 @@ int kdbus_test_name_queue(struct kdbus_test_env *env) + ASSERT_RETURN(ret == 0); + ASSERT_RETURN(flags & KDBUS_NAME_IN_QUEUE); + ++ t[0].name = name; ++ t[0].owner_id = env->conn->id; ++ t[0].flags = KDBUS_NAME_PRIMARY; ++ t[1].name = name; ++ t[1].owner_id = conn->id; ++ t[1].flags = KDBUS_NAME_QUEUE | KDBUS_NAME_IN_QUEUE; ++ ret = conn_test_names(conn, t, 2); ++ ASSERT_RETURN(ret == true); ++ + /* release name from 1st connection */ + ret = kdbus_name_release(env->conn, name); + ASSERT_RETURN(ret == 0); + + /* now the name should be owned by the 2nd connection */ +- ret = conn_is_name_owner(conn, name); ++ t[0].name = name; ++ t[0].owner_id = conn->id; ++ t[0].flags = KDBUS_NAME_PRIMARY | KDBUS_NAME_QUEUE; ++ ret = conn_test_names(conn, t, 1); ++ ASSERT_RETURN(ret == true); ++ ++ kdbus_conn_free(conn); ++ ++ return TEST_OK; ++} ++ ++int kdbus_test_name_takeover(struct kdbus_test_env *env) ++{ ++ struct kdbus_conn *conn; ++ struct test_name t; ++ const char *name; ++ uint64_t flags; ++ int ret; ++ ++ name = "foo.bla.blaz"; ++ ++ flags = KDBUS_NAME_ALLOW_REPLACEMENT; ++ ++ /* create a 2nd connection */ ++ conn = kdbus_hello(env->buspath, 0, NULL, 0); ++ ASSERT_RETURN(conn != NULL); ++ ++ /* acquire name for 1st connection */ ++ ret = kdbus_name_acquire(env->conn, name, &flags); + ASSERT_RETURN(ret == 0); + ++ t.name = name; ++ t.owner_id = env->conn->id; ++ t.flags = KDBUS_NAME_ALLOW_REPLACEMENT | KDBUS_NAME_PRIMARY; ++ ret = conn_test_names(conn, &t, 1); ++ ASSERT_RETURN(ret == true); ++ ++ /* now steal name with 2nd connection */ ++ flags = KDBUS_NAME_REPLACE_EXISTING; ++ ret = kdbus_name_acquire(conn, name, &flags); ++ ASSERT_RETURN(ret == 0); ++ ASSERT_RETURN(flags & KDBUS_NAME_ACQUIRED); ++ ++ ret = conn_is_name_primary_owner(conn, name); ++ ASSERT_RETURN(ret == true); ++ + kdbus_conn_free(conn); + + return TEST_OK; +-- +2.4.3 +