Thread (6 messages) 6 messages, 3 authors, 13d ago

[PATCH BlueZ 2/3] unit: Test pending HoG reports across detach

COOLING13d

From: Nicholas Hammond <hidden>
Date: 2026-09-22 01:15:36
Subsystem: the rest · Maintainer: Linus Torvalds

From: Nick Hammond <redacted>

Exercise the real HoG, GAttrib and ATT implementations with socketpairs
for ATT and UHID, without requiring a controller or /dev/uhid.

Cover pending reads, READ_BLOB continuations and writes whose responses
arrive after detach and object destruction. Also check normal completion
and reuse of a surviving object after detach. Keep another GAttrib
reference alive to model the connection shared with other profiles.

Include hog-lib.c to construct a single report and invoke its request
handlers without unrelated service discovery or new production APIs.
The follow-up build change registers this test with make check.

Without the preceding fix, the three teardown cases fail under
AddressSanitizer and the reuse case receives a stale reply. All five
cases pass with the fix and leak detection enabled.

Assisted-by: Codex:gpt-6 AddressSanitizer
---
 unit/test-hog-lifetime.c | 152 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)
 create mode 100644 unit/test-hog-lifetime.c
diff --git a/unit/test-hog-lifetime.c b/unit/test-hog-lifetime.c
new file mode 100644
index 000000000..53a563c40
--- /dev/null
+++ b/unit/test-hog-lifetime.c
@@ -0,0 +1,152 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Regression for outstanding HID report requests surviving detach.
+ * Uses socketpairs and the real HoG/GAttrib/ATT implementation; no hardware.
+ */
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <unistd.h>
+#include <sys/socket.h>
+
+#include <glib.h>
+
+#include "profiles/input/hog-lib.c"
+
+static void pump(void)
+{
+	while (g_main_context_iteration(NULL, FALSE))
+		;
+}
+
+static void test_report(gconstpointer data)
+{
+	const char *name = data;
+	bool blob = !strcmp(name, "blob");
+	bool write_test = !strcmp(name, "write");
+	bool normal = !strcmp(name, "normal");
+	bool reconnect = !strcmp(name, "reconnect");
+	int attfd[2], uhidfd[2];
+	GIOChannel *io;
+	GAttrib *attrib;
+	struct bt_hog *hog;
+	struct report *report;
+	struct uhid_event ev = { 0 }, reply;
+	uint8_t pdu[64];
+	uint8_t response[2] = {
+		write_test ? 0x13 : blob ? 0x0d : 0x0b, 0x55
+	};
+	size_t size = write_test ? 1 : 2;
+	ssize_t n;
+
+	g_assert_cmpint(socketpair(AF_UNIX,
+			SOCK_SEQPACKET | SOCK_NONBLOCK, 0, attfd), ==, 0);
+	g_assert_cmpint(socketpair(AF_UNIX,
+			SOCK_SEQPACKET | SOCK_NONBLOCK, 0, uhidfd), ==, 0);
+	io = g_io_channel_unix_new(attfd[0]);
+	g_io_channel_set_close_on_unref(io, TRUE);
+	attrib = g_attrib_new(io, 23, false);
+	g_io_channel_unref(io);
+	hog = bt_hog_new(uhidfd[0], "lifetime-test", 2, 1, 1, 0, NULL);
+	g_assert(hog && attrib);
+	hog->attrib = g_attrib_ref(attrib);
+	report = g_new0(struct report, 1);
+	report->hog = hog;
+	report->type = HOG_REPORT_TYPE_FEATURE;
+	report->value_handle = 0x31;
+	hog->reports = g_slist_append(hog->reports, report);
+
+	if (write_test) {
+		ev.type = UHID_SET_REPORT;
+		ev.u.set_report.id = 123;
+		ev.u.set_report.rtype = UHID_FEATURE_REPORT;
+		ev.u.set_report.size = 1;
+		ev.u.set_report.data[0] = 0x55;
+		set_report(&ev, hog);
+		g_assert(hog->setrep_att != 0);
+	} else {
+		ev.type = UHID_GET_REPORT;
+		ev.u.get_report.id = 123;
+		ev.u.get_report.rtype = UHID_FEATURE_REPORT;
+		get_report(&ev, hog);
+		g_assert(hog->getrep_att != 0);
+	}
+
+	pump();
+	n = read(attfd[1], pdu, sizeof(pdu));
+	g_assert(n >= 3 && pdu[0] == (write_test ? 0x12 : 0x0a));
+
+	if (blob) {
+		uint8_t first[23] = { 0x0b };
+
+		g_assert_cmpint(write(attfd[1], first, sizeof(first)),
+						==, sizeof(first));
+		pump();
+		n = read(attfd[1], pdu, sizeof(pdu));
+		g_assert(n == 5 && pdu[0] == 0x0c);
+	}
+
+	if (!normal) {
+		bt_hog_detach(hog, true);
+
+		if (!reconnect) {
+			bt_hog_unref(hog);
+			hog = NULL;
+		}
+	}
+
+	g_assert(write(attfd[1], response, size) == size);
+	pump();
+	n = read(uhidfd[1], &reply, sizeof(reply));
+
+	if (normal) {
+		g_assert(n == sizeof(reply));
+		g_assert(reply.type == UHID_GET_REPORT_REPLY);
+		g_assert(reply.u.get_report_reply.id == 123);
+		g_assert(reply.u.get_report_reply.err == 0);
+		g_assert(reply.u.get_report_reply.data[0] == 0x55);
+	} else {
+		g_assert(n == -1 && errno == EAGAIN);
+	}
+
+	if (reconnect) {
+		/* Reuse the detached object for another report request. */
+		g_assert(hog->getrep_att == 0 && hog->setrep_att == 0);
+		hog->attrib = g_attrib_ref(attrib);
+		ev.u.get_report.id = 124;
+		get_report(&ev, hog);
+		pump();
+		n = read(attfd[1], pdu, sizeof(pdu));
+		g_assert(n == 3 && pdu[0] == 0x0a);
+		g_assert(write(attfd[1], response, 2) == 2);
+		pump();
+		g_assert_cmpint(read(uhidfd[1], &reply, sizeof(reply)),
+						==, sizeof(reply));
+		g_assert(reply.u.get_report_reply.id == 124);
+		g_assert(reply.u.get_report_reply.err == 0);
+	}
+
+	if (hog)
+		bt_hog_unref(hog);
+
+	g_attrib_unref(attrib);
+	close(attfd[1]);
+	close(uhidfd[0]);
+	close(uhidfd[1]);
+	pump();
+}
+
+int main(int argc, char *argv[])
+{
+	g_test_init(&argc, &argv, NULL);
+
+	g_test_add_data_func("/hog/detach/read", "read", test_report);
+	g_test_add_data_func("/hog/detach/blob", "blob", test_report);
+	g_test_add_data_func("/hog/detach/write", "write", test_report);
+	g_test_add_data_func("/hog/normal", "normal", test_report);
+	g_test_add_data_func("/hog/reconnect", "reconnect", test_report);
+
+	return g_test_run();
+}
-- 
2.43.0



On Mon, 21 Sep 2026 18:15:15 -0700, Nicholas Hammond
<nicholas.hammond0@gmail.com> wrote:
> From: Nick Hammond <blender7@gmail.com>
>
> This series fixes outstanding HID report callbacks surviving HoG detach.
>
> The investigation started with a bluetoothd crash on Ubuntu BlueZ
> 5.72-0ubuntu5.5 while reconnecting an MX Keys S keyboard after switching
> hosts. The saved stack passes through report_reply/get_report_cb,
> read_blob_helper and attrib_callback_result. The original core and device
> identifiers are intentionally not included.
>
> On current master (17e624d1c), getrep_att and setrep_att still bypass the
> gatt_op queue cancelled by bt_hog_detach. If another owner retains the
> GAttrib connection, a late response can invoke a callback after its HoG
> object/report is freed. A surviving HoG object can also receive a stale
> reply after detach.
>
> A socketpair test on that revision reproduces the read, long-read and
> write use-after-free under AddressSanitizer. For example, the long-read
> case reports:
>
>   ERROR: AddressSanitizer: heap-use-after-free
>     get_report_cb       profiles/input/hog-lib.c:929
>     read_blob_helper    attrib/gatt.c:791
>     attrib_callback_result attrib/gattrib.c:235
>     handle_rsp          src/shared/att.c:913
>
> This is a lifetime regression test with a shared live GAttrib reference,
> not an over-the-air reproducer or a claim of unauthenticated reachability.
> It does not establish that every historical keyboard outage had this
> cause.
>
> Patch 1 cancels the separately tracked report requests before dropping
> attrib. Patches 2 and 3 add and register five hardware-independent tests.
> The test includes hog-lib.c to access private report state, avoiding new
> production APIs and unrelated discovery traffic.
>
> Validation on current master plus this series:
> - AddressSanitizer with detect_leaks=1: all five new tests pass.
> - Existing test-hog (6), test-gattrib (6), test-gatt (193): all pass.
> - checkpatch --no-tree --no-signoff: no errors or warnings.
> - A backport to Ubuntu 5.72 is installed on the affected host; keyboard
>   and mouse reconnected after restarting the daemon. Long-term stability
>   is not yet established.
>
> To reproduce the failures, apply only patches 2 and 3 to the base revision
> and build with --enable-asan. Run unit/test-hog-lifetime -p followed by
> /hog/detach/read, /hog/detach/blob or /hog/detach/write. Each detects a
> use-after-free without patch 1. /hog/reconnect catches the stale reply;
> /hog/normal passes before and after the fix.
>
> AI assistance disclosure: Codex:gpt-6 analyzed the saved crash, prepared
> the fix and tests, and ran the validation commands. This contribution
> includes Assisted-by tags. No claim of an independent human code review
> is made.
>
> Thanks,
> Nick Hammond
>
> Nick Hammond (3):
>   input/hog: Cancel pending report requests on detach
>   unit: Test pending HoG reports across detach
>   build: Register the HoG report lifetime tests
>
>  Makefile.am              |  14 ++++
>  profiles/input/hog-lib.c |  11 +++
>  unit/test-hog-lifetime.c | 152 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 177 insertions(+)
>  create mode 100644 unit/test-hog-lifetime.c
>
>
> base-commit: 17e624d1cd9a4d94f90e0ca008548456c3455492
> --
> 2.43.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help