All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v1 0/7] Add HID over GATT functional tests
@ 2026-09-23 19:31 Luiz Augusto von Dentz
  2026-09-23 19:31 ` [PATCH BlueZ v1 1/7] client/gatt: Fix setting descriptor value from scripts Luiz Augusto von Dentz
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-23 19:31 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This adds functional tests for HID over GATT (HoG), with bluetoothctl
registering a HID Service acting as a keyboard, with and without
Shorter Connection Interval (SCI) support, using the new
client/scripts/hog-device*.bt scripts, and the HID host:

 - checking HID Information, HID SCI Mode and HID SCI Information over
   GATT with gatt.select-attribute/gatt.read
 - receiving a few Input Reports notified by the HID device
 - with SCI support, changing the SCI mode, followed by the connection
   rate with mgmt.conn-subrate, and receiving the notification from
   the HID device confirming the mode has been changed

The tests are documented in doc/functional-hog.rst.

To support them:

 - bluetoothctl can now set descriptor values from scripts, and prints
   the MGMT Connection Subrate event
 - btvirt defaults to the latest BR/EDR+LE version (6.2), so Shorter
   Connection Intervals are supported by the emulated controllers,
   with the new -C/--core option to emulate older versions

Also, with -n auto, the number of functional test workers is now
limited by the memory available instead of one per CPU, since running
out of memory with so many VM instances made tests fail at random, and
check-functional uses -n auto by default (override with
CHECK_FUNCTIONAL_JOBS).

Luiz Augusto von Dentz (7):
  client/gatt: Fix setting descriptor value from scripts
  client/mgmt: Print Connection Subrate event
  emulator: Default to the latest BR/EDR+LE version
  client/scripts: Add HoG device scripts
  doc: Add functional-hog documentation
  test: functional: add HoG tests
  test: functional: limit the workers by the memory available

 Makefile.am                      |   8 +-
 client/gatt.c                    |  25 ++-
 client/mgmt.c                    |  31 ++++
 client/scripts/hog-device-sci.bt |  49 ++++++
 client/scripts/hog-device.bt     |  38 +++++
 doc/functional-hog.rst           | 188 +++++++++++++++++++++++
 doc/functional-testing.rst       |   1 +
 doc/test-functional.rst          |  25 +++
 emulator/main.c                  |  54 ++++++-
 emulator/server.c                |  15 +-
 emulator/server.h                |   2 +
 test/functional/conftest.py      |  46 ++++++
 test/functional/test_hog.py      | 252 +++++++++++++++++++++++++++++++
 13 files changed, 722 insertions(+), 12 deletions(-)
 create mode 100644 client/scripts/hog-device-sci.bt
 create mode 100644 client/scripts/hog-device.bt
 create mode 100644 doc/functional-hog.rst
 create mode 100644 test/functional/test_hog.py

-- 
2.55.0


^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH BlueZ v2 1/8] attrib: Fix unregistering notifications registered with bt_gatt_client
@ 2026-09-24 13:55 Luiz Augusto von Dentz
  2026-09-24 15:25 ` Add HID over GATT functional tests bluez.test.bot
  0 siblings, 1 reply; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-24 13:55 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

g_attrib_register registers ATT_OP_HANDLE_NOTIFY with bt_gatt_client,
when there is one, returning its id, but g_attrib_unregister always
unregisters with bt_att, so the notification was never unregistered
and its destroy callback was called once bt_gatt_client was freed,
after the user data was freed, e.g. the reports of HoG:

 ERROR: AddressSanitizer: heap-use-after-free
 #11 report_notify_destroy profiles/input/hog-lib.c:359
 #12 attrib_callbacks_destroy attrib/gattrib.c:126
 #13 notify_data_unref src/shared/gatt-client.c:256
 ...
 #17 bt_gatt_client_free src/shared/gatt-client.c:2293
 #18 bt_gatt_client_unref src/shared/gatt-client.c:2605
 #19 g_attrib_unref attrib/gattrib.c:154
 #20 attio_cleanup src/device.c:874

Give out ids of our own, keeping track of how each one was registered
so it is unregistered accordingly.
---
 attrib/gattrib.c | 90 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 27 deletions(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 3a988e131e94..05a17ba9393f 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -45,6 +45,7 @@ struct _GAttrib {
 	uint8_t *buf;
 	int buflen;
 	struct queue *track_ids;
+	unsigned int next_reg_id;
 };
 
 struct attrib_callbacks {
@@ -55,6 +56,9 @@ struct attrib_callbacks {
 	gpointer user_data;
 	GAttrib *parent;
 	uint16_t notify_handle;
+	unsigned int reg_id;
+	unsigned int att_id;
+	unsigned int client_id;
 };
 
 GAttrib *g_attrib_new(GIOChannel *io, guint16 mtu, bool ext_signed)
@@ -363,38 +367,52 @@ guint g_attrib_register(GAttrib *attrib, guint8 opcode, guint16 handle,
 				GAttribNotifyFunc func, gpointer user_data,
 				GDestroyNotify notify)
 {
-	struct attrib_callbacks *cb = NULL;
+	struct attrib_callbacks *cb;
 
 	if (!attrib)
 		return 0;
 
-	if (func || notify) {
-		cb = new0(struct attrib_callbacks, 1);
-		if (!cb)
-			return 0;
-		cb->notify_func = func;
-		cb->notify_handle = handle;
-		cb->user_data = user_data;
-		cb->destroy_func = notify;
-		cb->parent = attrib;
-		queue_push_head(attrib->callbacks, cb);
-	}
+	cb = new0(struct attrib_callbacks, 1);
+	if (!cb)
+		return 0;
 
-	if (opcode == ATT_OP_HANDLE_NOTIFY && attrib->client) {
-		unsigned int id;
+	cb->notify_func = func;
+	cb->notify_handle = handle;
+	cb->user_data = user_data;
+	cb->destroy_func = notify;
+	cb->parent = attrib;
+	queue_push_head(attrib->callbacks, cb);
 
-		id = bt_gatt_client_register_notify(attrib->client, handle,
-						NULL, client_notify_cb, cb,
-						attrib_callbacks_remove);
-		if (id)
-			return id;
-	}
-
-	if (opcode == GATTRIB_ALL_REQS)
-		opcode = BT_ATT_ALL_REQUESTS;
-
-	return bt_att_register(attrib->att, opcode, attrib_callback_notify,
+	/* The notifications are registered with bt_gatt_client when there
+	 * is one, which uses ids of its own, so give out ids of our own to
+	 * know how to unregister them.
+	 */
+	if (opcode == ATT_OP_HANDLE_NOTIFY && attrib->client)
+		cb->client_id = bt_gatt_client_register_notify(attrib->client,
+						handle, NULL, client_notify_cb,
 						cb, attrib_callbacks_remove);
+
+	if (!cb->client_id) {
+		if (opcode == GATTRIB_ALL_REQS)
+			opcode = BT_ATT_ALL_REQUESTS;
+
+		cb->att_id = bt_att_register(attrib->att, opcode,
+						attrib_callback_notify, cb,
+						attrib_callbacks_remove);
+	}
+
+	if (!cb->client_id && !cb->att_id) {
+		queue_remove(attrib->callbacks, cb);
+		free(cb);
+		return 0;
+	}
+
+	if (++attrib->next_reg_id == 0)
+		++attrib->next_reg_id;
+
+	cb->reg_id = attrib->next_reg_id;
+
+	return cb->reg_id;
 }
 
 uint8_t *g_attrib_get_buffer(GAttrib *attrib, size_t *len)
@@ -456,12 +474,30 @@ gboolean g_attrib_attach_client(GAttrib *attrib, struct bt_gatt_client *client)
 	return TRUE;
 }
 
+static bool match_reg_id(const void *data, const void *match_data)
+{
+	const struct attrib_callbacks *cb = data;
+
+	return cb->reg_id && cb->reg_id == PTR_TO_UINT(match_data);
+}
+
 gboolean g_attrib_unregister(GAttrib *attrib, guint id)
 {
-	if (!attrib)
+	struct attrib_callbacks *cb;
+
+	if (!attrib || !id)
 		return FALSE;
 
-	return bt_att_unregister(attrib->att, id);
+	cb = queue_find(attrib->callbacks, match_reg_id, UINT_TO_PTR(id));
+	if (!cb)
+		return FALSE;
+
+	/* cb is freed by attrib_callbacks_remove once unregistered */
+	if (cb->client_id)
+		return bt_gatt_client_unregister_notify(attrib->client,
+							cb->client_id);
+
+	return bt_att_unregister(attrib->att, cb->att_id);
 }
 
 gboolean g_attrib_unregister_all(GAttrib *attrib)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH BlueZ v3 1/9] shared/gatt-client: Fix calling destroy after unregistering notify
@ 2026-09-24 15:46 Luiz Augusto von Dentz
  2026-09-24 19:16 ` Add HID over GATT functional tests bluez.test.bot
  2026-10-09 19:52 ` bluez.test.bot
  0 siblings, 2 replies; 12+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-24 15:46 UTC (permalink / raw)
  To: linux-bluetooth

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

bt_gatt_client_unregister_notify resets the callbacks of the
notification but not its destroy callback, which is called once the
notify_data is freed. If a procedure is still pending at that point,
e.g. the write of the CCC to disable the notifications, it holds a
reference to notify_data so destroy is called later, once the user data
may have been freed, e.g. the reports of HoG:

 ERROR: AddressSanitizer: heap-use-after-free
 #11 report_notify_destroy profiles/input/hog-lib.c:359
 #12 attrib_callbacks_destroy attrib/gattrib.c:130
 #13 notify_data_unref src/shared/gatt-client.c:256
 #15 destroy_write_op src/shared/gatt-client.c:3189
 #16 request_unref src/shared/gatt-client.c:201
 #17 destroy_att_send_op src/shared/att.c:215
 #18 bt_att_cancel src/shared/att.c:1925
 #19 cancel_request src/shared/gatt-client.c:2783
 ...
 #21 bt_gatt_client_cancel_all src/shared/gatt-client.c:2811
 #22 bt_gatt_client_free src/shared/gatt-client.c:2290

Call destroy when unregistering instead.
---
 src/shared/gatt-client.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 92ad7c39c115..cd4270291827 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -3858,6 +3858,15 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
 	notify_data->callback = NULL;
 	notify_data->notify = NULL;
 
+	/* Call destroy now as the user data may be freed once unregistered,
+	 * while notify_data may still be referenced by a pending procedure,
+	 * e.g. the write of the CCC.
+	 */
+	if (notify_data->destroy) {
+		notify_data->destroy(notify_data->user_data);
+		notify_data->destroy = NULL;
+	}
+
 	complete_unregister_notify(notify_data);
 	return true;
 }
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-10-09 19:52 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-23 19:31 [PATCH BlueZ v1 0/7] Add HID over GATT functional tests Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 1/7] client/gatt: Fix setting descriptor value from scripts Luiz Augusto von Dentz
2026-09-23 22:30   ` Add HID over GATT functional tests bluez.test.bot
2026-09-23 19:31 ` [PATCH BlueZ v1 2/7] client/mgmt: Print Connection Subrate event Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 3/7] emulator: Default to the latest BR/EDR+LE version Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 4/7] client/scripts: Add HoG device scripts Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 5/7] doc: Add functional-hog documentation Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 6/7] test: functional: add HoG tests Luiz Augusto von Dentz
2026-09-23 19:31 ` [PATCH BlueZ v1 7/7] test: functional: limit the workers by the memory available Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2026-09-24 13:55 [PATCH BlueZ v2 1/8] attrib: Fix unregistering notifications registered with bt_gatt_client Luiz Augusto von Dentz
2026-09-24 15:25 ` Add HID over GATT functional tests bluez.test.bot
2026-09-24 15:46 [PATCH BlueZ v3 1/9] shared/gatt-client: Fix calling destroy after unregistering notify Luiz Augusto von Dentz
2026-09-24 19:16 ` Add HID over GATT functional tests bluez.test.bot
2026-10-09 19:52 ` bluez.test.bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.