[PATCH 6.6.y] Bluetooth: hci_core: Fix UAF in hci_unregister_dev()

[PATCH 6.6.y] Bluetooth: hci_core: Fix UAF in hci_unregister_dev()

[Jordan Walters]

commit eec3deaeaafe upstream.

[manual backport: 6.6.y uses cancel_*_work_sync() instead of
 disable_*_work_sync() which was introduced in a later cycle]

hci_unregister_dev() does not cancel cmd_timer and ncmd_timer
before the hci_dev structure is freed. If a timeout fires
during device teardown, the callback dereferences freed memory
(including the hdev->reset function pointer), leading to a
use-after-free.

Add cancel_delayed_work_sync() calls alongside the existing
cancel_work_sync() calls to ensure both timers are fully
quiesced before teardown proceeds.

Signed-off-by: Jordan Walters <jaggyaur@gmail.com>
---
 net/bluetooth/hci_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 824208a53c2..b0e0a553892 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2701,6 +2701,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	cancel_work_sync(&hdev->tx_work);
 	cancel_work_sync(&hdev->power_on);
 	cancel_work_sync(&hdev->error_reset);
+	cancel_delayed_work_sync(&hdev->cmd_timer);
+	cancel_delayed_work_sync(&hdev->ncmd_timer);
 
 	hci_cmd_sync_clear(hdev);
 
-- 
2.43.0

Re: [PATCH 6.6.y] Bluetooth: hci_core: Fix UAF in hci_unregister_dev()

[Sasha Levin]

> [PATCH 6.6.y] Bluetooth: hci_core: Fix UAF in hci_unregister_dev()

Thanks for the patch, but I can't take this as-is -- a v2 is needed:

1) The "commit ... upstream" line cites a hash that doesn't exist in
   mainline, and there's no upstream commit titled "Fix UAF in
   hci_unregister_dev()".

2) The diff doesn't match that fix: upstream uses disable_work_sync() /
   disable_delayed_work_sync() (guarded by HCI_UNREGISTER), not an added
   cancel_delayed_work_sync() in hci_unregister_dev(). cancel_* doesn't
   prevent re-queue, and hci_unregister_dev() later calls
   hci_dev_close_sync() which can re-touch those timers, so this may not
   actually close the UAF.

-- 
Thanks,
Sasha