connection: make conn->name_count atomic

Make conn->name_count an atomic type, so it can be checked safely later
when checking that the connection does really own names.

While we are it fix another count bug in kdbus_cmd_name_acquire() now we
register a slot by increment the counter before all operations, and we
decrement it before returning, this way we do not race for names and no
need to use complex locks.

Signed-off-by: Djalal Harouni <tixxdz@opendz.org>

diff --git a/connection.c b/connection.c
index 013329cc60ffb25ab456f0f6d48cfb4c77530471..ef71241155cc58746d192f654e49d5c3af3d138f 100644 (file)
--- a/connection.c
+++ b/connection.c
@@ -1520,6 +1520,7 @@ int kdbus_conn_new(struct kdbus_ep *ep,
        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->reply_count, 0);
        INIT_DELAYED_WORK(&conn->work, kdbus_conn_work);
        conn->cred = get_current_cred();

diff --git a/connection.h b/connection.h
index bcfd9faaec39103a18b5fb064ce0b9479f7a2dad..6a9c557024c3f8e42f398eaafdb3ffee01fab2cd 100644 (file)
--- a/connection.h
+++ b/connection.h
@@ -93,7 +93,7 @@ struct kdbus_conn {
        struct kdbus_pool *pool;
        struct kdbus_domain_user *user;
        const struct cred *cred;
-       size_t name_count;
+       atomic_t name_count;
        atomic_t reply_count;
        wait_queue_head_t wait;
        struct kdbus_queue queue;

diff --git a/names.c b/names.c
index 86c13734241f5de6066a24ec87323323267ff5c7..9b69f1332a57597e763963e3fa0c4efe7e95c20d 100644 (file)
--- a/names.c
+++ b/names.c
@@ -122,7 +122,7 @@ static void kdbus_name_entry_remove_owner(struct kdbus_name_entry *e)
        BUG_ON(!e->conn);
        BUG_ON(!mutex_is_locked(&e->conn->lock));
 
-       e->conn->name_count--;
+       atomic_dec(&e->conn->name_count);
        list_del(&e->conn_entry);
        e->conn = kdbus_conn_unref(e->conn);
 }
@@ -135,7 +135,7 @@ static void kdbus_name_entry_set_owner(struct kdbus_name_entry *e,
 
        e->conn = kdbus_conn_ref(conn);
        list_add_tail(&e->conn_entry, &e->conn->names_list);
-       conn->name_count++;
+       atomic_inc(&conn->name_count);
 }
 
 static int kdbus_name_replace_owner(struct kdbus_name_entry *e,
@@ -647,21 +647,29 @@ int kdbus_cmd_name_acquire(struct kdbus_name_registry *reg,
                           KDBUS_NAME_QUEUE))
                return -EOPNOTSUPP;
 
-       if (conn->name_count > KDBUS_CONN_MAX_NAMES)
-               return -E2BIG;
-
        ret = kdbus_items_get_str(cmd->items, KDBUS_ITEMS_SIZE(cmd, items),
                                  KDBUS_ITEM_NAME, &name);
        if (ret < 0)
                return -EINVAL;
 
+       /*
+        * Do atomic_inc_return here to reserve our slot, then decrement
+        * it before returning.
+        */
+       ret = -E2BIG;
+       if (atomic_inc_return(&conn->name_count) > KDBUS_CONN_MAX_NAMES)
+               goto out_dec;
+
        ret = kdbus_ep_policy_check_own_access(conn->ep, conn, name);
        if (ret < 0)
-               return ret;
+               goto out_dec;
 
        ret = kdbus_name_acquire(reg, conn, name, &cmd->flags, &e);
        kdbus_notify_flush(conn->bus);
 
+out_dec:
+       /* Decrement the previous allocated slot */
+       atomic_dec(&conn->name_count);
        return ret;
 }