Bluetooth: hci_uart: fix UAFs and race conditions in close and init paths

Vulnerabilities leading to Use-After-Free (UAF) and Null Pointer
Dereference (NPD) conditions were observed in the lifecycle management
of hci_uart.

The primary issue arises because the workqueues (init_ready and
write_work) are only flushed/cancelled if the HCI_UART_PROTO_READY
flag is set during TTY close. If a hangup occurs before setup completes,
hci_uart_tty_close() skips the teardown of these workqueues and
proceeds to free the `hu` struct. When the scheduled work executes
later, it blindly dereferences the freed `hu` struct.

Furthermore, several data races and UAFs were identified in the teardown
sequence:
1. Calling hci_uart_flush() from hci_uart_close() without effectively
   disabling write_work causes a race condition where both can concurrently
   double-free hu->tx_skb. This happens because protocol timers can
   concurrently invoke hci_uart_tx_wakeup() and requeue write_work.
2. Calling hci_free_dev(hdev) before hu->proto->close(hu) causes a UAF
   when vendor specific protocol close callbacks dereference hu->hdev.
3. In the initialization error paths, failing to take the proto_lock
   write lock before clearing PROTO_READY leads to races with active
   readers. Additionally, hci_uart_tty_receive() accesses hu->hdev
   outside the read lock, leading to UAFs if the initialization error
   path frees hdev concurrently.

Fix these synchronization and lifecycle issues by:
1. Re-ordering hci_uart_tty_close() to clear HCI_UART_PROTO_READY first,
   followed immediately by a cancel_work_sync(&hu->write_work). Clearing
   the flag locks out concurrent protocol timers from successfully invoking
   hci_uart_tx_wakeup(), effectively rendering the cancellation permanent
   and preventing the tx_skb double-free.
2. Note: Clearing PROTO_READY early causes hci_uart_close() to skip
   hu->proto->flush(). This is perfectly safe in the tty_close path
   because hu->proto->close() executes shortly after, which intrinsically
   purges all protocol SKB queues and tears down the state.
3. Relocating hu->proto->close(hu) strictly prior to hci_free_dev(hdev)
   across all close and error paths to prevent vendor-level UAFs.
4. Moving the hdev->stat.byte_rx increment in hci_uart_tty_receive()
   inside the proto_lock read-side critical section to safely synchronize
   with device unregistration.
5. Adding cancel_work_sync(&hu->write_work) to hci_uart_close() to safely
   flush the workqueue before hci_uart_flush() is invoked via the HCI core.
6. Utilizing cancel_work_sync() instead of disable_work_sync() across
   all paths to prevent permanently breaking user-space retry capabilities.

Fixes: 3b799254cf6f ("Bluetooth: hci_uart: Cancel init work before unregistering")
Cc: stable@vger.kernel.org
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 275ea865bc297b2725e0ae83c3d2210030467b24..47f4902b40b47dd31eacb62a11edeceb9b47d8f8 100644 (file)
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -194,7 +194,15 @@ void hci_uart_init_work(struct work_struct *work)
        err = hci_register_dev(hu->hdev);
        if (err < 0) {
                BT_ERR("Can't register HCI device");
+
+               percpu_down_write(&hu->proto_lock);
                clear_bit(HCI_UART_PROTO_READY, &hu->flags);
+               percpu_up_write(&hu->proto_lock);
+
+               /* Safely cancel work after clearing flags */
+               cancel_work_sync(&hu->write_work);
+
+               /* Close protocol before freeing hdev */
                hu->proto->close(hu);
                hdev = hu->hdev;
                hu->hdev = NULL;
@@ -263,8 +271,12 @@ static int hci_uart_open(struct hci_dev *hdev)
 /* Close device */
 static int hci_uart_close(struct hci_dev *hdev)
 {
+       struct hci_uart *hu = hci_get_drvdata(hdev);
+
        BT_DBG("hdev %p", hdev);
 
+       cancel_work_sync(&hu->write_work);
+
        hci_uart_flush(hdev);
        hdev->flush = NULL;
        return 0;
@@ -531,6 +543,7 @@ static void hci_uart_tty_close(struct tty_struct *tty)
 {
        struct hci_uart *hu = tty->disc_data;
        struct hci_dev *hdev;
+       bool proto_ready;
 
        BT_DBG("tty %p", tty);
 
@@ -540,24 +553,38 @@ static void hci_uart_tty_close(struct tty_struct *tty)
        if (!hu)
                return;
 
-       hdev = hu->hdev;
-       if (hdev)
-               hci_uart_close(hdev);
+       /* Wait for init_ready to finish to prevent registration races */
+       cancel_work_sync(&hu->init_ready);
 
-       if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) {
+       proto_ready = test_bit(HCI_UART_PROTO_READY, &hu->flags);
+       if (proto_ready) {
                percpu_down_write(&hu->proto_lock);
                clear_bit(HCI_UART_PROTO_READY, &hu->flags);
                percpu_up_write(&hu->proto_lock);
+       }
 
-               cancel_work_sync(&hu->init_ready);
-               cancel_work_sync(&hu->write_work);
+       /*
+        * Unconditionally cancel write_work AFTER clearing PROTO_READY.
+        * This ensures that concurrent protocol timers cannot requeue
+        * write_work via hci_uart_tx_wakeup(), permanently preventing
+        * double-free races and UAFs.
+        */
+       cancel_work_sync(&hu->write_work);
+
+       hdev = hu->hdev;
+       if (hdev)
+               hci_uart_close(hdev); /* proto->flush is safely skipped */
 
+       if (proto_ready) {
                if (hdev) {
                        if (test_bit(HCI_UART_REGISTERED, &hu->flags))
                                hci_unregister_dev(hdev);
-                       hci_free_dev(hdev);
                }
+               /* Close protocol before freeing hdev (intrinsically purges queues) */
                hu->proto->close(hu);
+
+               if (hdev)
+                       hci_free_dev(hdev);
        }
        clear_bit(HCI_UART_PROTO_SET, &hu->flags);
 
@@ -625,11 +652,12 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
         * tty caller
         */
        hu->proto->recv(hu, data, count);
-       percpu_up_read(&hu->proto_lock);
 
        if (hu->hdev)
                hu->hdev->stat.byte_rx += count;
 
+       percpu_up_read(&hu->proto_lock);
+
        tty_unthrottle(tty);
 }
 
@@ -695,6 +723,10 @@ static int hci_uart_register_dev(struct hci_uart *hu)
                percpu_down_write(&hu->proto_lock);
                clear_bit(HCI_UART_PROTO_INIT, &hu->flags);
                percpu_up_write(&hu->proto_lock);
+               /* Cancel work after clearing flags */
+               cancel_work_sync(&hu->write_work);
+
+               /* Close protocol before freeing hdev */
                hu->proto->close(hu);
                hu->hdev = NULL;
                hci_free_dev(hdev);