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

ZhaoJinming <[email protected]>
Newsgroups org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel
Message-ID <D747A53C08BD2456+20260810-bluetooth-hci-serdev-uart-unregister-v2-1-690493a6ab95@uniontech.com>
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
and write to hdev->stat 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. Save the PROTO_READY state and clear it under the write lock so
   a concurrent hci_uart_tx_wakeup() cannot re-schedule write_work
2. Cancel write_work (no new work can be scheduled and no work is
   in flight)
3. Unregister the HCI device
4. Close the protocol (may access hu->hdev and the serdev device)
5. Close the serdev port (safe now that write_work is quiesced and
   protocol is done)
6. Free the HCI device

Also free any partially transmitted frame (hu->tx_skb) left over by
write_work once the transmit path is quiesced, since hci_uart_close()
would skip hci_uart_flush() because HCI_UART_PROTO_READY is cleared.

Signed-off-by: ZhaoJinming <[email protected]>
---
Fix a use-after-free in hci_uart_unregister_device() where the HCI
device could be freed (hci_free_dev) before write_work was cancelled,
allowing a concurrently running write_work to access hu->hdev after
the memory had been freed.

The teardown sequence is reordered to mirror the tty/ldisc path
(hci_uart_tty_close) and to account for the serdev-specific teardown:
the serdev port is closed only after write_work and the protocol are
fully torn down, and any partially transmitted frame is freed once the
transmit path is quiesced.

Changes in v2:
- Clear HCI_UART_PROTO_READY under percpu_down_write() to prevent a
  concurrent hci_uart_tx_wakeup() from re-scheduling write_work via
  the write_wakeup callback once the device is torn down.
- Cancel write_work before closing the serdev device to avoid a
  use-after-free in the serdev/TTY backend.
- Close the serdev device after the protocol close, since some protocol
  close handlers (e.g. qca_close) still access the serdev device.
- Free any partially transmitted frame (hu->tx_skb) after write_work is
  quiesced.
---
 drivers/bluetooth/hci_serdev.c | 46 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/hci_serdev.c b/drivers/bluetooth/hci_serdev.c
index 593d9cefbbf925b4d3f8d37a12420a99e08a9477..13346c205591056eaca6e4f0dae53db398064230 100644
--- a/drivers/bluetooth/hci_serdev.c
+++ b/drivers/bluetooth/hci_serdev.c
@@ -395,20 +395,54 @@ EXPORT_SYMBOL_GPL(hci_uart_register_device_priv);
 void hci_uart_unregister_device(struct hci_uart *hu)
 {
 	struct hci_dev *hdev = hu->hdev;
+	bool proto_ready;
 
+	/* Wait for init_ready to finish to prevent registration races */
 	cancel_work_sync(&hu->init_ready);
-	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
-		hci_unregister_dev(hdev);
-	hci_free_dev(hdev);
 
+	proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
+	if (proto_ready) {
+		/* Clear HCI_UART_PROTO_READY under the write lock so a
+		 * concurrent hci_uart_tx_wakeup() cannot re-schedule
+		 * write_work via the write_wakeup callback once the device
+		 * is torn down.
+		 */
+		percpu_down_write(&hu->proto_lock);
+		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+		percpu_up_write(&hu->proto_lock);
+	}
+
+	/* Unconditionally cancel write_work AFTER clearing PROTO_READY.
+	 * This ensures that concurrent protocol timers cannot requeue
+	 * write_work, permanently preventing double-free races and UAFs,
+	 * and guarantees no write_work is in flight before the serdev
+	 * device is closed.
+	 */
 	cancel_work_sync(&hu->write_work);
 
+	/* Free any partially transmitted frame left over by write_work now
+	 * that the transmit path is fully quiesced. hci_uart_close() would
+	 * skip hci_uart_flush() because HCI_UART_PROTO_READY is cleared.
+	 */
+	if (hu->tx_skb) {
+		kfree_skb(hu->tx_skb);
+		hu->tx_skb = NULL;
+	}
+
+	if (test_bit(HCI_UART_REGISTERED, &hu->flags))
+		hci_unregister_dev(hdev);
+
+	/* Close the protocol before freeing hdev (intrinsically purges queues).
+	 * Some protocol close handlers (e.g. qca_close) may still access the
+	 * serdev device, so keep the serdev port open until this completes.
+	 */
 	hu->proto->close(hu);
 
-	if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
-		clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+	if (proto_ready)
 		serdev_device_close(hu->serdev);
-	}
+
+	hci_free_dev(hdev);
+
 	percpu_free_rwsem(&hu->proto_lock);
 }
 EXPORT_SYMBOL_GPL(hci_uart_unregister_device);

---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260810-bluetooth-hci-serdev-uart-unregister-ce37ebb29283

Best regards,
-- 
ZhaoJinming <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.