[PATCH] Bluetooth: hci_serdev: Fix use-after-free in hci_uart_unregister_device()

[PATCH] Bluetooth: hci_serdev: Fix use-after-free in hci_uart_unregister_device()

[ZhaoJinming]

hci_uart_unregister_device() frees the HCI device (hci_free_dev)
before cancelling write_work via cancel_work_sync(). If write_work
is executing concurrently on another CPU, it can access hu->hdev
(serdev.c:61) and write to hdev->stat (serdev.c:75, 83) after the
memory has been freed.

Additionally, HCI_UART_PROTO_READY is not cleared until after
cancel_work_sync, so the write_wakeup serdev callback can still
schedule write_work via hci_uart_tx_wakeup() even after
hci_free_dev has freed the device.

Fix this by mirroring the same ordering used in the tty/ldisc path
(hci_uart_tty_close, hci_ldisc.c:565-593):
1. Clear HCI_UART_PROTO_READY and close the serdev port
2. Cancel write_work (no new work can be scheduled)
3. Unregister the HCI device
4. Close the protocol (may access hu->hdev, e.g. bcm_close)
5. Free the HCI device

Signed-off-by: ZhaoJinming <zhaojinming@uniontech.com>
---
 drivers/bluetooth/hci_serdev.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 593d9cefbbf9..e06b9d5b3846 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -397,18 +397,22 @@ void hci_uart_unregister_device(struct hci_uart *hu)
 	struct hci_dev *hdev = hu->hdev;
 
 	cancel_work_sync(&hu->init_ready);
-	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
-		hci_unregister_dev(hdev);
-	hci_free_dev(hdev);
-
-	cancel_work_sync(&hu->write_work);
-
-	hu->proto->close(hu);
 
+	/* Clear HCI_UART_PROTO_READY first to prevent the write_wakeup
+	 * callback from re-scheduling write_work via hci_uart_tx_wakeup().
+	 */
 	if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
 		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
 		serdev_device_close(hu->serdev);
 	}
+
+	cancel_work_sync(&hu->write_work);
+
+	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
+		hci_unregister_dev(hdev);
+
+	hu->proto->close(hu);
+	hci_free_dev(hdev);
 	percpu_free_rwsem(&hu->proto_lock);
 }
 EXPORT_SYMBOL_GPL(hci_uart_unregister_device);
-- 
2.51.0

RE: Bluetooth: hci_serdev: Fix use-after-free in hci_uart_unregister_device()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1140705

---Test result---

Test Summary:
CheckPatch                    PASS      0.59 seconds
VerifyFixes                   PASS      0.09 seconds
VerifySignedoff               PASS      0.09 seconds
GitLint                       PASS      0.24 seconds
SubjectPrefix                 PASS      0.08 seconds
BuildKernel                   PASS      27.41 seconds
CheckAllWarning               PASS      30.71 seconds
CheckSparse                   PASS      29.08 seconds
BuildKernel32                 PASS      27.16 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      496.65 seconds
IncrementalBuild              PASS      24.97 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


https://github.com/bluez/bluetooth-next/pull/534

---
Regards,
Linux Bluetooth