All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH BlueZ v3 0/9] Add HID over GATT functional tests
@ 2026-09-24 15:46 Luiz Augusto von Dentz
  2026-09-24 15:46 ` [PATCH BlueZ v3 1/9] shared/gatt-client: Fix calling destroy after unregistering notify Luiz Augusto von Dentz
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ 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>

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).

v2:
 - Add "attrib: Fix unregistering notifications registered with
   bt_gatt_client", fixing the heap-use-after-free in
   report_notify_destroy reported by TestFunctional on the HoG tests:
   g_attrib_unregister did not unregister the notifications registered
   with bt_gatt_client, so their destroy callback was called after
   HoG had freed its reports.

v3:
 - Add "shared/gatt-client: Fix calling destroy after unregistering
   notify", fixing the heap-use-after-free in report_notify_destroy
   still reported by TestFunctional on the HoG tests with v2: once
   unregistered, the destroy callback of the notification was still
   called later if the write of the CCC disabling it was pending, after
   HoG had freed its reports.

Luiz Augusto von Dentz (9):
  shared/gatt-client: Fix calling destroy after unregistering notify
  attrib: Fix unregistering notifications registered with bt_gatt_client
  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 +-
 attrib/gattrib.c                 |  90 +++++++----
 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 +
 src/shared/gatt-client.c         |   9 ++
 test/functional/conftest.py      |  46 ++++++
 test/functional/test_hog.py      | 252 +++++++++++++++++++++++++++++++
 15 files changed, 794 insertions(+), 39 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] 15+ 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; 15+ 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] 15+ messages in thread
* [PATCH BlueZ v1 1/7] client/gatt: Fix setting descriptor value from scripts
@ 2026-09-23 19:31 Luiz Augusto von Dentz
  2026-09-23 22:30 ` Add HID over GATT functional tests bluez.test.bot
  0 siblings, 1 reply; 15+ 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>

gatt.register-descriptor completed the command right after prompting
for the value, so when run from a script the line with the value was
executed as a command instead of being passed to the prompt, causing
the descriptor to be unregistered.

Complete the command once the value is set, as done for
characteristics, and parse a copy of the value so the input line is
not truncated by strsep while still in use by the shell.
---
 client/gatt.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/client/gatt.c b/client/gatt.c
index 6dc80e2a31cd..ebbe4e3c7a32 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -700,13 +700,20 @@ void gatt_read_local_attribute(char *data, int argc, char *argv[])
 	return bt_shell_noninteractive_quit(EXIT_FAILURE);
 }
 
-static uint8_t *str2bytearray(char *arg, size_t *val_len)
+static uint8_t *str2bytearray(const char *arg, size_t *val_len)
 {
 	uint8_t value[MAX_ATTR_VAL_LEN];
-	char *entry;
+	char *str, *next, *entry;
 	unsigned int i;
 
-	for (i = 0; (entry = strsep(&arg, " \t")) != NULL; i++) {
+	/* Parse a copy as strsep modifies the string, which may still be
+	 * in use by the caller, e.g. the shell printing the input line.
+	 */
+	str = next = strdup(arg);
+	if (!str)
+		return NULL;
+
+	for (i = 0; (entry = strsep(&next, " \t")) != NULL; i++) {
 		long val;
 		char *endptr = NULL;
 
@@ -715,18 +722,22 @@ static uint8_t *str2bytearray(char *arg, size_t *val_len)
 
 		if (i >= G_N_ELEMENTS(value)) {
 			bt_shell_printf("Too much data\n");
+			free(str);
 			return NULL;
 		}
 
 		val = strtol(entry, &endptr, 0);
 		if (!endptr || *endptr != '\0' || val > UINT8_MAX) {
 			bt_shell_printf("Invalid value at index %d\n", i);
+			free(str);
 			return NULL;
 		}
 
 		value[i] = val;
 	}
 
+	free(str);
+
 	*val_len = i;
 
 	return util_memdup(value, i);
@@ -2788,7 +2799,7 @@ static void chrc_set_value(const char *input, void *user_data)
 
 	g_free(chrc->value);
 
-	chrc->value = str2bytearray((char *) input, &chrc->value_len);
+	chrc->value = str2bytearray(input, &chrc->value_len);
 
 	if (!chrc->value) {
 		print_chrc(chrc, COLORED_DEL);
@@ -3078,7 +3089,7 @@ static void desc_set_value(const char *input, void *user_data)
 
 	g_free(desc->value);
 
-	desc->value = str2bytearray((char *) input, &desc->value_len);
+	desc->value = str2bytearray(input, &desc->value_len);
 
 	if (!desc->value) {
 		print_desc(desc, COLORED_DEL);
@@ -3086,6 +3097,8 @@ static void desc_set_value(const char *input, void *user_data)
 	}
 
 	desc->max_val_len = desc->value_len;
+
+	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
 
 void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
@@ -3134,8 +3147,6 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
 	print_desc(desc, COLORED_NEW);
 
 	bt_shell_prompt_input(desc->path, "Enter value:", desc_set_value, desc);
-
-	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
 
 static struct desc *desc_find(const char *pattern)
-- 
2.55.0


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

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 15:46 [PATCH BlueZ v3 0/9] Add HID over GATT functional tests Luiz Augusto von Dentz
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
2026-09-24 15:46 ` [PATCH BlueZ v3 2/9] attrib: Fix unregistering notifications registered with bt_gatt_client Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 3/9] client/gatt: Fix setting descriptor value from scripts Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 4/9] client/mgmt: Print Connection Subrate event Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 5/9] emulator: Default to the latest BR/EDR+LE version Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 6/9] client/scripts: Add HoG device scripts Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 7/9] doc: Add functional-hog documentation Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 8/9] test: functional: add HoG tests Luiz Augusto von Dentz
2026-09-24 15:46 ` [PATCH BlueZ v3 9/9] test: functional: limit the workers by the memory available Luiz Augusto von Dentz
2026-09-29 20:50 ` [PATCH BlueZ v3 0/9] Add HID over GATT functional tests patchwork-bot+bluetooth
  -- 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-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

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.