Bug 208409

Summary:	segfault in bluetoothd on mouse pairing
Product:	Drivers	Reporter:	David Arroyo (david)
Component:	Bluetooth	Assignee:	linux-bluetooth (linux-bluetooth)
Status:	NEW ---
Severity:	normal
Priority:	P1
Hardware:	Intel
OS:	Linux
Kernel Version:	5.4.49	Subsystem:
Regression:	No	Bisected commit-id:
Attachments:	output of bluetoothd -n -d btmon trace

Description David Arroyo 2020-07-02 05:27:10 UTC

Created attachment 290051 [details]
output of bluetoothd -n -d

I have a "3D connexion cadmouse pro wireless" that reliably causes bluetoothd to segfault whenever I try to pair it with my Dell XPS 13 (9350). Here is the stack trace:

	#0  g_io_channel_unix_get_fd (channel=0x0) at ../glib/giounix.c:656
	#1  0x00005555555b68a3 in bt_io_get_type (io=<optimized out>, gerr=0x7fffffffc2a0) at btio/btio.c:105
	#2  0x00005555555b88dc in bt_io_get (io=0x0, err=0x7fffffffc2a0, opt1=BT_IO_OPT_SOURCE) at btio/btio.c:1513
	#3  0x00005555555a1e82 in report_map_read_cb (status=<optimized out>, pdu=<optimized out>, plen=<optimized out>,
	    user_data=<optimized out>) at profiles/input/hog-lib.c:992
	#4  0x00005555555b335c in read_blob_helper (status=0 '\000', rpdu=<optimized out>, rlen=13,
	    user_data=0x5555556d3eb0) at attrib/gatt.c:804
	#5  0x00005555555b44c6 in attrib_callback_result (opcode=<optimized out>, pdu=0x5555556d5c01,
	    length=<optimized out>, user_data=0x5555556c6880) at attrib/gattrib.c:273
	#6  0x000055555560b950 in handle_rsp (att=0x5555556c4200, opcode=<optimized out>, pdu=<optimized out>,
	    pdu_len=<optimized out>) at src/shared/att.c:715
	#7  0x000055555560baed in can_read_data (io=<optimized out>, user_data=0x5555556c4200) at src/shared/att.c:904
	#8  0x0000555555615429 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>)
	    at src/shared/io-glib.c:170
	#9  0x00007ffff7ef9c1e in g_main_dispatch (context=0x555555689d30) at ../glib/gmain.c:3179
	#10 g_main_context_dispatch (context=context@entry=0x555555689d30) at ../glib/gmain.c:3844
	#11 0x00007ffff7ef9fd0 in g_main_context_iterate (context=0x555555689d30, block=block@entry=1,
	    dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:3917
	#12 0x00007ffff7efa2a3 in g_main_loop_run (loop=0x55555568aee0) at ../glib/gmain.c:4111
	#13 0x00005555556159b1 in mainloop_run () at src/shared/mainloop-glib.c:79
	#14 0x0000555555615db8 in mainloop_run_with_signal (func=<optimized out>, user_data=0x0)
	    at src/shared/mainloop-notify.c:201
	#15 0x00005555555ba6b5 in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:729

I stepped through a core dump with gdb. The mouse is sending a BT_ATT_OP_READ_BLOB_RSP command. report_map_read_cb passes a NULL pointer to g_attrib_get_channel here: https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/profiles/input/hog-lib.c?h=5.53&id=1524499483a3678951c0e3059b158836398c4e9b#n992 which is not checked before being dereferenced.

btmon trace to follow...

Comment 1 David Arroyo 2020-07-02 05:28:16 UTC

Created attachment 290053 [details]
btmon trace

Comment 2 David Arroyo 2020-07-02 21:41:20 UTC

This appears to be a 'use after free'-type issue. In report_map_read_cb,

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/profiles/input/hog-lib.c?h=5.53&id=1524499483a3678951c0e3059b158836398c4e9b#n950

destroy_gatt_req(req) decrements the reference count for req->hog. When it reaches zero, the call stack destroy_gatt_req -> bt_hog_unref -> bt_hog_free -> bt_hog_detach sets hog->attrib to NULL. Then, later in report_map_read_cb, there is

	bt_io_get(g_attrib_get_channel(hog->attrib), &gerr,
			BT_IO_OPT_SOURCE, ev.u.create.phys,
			BT_IO_OPT_DEST, ev.u.create.uniq,
			BT_IO_OPT_INVALID);

which is invalid if hog's refcount reaches zero. I can see that there is a very common pattern in hog-lib.c:

	struct bt_hog *hog = req->user_data;
	...
	destroy_gatt_req(req);
	...
	/* Do stuff with hog */

This pattern doesn't seem safe since destroy_gatt_req decrements req->hog's refcount, and several other reference counts. I think the safest thing would be to move destroy_gatt_req to the end of each callback's execution, but I still don't understand the code well enough to be sure.

That said, I have a bluetooth keyboard that I can connect to this machine just fine (I'm typing on it now). So clearly some devices do not trigger this behavior. Is the report_map_read_cb callback not triggered by most hardware?

Comment 3 David Arroyo 2020-07-02 23:22:50 UTC

The change I described in the previous comment of moving destroy_gatt_req didn't work; the first segfault was avoided, but I eventually got a double-free. Back to the drawing board...