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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> ---
>  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