[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: [PATCH] Bluetooth: hci_serdev: Fix use-after-free in hci_uart_unregister_device()

[Luiz Augusto von Dentz]

Hi ZhaoJinming,

On Wed, Aug 5, 2026 at 6:36 AM ZhaoJinming <zhaojinming@uniontech.com> wrote:
>
> 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

It looks like sashiko found some problems with these changes:

https://sashiko.dev/#/patchset/0BDE51B0554940FB%2B20260805103612.916678-1-zhaojinming%40uniontech.com

-- 
Luiz Augusto von Dentz

[PATCH v2] 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
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 <zhaojinming@uniontech.com>
---
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 <zhaojinming@uniontech.com>

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

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 10 Aug 2026 15:08:45 +0800 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v2] Bluetooth: hci_serdev: Fix use-after-free in hci_uart_unregister_device()
    https://git.kernel.org/bluetooth/bluetooth-next/c/3ea6bd320276

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html