Linux bluetooth development
 help / color / mirror / Atom feed
From: "Frédéric Danis" <frederic.danis@collabora.com>
To: linux-bluetooth@vger.kernel.org
Subject: [PATCH BlueZ 1/7] plugins/admin: make AdminPolicy state per-adapter
Date: Thu,  2 Jul 2026 10:36:35 +0200	[thread overview]
Message-ID: <20260702083641.378994-1-frederic.danis@collabora.com> (raw)

Fix AdminPolicy D-Bus updates being emitted on the wrong adapter path
by removing the single global policy context and moving to per-adapter
policy objects.

Changes include:
- track policy contexts in a policy queue keyed by adapter pointer
- keep per-adapter device lists inside each policy context
- emit ServiceAllowList changes using the callback's adapter context
- scope device affected updates to the current adapter only
- clean up probe/remove lifecycle so adapters are registered and torn
  down independently
- remove remaining global policy_data/devices coupling

Assisted-by: GPT:GPT-5.3-Codex
---
 plugins/admin.c | 130 ++++++++++++++++++++++++++++++++++++------------
 1 file changed, 99 insertions(+), 31 deletions(-)

diff --git a/plugins/admin.c b/plugins/admin.c
index 76c65d068..6c5c8d97f 100644
--- a/plugins/admin.c
+++ b/plugins/admin.c
@@ -39,21 +39,45 @@
 #define BTD_DEVICE_INTERFACE		"org.bluez.Device1"
 
 static DBusConnection *dbus_conn;
-static struct queue *devices; /* List of struct device_data objects */
+static struct queue *policies; /* List of struct btd_admin_policy objects */
 
-/* |policy_data| has the same life cycle as btd_adapter */
-static struct btd_admin_policy {
+/* One policy context per adapter */
+struct btd_admin_policy {
 	struct btd_adapter *adapter;
 	uint16_t adapter_id;
 	struct queue *service_allowlist;
-} *policy_data = NULL;
+	struct queue *devices;
+};
 
 struct device_data {
 	struct btd_device *device;
 	char *path;
 	bool affected;
+	struct btd_admin_policy *policy;
 };
 
+static void free_device_data(void *data);
+static void unregister_device_data(void *data, void *user_data);
+
+static bool policy_match_adapter(const void *data, const void *match_data)
+{
+	const struct btd_admin_policy *policy = data;
+	const struct btd_adapter *adapter = match_data;
+
+	if (!policy)
+		return false;
+
+	return policy->adapter == adapter;
+}
+
+static struct btd_admin_policy *admin_policy_find(struct btd_adapter *adapter)
+{
+	if (!policies)
+		return NULL;
+
+	return queue_find(policies, policy_match_adapter, adapter);
+}
+
 static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
 {
 	struct btd_admin_policy *admin_policy = NULL;
@@ -68,6 +92,16 @@ static struct btd_admin_policy *admin_policy_new(struct btd_adapter *adapter)
 	admin_policy->adapter = adapter;
 	admin_policy->adapter_id = btd_adapter_get_index(adapter);
 	admin_policy->service_allowlist = queue_new();
+	admin_policy->devices = queue_new();
+
+	if (!admin_policy->service_allowlist || !admin_policy->devices) {
+		queue_destroy(admin_policy->service_allowlist, free);
+		queue_destroy(admin_policy->devices, NULL);
+		g_free(admin_policy);
+		btd_error(btd_adapter_get_index(adapter),
+				"Failed to allocate queues for admin_policy");
+		return NULL;
+	}
 
 	return admin_policy;
 }
@@ -82,6 +116,7 @@ static void admin_policy_free(void *data)
 	struct btd_admin_policy *admin_policy = data;
 
 	free_service_allowlist(admin_policy->service_allowlist);
+	queue_destroy(admin_policy->devices, free_device_data);
 	g_free(admin_policy);
 }
 
@@ -89,6 +124,8 @@ static void admin_policy_destroy(struct btd_admin_policy *admin_policy)
 {
 	const char *path = adapter_get_path(admin_policy->adapter);
 
+	queue_foreach(admin_policy->devices, unregister_device_data, NULL);
+
 	g_dbus_unregister_interface(dbus_conn, path,
 						ADMIN_POLICY_SET_INTERFACE);
 	g_dbus_unregister_interface(dbus_conn, path,
@@ -346,7 +383,7 @@ static void load_policy_settings(struct btd_admin_policy *admin_policy)
 			btd_adapter_get_storage_dir(admin_policy->adapter));
 
 	if (stat(filename, &st) < 0)
-		store_policy_settings(policy_data);
+		store_policy_settings(admin_policy);
 
 	key_file = g_key_file_new();
 
@@ -387,11 +424,11 @@ static DBusMessage *set_service_allowlist(DBusConnection *conn,
 	}
 
 	g_dbus_emit_property_changed(dbus_conn,
-					adapter_get_path(policy_data->adapter),
+					adapter_get_path(admin_policy->adapter),
 					ADMIN_POLICY_STATUS_INTERFACE,
 					"ServiceAllowList");
 
-	queue_foreach(devices, update_device_affected, NULL);
+	queue_foreach(admin_policy->devices, update_device_affected, NULL);
 
 	return dbus_message_new_method_return(msg);
 }
@@ -488,66 +525,86 @@ static void remove_device_data(void *data)
 
 	DBG("device_data for %s removing", device_data->path);
 
-	queue_remove(devices, device_data);
+	if (device_data->policy)
+		queue_remove(device_data->policy->devices, device_data);
+
 	free_device_data(device_data);
 }
 
 static int admin_policy_adapter_probe(struct btd_adapter *adapter)
 {
+	struct btd_admin_policy *policy;
 	const char *adapter_path;
 
-	if (!devices)
-		devices = queue_new();
+	if (!policies)
+		policies = queue_new();
 
-	if (policy_data) {
-		btd_warn(policy_data->adapter_id,
+	if (!policies)
+		return -ENOMEM;
+
+	if (admin_policy_find(adapter)) {
+		btd_warn(btd_adapter_get_index(adapter),
 						"Policy data already exists");
-		policy_data = NULL;
+		return -EALREADY;
 	}
 
-	policy_data = admin_policy_new(adapter);
-	if (!policy_data)
+	policy = admin_policy_new(adapter);
+	if (!policy)
 		return -ENOMEM;
 
-	load_policy_settings(policy_data);
+	load_policy_settings(policy);
 	adapter_path = adapter_get_path(adapter);
 
 	if (!g_dbus_register_interface(dbus_conn, adapter_path,
 					ADMIN_POLICY_SET_INTERFACE,
 					admin_policy_adapter_methods, NULL,
-					NULL, policy_data, NULL)) {
-		btd_error(policy_data->adapter_id,
+					NULL, policy, NULL)) {
+		btd_error(policy->adapter_id,
 			"Admin Policy Set interface init failed on path %s",
 								adapter_path);
+		admin_policy_free(policy);
 		return -EINVAL;
 	}
 
-	btd_info(policy_data->adapter_id,
+	btd_info(policy->adapter_id,
 				"Admin Policy Set interface registered");
 
 	if (!g_dbus_register_interface(dbus_conn, adapter_path,
 					ADMIN_POLICY_STATUS_INTERFACE,
 					NULL, NULL,
 					admin_policy_adapter_properties,
-					policy_data, NULL)) {
-		btd_error(policy_data->adapter_id,
+					policy, NULL)) {
+		btd_error(policy->adapter_id,
 			"Admin Policy Status interface init failed on path %s",
 								adapter_path);
+		g_dbus_unregister_interface(dbus_conn, adapter_path,
+						ADMIN_POLICY_SET_INTERFACE);
+		admin_policy_free(policy);
 		return -EINVAL;
 	}
 
-	btd_info(policy_data->adapter_id,
+	btd_info(policy->adapter_id,
 				"Admin Policy Status interface registered");
 
+	queue_push_tail(policies, policy);
+
 	return 0;
 }
 
 static void admin_policy_device_added(struct btd_adapter *adapter,
 						struct btd_device *device)
 {
+	struct btd_admin_policy *policy;
 	struct device_data *data;
 
-	if (queue_find(devices, device_data_match, device))
+	policy = admin_policy_find(adapter);
+	if (!policy) {
+		btd_warn(btd_adapter_get_index(adapter),
+				"Policy data not found for adapter");
+		return;
+	}
+
+	if (queue_find(policy->devices, device_data_match, device))
 		return;
 
 	data = g_new0(struct device_data, 1);
@@ -560,6 +617,7 @@ static void admin_policy_device_added(struct btd_adapter *adapter,
 	data->device = device;
 	data->path = g_strdup(device_get_path(device));
 	data->affected = !btd_device_all_services_allowed(data->device);
+	data->policy = policy;
 
 	if (!g_dbus_register_interface(dbus_conn, data->path,
 					ADMIN_POLICY_STATUS_INTERFACE,
@@ -573,7 +631,7 @@ static void admin_policy_device_added(struct btd_adapter *adapter,
 		return;
 	}
 
-	queue_push_tail(devices, data);
+	queue_push_tail(policy->devices, data);
 
 	DBG("device_data for %s added", data->path);
 }
@@ -589,9 +647,14 @@ static void unregister_device_data(void *data, void *user_data)
 static void admin_policy_device_removed(struct btd_adapter *adapter,
 						struct btd_device *device)
 {
+	struct btd_admin_policy *policy;
 	struct device_data *data;
 
-	data = queue_find(devices, device_data_match, device);
+	policy = admin_policy_find(adapter);
+	if (!policy)
+		return;
+
+	data = queue_find(policy->devices, device_data_match, device);
 
 	if (data)
 		unregister_device_data(data, NULL);
@@ -599,15 +662,20 @@ static void admin_policy_device_removed(struct btd_adapter *adapter,
 
 static void admin_policy_remove(struct btd_adapter *adapter)
 {
+	struct btd_admin_policy *policy;
+
 	DBG("");
 
-	queue_foreach(devices, unregister_device_data, NULL);
-	queue_destroy(devices, g_free);
-	devices = NULL;
+	policy = admin_policy_find(adapter);
+	if (!policy)
+		return;
+
+	queue_remove(policies, policy);
+	admin_policy_destroy(policy);
 
-	if (policy_data) {
-		admin_policy_destroy(policy_data);
-		policy_data = NULL;
+	if (!queue_length(policies)) {
+		queue_destroy(policies, NULL);
+		policies = NULL;
 	}
 }
 
-- 
2.43.0


             reply	other threads:[~2026-07-02  8:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02  8:36 Frédéric Danis [this message]
2026-07-02  8:36 ` [PATCH BlueZ 2/7] client/bluetoothctl: make admin.allow controller-aware Frédéric Danis
2026-07-02  8:36 ` [PATCH BlueZ 3/7] doc: document admin.allow optional controller argument Frédéric Danis
2026-07-02  8:36 ` [PATCH BlueZ 4/7] src/adapter: enforce allowlist for local services Frédéric Danis
2026-07-02  8:36 ` [PATCH BlueZ 5/7] plugins/admin: reapply allowlist on policy updates Frédéric Danis
2026-07-02  8:36 ` [PATCH BlueZ 6/7] doc: describe admin allowlist runtime enforcement Frédéric Danis
2026-07-02  8:36 ` [PATCH BlueZ 7/7] profiles/audio: fix UAF on external media service teardown Frédéric Danis
2026-07-02 11:57 ` [BlueZ,1/7] plugins/admin: make AdminPolicy state per-adapter bluez.test.bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260702083641.378994-1-frederic.danis@collabora.com \
    --to=frederic.danis@collabora.com \
    --cc=linux-bluetooth@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox