[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

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: [v2] 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=1143156

---Test result---

Test Summary:
CheckPatch                    PASS      0.75 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       PASS      0.67 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      27.33 seconds
CheckAllWarning               PASS      29.56 seconds
CheckSparse                   PASS      28.05 seconds
BuildKernel32                 PASS      28.68 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      497.18 seconds
IncrementalBuild              PASS      25.38 seconds

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


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

---
Regards,
Linux Bluetooth

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