[RFC PATCH] Bluetooth: btusb: wait for rx_work before freeing data on disconnect

Philipp Weber posted 1 patch 5 days, 9 hours ago
drivers/bluetooth/btusb.c | 9 +++++++++
1 file changed, 9 insertions(+)
[RFC PATCH] Bluetooth: btusb: wait for rx_work before freeing data on disconnect
Posted by Philipp Weber 5 days, 9 hours ago
syzbot reports a slab-use-after-free in skb_dequeue() called from
btusb_rx_work(), with the freed object being the btusb_data struct
released by btusb_disconnect() via usb_unbind_interface() -> kfree().

The race:

  btusb_close() (via hci_unregister_dev -> hdev->close)
    cancel_delayed_work(&data->rx_work);   <-- non-sync
    ...
    btusb_stop_traffic(data);              <-- kills URBs

A URB completion callback fired between the non-sync cancel and
btusb_stop_traffic() can call data->recv_acl() -> hci_recv_frame(),
which enqueues to data->acl_q and schedules data->rx_work again.
The cancel above already returned, so the newly-scheduled rx_work
is left pending. btusb_disconnect() then proceeds to kfree(data)
while rx_work may still execute, dereferencing data->acl_q in
skb_dequeue().

Drain rx_work in btusb_disconnect() before kfree(data). At that
point hci_unregister_dev() has fully returned, btusb_close() has
already killed all URBs via btusb_stop_traffic(), so no new
scheduling can happen. Any rx_work item that was re-scheduled by a
late URB callback in the close path is guaranteed to be drained.

This runs without hci_req_sync_lock held (it was acquired by
hci_dev_do_close and released before btusb_disconnect resumes), so
the sync cancel has no deadlock interaction with the close path.

Fixes: 800fe5ec302e ("Bluetooth: btusb: Add support for queuing during polling interval")
Reported-by: syzbot+d06554f43a8fb48030b0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d06554f43a8fb48030b0
Signed-off-by: Philipp Weber <kernel@phwe.de>
---
 drivers/bluetooth/btusb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7f5fce93d984..5d4ea44cd3c9 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -4462,6 +4462,15 @@ static void btusb_disconnect(struct usb_interface *intf)
 		usb_driver_release_interface(&btusb_driver, data->intf);
 	}
 
+	/*
+	 * rx_work is scheduled from URB completion handlers; btusb_close()
+	 * (called via hci_unregister_dev) uses a non-sync cancel, so a work
+	 * item may still be queued or executing when we reach this point.
+	 * Wait for it before freeing data, otherwise the worker dereferences
+	 * freed memory through skb_dequeue(&data->acl_q).
+	 */
+	cancel_delayed_work_sync(&data->rx_work);
+
 	hci_free_dev(hdev);
 	kfree(data);
 }

base-commit: ab5fce87a778cb780a05984a2ca448f2b41aafbf
-- 
2.53.0