policy: make kdbus_policy_check_see_access() unlocked

We have a race condition in names, because kdbus_name_list_write() is
called twice - once for gathering the size of the resulting list, and
once for the actual write.

We need to make sure that the policy doesn't make different decisions in
both cases, so we have to hold the policy db's entries_lock around all
calls of kdbus_name_list_all().

For this, struct kdbus_policy_db has to be made public. Also, rename
kdbus_policy_check_see_access() to
kdbus_policy_check_see_access_unlocked().

3 files changed, 37 insertions, 26 deletions

diff --git a/names.c b/names.c
index daa13b91e0b..b1702839588 100644
--- a/names.c
+++ b/names.c
@@ -691,10 +691,15 @@ static int kdbus_name_list_write(struct kdbus_conn *conn,
 		 * Note that policy DBs instanciated along with connections
 		 * don't have SEE rules, so it's sufficient to check the
 		 * endpoint's database.
+		 *
+		 * The lock for the policy db is held across all calls of
+		 * kdbus_name_list_all(), so the entries in both writing
+		 * and non-writing runs of kdbus_name_list_write() are the
+		 * same.
 		 */
 		if (conn->ep->policy_db &&
-		    kdbus_policy_check_see_access(conn->ep->policy_db,
-						  e->name) < 0)
+		    kdbus_policy_check_see_access_unlocked(conn->ep->policy_db,
+							   e->name) < 0)
 				return 0;
 	}
 
@@ -812,13 +817,19 @@ int kdbus_cmd_name_list(struct kdbus_name_registry *reg,
 			struct kdbus_conn *conn,
 			struct kdbus_cmd_name_list *cmd)
 {
+	struct kdbus_policy_db *policy_db;
 	struct kdbus_name_list list = {};
 	size_t size, off, pos;
 	int ret;
 
+	policy_db = conn->ep->policy_db;
+
 	mutex_lock(&conn->bus->lock);
 	mutex_lock(&reg->entries_lock);
 
+	if (policy_db)
+		mutex_lock(&policy_db->entries_lock);
+
 	/* size of header */
 	size = sizeof(struct kdbus_name_list);
 
@@ -853,6 +864,9 @@ exit_pool_free:
 	if (ret < 0)
 		kdbus_pool_free_range(conn->pool, off);
 exit_unlock:
+	if (policy_db)
+		mutex_unlock(&policy_db->entries_lock);
+
 	mutex_unlock(&reg->entries_lock);
 	mutex_unlock(&conn->bus->lock);
 
diff --git a/policy.c b/policy.c
index 3a04024c681..0bc0a9999b6 100644
--- a/policy.c
+++ b/policy.c
@@ -30,20 +30,6 @@
 #define KDBUS_POLICY_HASH_SIZE	64
 
 /**
- * struct kdbus_policy_db - policy database
- * @entries_hash:	Hashtable of entries
- * @send_access_hash:	Hashtable of send access elements
- * @entries_lock:	Mutex to protect the database's access entries
- * @cache_lock:		Mutex to protect the database's cache
- */
-struct kdbus_policy_db {
-	DECLARE_HASHTABLE(entries_hash, 6);
-	DECLARE_HASHTABLE(send_access_hash, 6);
-	struct mutex entries_lock;
-	struct mutex cache_lock;
-};
-
-/**
  * struct kdbus_policy_db_cache_entry - a cached entry
  * @conn_a:		Connection A
  * @conn_b:		Connection B
@@ -379,21 +365,18 @@ exit_unlock_entries:
  *
  * Return: 0 if permission to see the name is granted, -EPERM otherwise
  */
-int kdbus_policy_check_see_access(struct kdbus_policy_db *db,
-				  const char *name)
+int kdbus_policy_check_see_access_unlocked(struct kdbus_policy_db *db,
+					   const char *name)
 {
 	struct kdbus_policy_db_entry *e;
-	int ret = -EPERM;
 	u32 hash;
 
 	hash = kdbus_str_hash(name);
-	mutex_lock(&db->entries_lock);
 	e = __kdbus_policy_lookup(db, name, hash, true);
 	if (kdbus_policy_check_access(e, KDBUS_POLICY_SEE))
-		ret = 0;
-	mutex_unlock(&db->entries_lock);
+		return 0;
 
-	return ret;
+	return -EPERM;
 }
 
 static void __kdbus_policy_remove_owner(struct kdbus_policy_db *db,
diff --git a/policy.h b/policy.h
index eceecdc6c13..380c06f14aa 100644
--- a/policy.h
+++ b/policy.h
@@ -14,12 +14,26 @@
 #define __KDBUS_POLICY_H
 
 struct kdbus_conn;
-struct kdbus_policy_db;
+
+/**
+ * struct kdbus_policy_db - policy database
+ * @entries_hash:	Hashtable of entries
+ * @send_access_hash:	Hashtable of send access elements
+ * @entries_lock:	Mutex to protect the database's access entries
+ * @cache_lock:		Mutex to protect the database's cache
+ */
+struct kdbus_policy_db {
+	DECLARE_HASHTABLE(entries_hash, 6);
+	DECLARE_HASHTABLE(send_access_hash, 6);
+	struct mutex entries_lock;
+	struct mutex cache_lock;
+};
 
 int kdbus_policy_db_new(struct kdbus_policy_db **db);
 void kdbus_policy_db_free(struct kdbus_policy_db *db);
-int kdbus_policy_check_see_access(struct kdbus_policy_db *db,
-				  const char *name);
+
+int kdbus_policy_check_see_access_unlocked(struct kdbus_policy_db *db,
+					   const char *name);
 int kdbus_policy_check_talk_access(struct kdbus_policy_db *db,
 				   struct kdbus_conn *conn_src,
 				   struct kdbus_conn *conn_dst);