Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH BlueZ v4 00/20] Add HoG functional tests and shared/hog
@ 2026-09-24 22:30 Luiz Augusto von Dentz
  2026-09-24 22:30 ` [PATCH BlueZ v4 01/20] shared/gatt-client: Fix calling destroy after unregistering notify Luiz Augusto von Dentz
                   ` (19 more replies)
  0 siblings, 20 replies; 23+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-24 22:30 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, requesting SCI Fast mode with the HID Control
   Point, as specified by HOGP.TS 4.6.1, followed by the connection
   rate with mgmt.conn-subrate, and receiving the notification of HID
   SCI Mode from the HID device confirming the mode has been changed

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

The input plugin is also moved away from GAttrib: the new src/shared/hog
implements HID over GATT on top of bt_gatt_client and gatt_db, as a
drop-in replacement of profiles/input/hog-lib, which is removed along
with the GAttrib based Battery, Device Information and Scan Parameters
implementations only used by it, these services being handled by their
own plugins. src/shared/hog supports HID SCI, requesting a mode with
the HID Control Point and enabling the notifications of HID SCI Mode.

With that GAttrib has no users left in bluetoothd, which now creates
the bt_att of the connection directly, so GAttrib is removed along with
the deprecated gatttool, its only other user.

unit/test-hog is ported to src/shared/hog, with the test cases renamed
after HOGP.TS p13 and the missing ones for the Report Host added, except
HID ISO which is not supported, including the HID SCI test cases
HGWF/BV-08-C to BV-11-C.

To support this:

 - bluetoothctl can now set descriptor values from scripts, prints the
   MGMT Connection Subrate event, and no longer crashes when the auto
   agent is canceled
 - 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
 - the tester can expect a PDU with no response, e.g. Write Command
 - unit/test-uhid tests the replies to Get Report, including through
   hidraw when run as root, which requires CONFIG_HIDRAW now added to
   the tester kernel config

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

v4:
 - Fix a use-after-free in bluetoothd introduced by "shared/gatt-client:
   Fix calling destroy after unregistering notify", when enabling
   notifications with StartNotify fails, and hold a reference to the
   client while calling destroy
 - Drop "attrib: Fix unregistering notifications registered with
   bt_gatt_client", as GAttrib is now removed
 - client/gatt: fix use-after-free on invalid values set from scripts,
   uninitialized bytes when parsing values with consecutive separators,
   and reject negative values
 - Add "client/agent: Fix crash on Cancel with no pending request",
   reported by TestFunctional with v3
 - Add src/shared/hog, replacing hog-lib in the input plugin, and port
   unit/test-hog to it with HOGP.TS p13 test cases, including HID SCI
 - Change the HID SCI mode with the HID Control Point in the functional
   test, HID SCI Mode being Read and Notify only as specified
 - Fix the size of the reply to Get Report with a Report ID in
   shared/uhid, and add unit/test-uhid Get Report tests along with
   CONFIG_HIDRAW in the tester kernel config
 - Add "device: Use bt_att instead of GAttrib" and "attrib: Remove
   GAttrib and gatttool"
 - Honour PYTEST_XDIST_AUTO_NUM_WORKERS and the CPU affinity when
   limiting the functional test workers

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.

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.

Luiz Augusto von Dentz (20):
  shared/gatt-client: Fix calling destroy after unregistering notify
  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
  client/agent: Fix crash on Cancel with no pending request
  shared/uhid: Fix size of Get Report reply with a Report ID
  shared/uhid: Keep reading when an event is not available
  shared/tester: Allow expecting a PDU with no response
  shared/hog: Add initial implementation
  unit/test-hog: Use shared/hog
  test: functional: change the HoG SCI mode with the HID Control Point
  input/hog: Use shared/hog
  doc: Add CONFIG_HIDRAW to the tester kernel config
  unit/test-uhid: Add Get Report tests
  device: Use bt_att instead of GAttrib
  attrib: Remove GAttrib and gatttool

 .gitignore                       |    2 -
 Makefile.am                      |   31 +-
 Makefile.plugins                 |    4 -
 Makefile.tools                   |   12 -
 attrib/gatt.c                    | 1150 +----------------
 attrib/gatt.h                    |   89 --
 attrib/gattrib.c                 |  473 -------
 attrib/gattrib.h                 |   65 -
 attrib/gatttool.c                |  612 ----------
 attrib/gatttool.h                |   17 -
 attrib/interactive.c             | 1020 ----------------
 attrib/utils.c                   |  110 --
 client/agent.c                   |   10 +-
 client/gatt.c                    |   39 +-
 client/mgmt.c                    |   31 +
 client/scripts/hog-device-sci.bt |   49 +
 client/scripts/hog-device.bt     |   38 +
 doc/functional-hog.rst           |  204 ++++
 doc/functional-testing.rst       |    1 +
 doc/test-functional.rst          |   29 +-
 doc/test-runner.rst              |    5 +
 doc/tester.config                |    1 +
 emulator/main.c                  |   55 +-
 emulator/server.c                |   15 +-
 emulator/server.h                |    2 +
 profiles/battery/bas.c           |  327 -----
 profiles/battery/bas.h           |   19 -
 profiles/deviceinfo/deviceinfo.c |    2 +-
 profiles/deviceinfo/dis.c        |  340 ------
 profiles/deviceinfo/dis.h        |   27 -
 profiles/input/hog-lib.c         | 1966 ------------------------------
 profiles/input/hog-lib.h         |   28 -
 profiles/input/hog.c             |   31 +-
 profiles/ranging/rap.c           |    1 -
 profiles/scanparam/scpp.c        |  342 ------
 profiles/scanparam/scpp.h        |   22 -
 src/adapter.c                    |    1 -
 src/device.c                     |   46 +-
 src/device.h                     |    1 -
 src/gatt-client.c                |    7 +-
 src/shared/gatt-client.c         |   21 +
 src/shared/hog.c                 | 1659 +++++++++++++++++++++++++
 src/shared/hog.h                 |   67 +
 src/shared/tester.c              |   19 +
 src/shared/uhid.c                |    7 +-
 test/functional/conftest.py      |   58 +
 test/functional/test_hog.py      |  269 ++++
 unit/test-gattrib.c              |  552 ---------
 unit/test-hog.c                  | 1431 ++++++++++++++++------
 unit/test-uhid.c                 |  272 +++++
 50 files changed, 3965 insertions(+), 7614 deletions(-)
 delete mode 100644 attrib/gattrib.c
 delete mode 100644 attrib/gattrib.h
 delete mode 100644 attrib/gatttool.c
 delete mode 100644 attrib/gatttool.h
 delete mode 100644 attrib/interactive.c
 delete mode 100644 attrib/utils.c
 create mode 100644 client/scripts/hog-device-sci.bt
 create mode 100644 client/scripts/hog-device.bt
 create mode 100644 doc/functional-hog.rst
 delete mode 100644 profiles/battery/bas.c
 delete mode 100644 profiles/battery/bas.h
 delete mode 100644 profiles/deviceinfo/dis.c
 delete mode 100644 profiles/deviceinfo/dis.h
 delete mode 100644 profiles/input/hog-lib.c
 delete mode 100644 profiles/input/hog-lib.h
 delete mode 100644 profiles/scanparam/scpp.c
 delete mode 100644 profiles/scanparam/scpp.h
 create mode 100644 src/shared/hog.c
 create mode 100644 src/shared/hog.h
 create mode 100644 test/functional/test_hog.py
 delete mode 100644 unit/test-gattrib.c

-- 
2.55.0


^ permalink raw reply	[flat|nested] 23+ messages in thread
* [PATCH BlueZ v6 01/23] shared/gatt-client: Fix calling destroy after unregistering notify
@ 2026-09-28 20:00 Luiz Augusto von Dentz
  2026-09-28 22:26 ` Add HoG functional tests and shared/hog bluez.test.bot
  0 siblings, 1 reply; 23+ messages in thread
From: Luiz Augusto von Dentz @ 2026-09-28 20:00 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, once done with notify_data and
holding a reference to the client in case destroy drops the last one.

As destroy is now called when unregistering, reply to StartNotify before
freeing the notify client when enabling the notifications fails, since
it frees the operation the reply is for.

Assisted-by: OpenCode:claude-opus-5.5
---
 src/gatt-client.c        |  7 +++++--
 src/shared/gatt-client.c | 21 +++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/gatt-client.c b/src/gatt-client.c
index 3baf95c4f79c..d94dc9d7fbf6 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -1488,12 +1488,15 @@ static void register_notify_cb(uint16_t att_ecode, void *user_data)
 	struct characteristic *chrc = client->chrc;
 
 	if (att_ecode) {
+		/* Reply first, as freeing the client unregisters the
+		 * notification, which frees op with its destroy callback.
+		 */
+		create_notify_reply(op, false, att_ecode);
+
 		queue_remove(chrc->notify_clients, client);
 		queue_remove(chrc->service->client->all_notify_clients, client);
 		notify_client_free(client);
 
-		create_notify_reply(op, false, att_ecode);
-
 		return;
 	}
 
diff --git a/src/shared/gatt-client.c b/src/shared/gatt-client.c
index 92ad7c39c115..b3bc62220e16 100644
--- a/src/shared/gatt-client.c
+++ b/src/shared/gatt-client.c
@@ -3842,6 +3842,8 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
 							unsigned int id)
 {
 	struct notify_data *notify_data;
+	bt_gatt_client_destroy_func_t destroy;
+	void *user_data;
 
 	if (!client || !id)
 		return false;
@@ -3858,7 +3860,26 @@ bool bt_gatt_client_unregister_notify(struct bt_gatt_client *client,
 	notify_data->callback = NULL;
 	notify_data->notify = NULL;
 
+	/* Call destroy once unregistered, as the user data may be freed then,
+	 * while notify_data may still be referenced by a pending procedure,
+	 * e.g. the write of the CCC, which would otherwise call it later.
+	 */
+	destroy = notify_data->destroy;
+	user_data = notify_data->user_data;
+	notify_data->destroy = NULL;
+
+	/* The client may be freed by destroy, e.g. if the user data holds
+	 * its last reference.
+	 */
+	bt_gatt_client_ref(client);
+
 	complete_unregister_notify(notify_data);
+
+	if (destroy)
+		destroy(user_data);
+
+	bt_gatt_client_unref(client);
+
 	return true;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2026-09-28 22:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 22:30 [PATCH BlueZ v4 00/20] Add HoG functional tests and shared/hog Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 01/20] shared/gatt-client: Fix calling destroy after unregistering notify Luiz Augusto von Dentz
2026-09-25  0:45   ` Add HoG functional tests and shared/hog bluez.test.bot
2026-09-24 22:30 ` [PATCH BlueZ v4 02/20] client/gatt: Fix setting descriptor value from scripts Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 03/20] client/mgmt: Print Connection Subrate event Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 04/20] emulator: Default to the latest BR/EDR+LE version Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 05/20] client/scripts: Add HoG device scripts Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 06/20] doc: Add functional-hog documentation Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 07/20] test: functional: add HoG tests Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 08/20] test: functional: limit the workers by the memory available Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 09/20] client/agent: Fix crash on Cancel with no pending request Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 10/20] shared/uhid: Fix size of Get Report reply with a Report ID Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 11/20] shared/uhid: Keep reading when an event is not available Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 12/20] shared/tester: Allow expecting a PDU with no response Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 13/20] shared/hog: Add initial implementation Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 14/20] unit/test-hog: Use shared/hog Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 15/20] test: functional: change the HoG SCI mode with the HID Control Point Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 16/20] input/hog: Use shared/hog Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 17/20] doc: Add CONFIG_HIDRAW to the tester kernel config Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 18/20] unit/test-uhid: Add Get Report tests Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 19/20] device: Use bt_att instead of GAttrib Luiz Augusto von Dentz
2026-09-24 22:30 ` [PATCH BlueZ v4 20/20] attrib: Remove GAttrib and gatttool Luiz Augusto von Dentz
  -- strict thread matches above, loose matches on Subject: below --
2026-09-28 20:00 [PATCH BlueZ v6 01/23] shared/gatt-client: Fix calling destroy after unregistering notify Luiz Augusto von Dentz
2026-09-28 22:26 ` Add HoG functional tests and shared/hog bluez.test.bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox