linux-kernel - Re: [PATCH] Bluetooth: btusb: Prevent autosuspend when le_scan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABBYNZJZeP1eFZXrVPMAhEtdNVSWkuAkyopLcrUG6wbQpvgsww@mail.gmail.com>
Date: Fri, 31 Oct 2025 10:35:22 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Linmao Li <lilinmao@...inos.cn>
Cc: linux-bluetooth@...r.kernel.org, marcel@...tmann.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: btusb: Prevent autosuspend when
 le_scan_disable work is pending

Hi,

On Fri, Oct 31, 2025 at 4:15 AM Linmao Li <lilinmao@...inos.cn> wrote:
>
> When USB autosuspend occurs while le_scan_disable work is scheduled,
> HCI commands sent by the work fail with timeout, leaving LE scan in
> an inconsistent state.
>
> Scenario:
>   T=0:     LE scan starts, le_scan_disable work queued (+10240ms)
>   T=8s:    Autosuspend check: tx_in_flight=0, suspend proceeds
>   T=10s:   le_scan_disable work executes on suspended device
>            → HCI command 0x2042 times out
>
> The tx_in_flight check only protects actively transmitted URBs, missing
> the window where work is queued but hasn't submitted its URB yet.
>
> Fix by checking delayed_work_pending(&hdev->le_scan_disable) during
> autosuspend. Return -EBUSY if pending to block suspend until work
> completes. Only set BTUSB_SUSPENDING after all checks pass to avoid
> leaving the flag set if suspend is aborted.

Hmm, we could also just cancel the work then, in face we do have
hci_suspend_sync->hci_stop_discovery_sync->cancel_delayed_work(&hdev->le_scan_disable);
but perhaps it is not being called always because HCI_LE_SCAN is not
enabled, so I wonder if we should do something like:

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 1cbdd2ce03f2..59618fde7bcb 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5464,6 +5464,11 @@ int hci_stop_discovery_sync(struct hci_dev *hdev)

        bt_dev_dbg(hdev, "state %u", hdev->discovery.state);

+       /* Always stop le_scan_disable since that works as discovery timer for
+        * the rounds of discovery irrespective of the discovery type.
+        */
+       cancel_delayed_work(&hdev->le_scan_disable);
+
        if (d->state == DISCOVERY_FINDING || d->state == DISCOVERY_STOPPING) {
                if (test_bit(HCI_INQUIRY, &hdev->flags)) {
                        err = __hci_cmd_sync_status(hdev, HCI_OP_INQUIRY_CANCEL,
@@ -5472,14 +5477,9 @@ int hci_stop_discovery_sync(struct hci_dev *hdev)
                                return err;
                }

-               if (hci_dev_test_flag(hdev, HCI_LE_SCAN)) {
-                       cancel_delayed_work(&hdev->le_scan_disable);
-
-                       err = hci_scan_disable_sync(hdev);
-                       if (err)
-                               return err;
-               }
-
+               err = hci_scan_disable_sync(hdev);
+               if (err)
+                       return err;
        } else {
                err = hci_scan_disable_sync(hdev);
                if (err)

We may as well rename/reworkd le_scan_disable to discovery.work since
that acts as I commented above it works as discovery timer not just
le_scan_disable.

> Signed-off-by: Linmao Li <lilinmao@...inos.cn>
> ---
>  drivers/bluetooth/btusb.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 5e9ebf0c5312..a344ea1dc466 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -4389,6 +4389,7 @@ static void btusb_disconnect(struct usb_interface *intf)
>  static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>  {
>         struct btusb_data *data = usb_get_intfdata(intf);
> +       struct hci_dev *hdev = data->hdev;
>
>         BT_DBG("intf %p", intf);
>
> @@ -4402,14 +4403,19 @@ static int btusb_suspend(struct usb_interface *intf, pm_message_t message)
>                 return 0;
>
>         spin_lock_irq(&data->txlock);
> -       if (!(PMSG_IS_AUTO(message) && data->tx_in_flight)) {
> -               set_bit(BTUSB_SUSPENDING, &data->flags);
> -               spin_unlock_irq(&data->txlock);
> -       } else {
> +       if (PMSG_IS_AUTO(message) && data->tx_in_flight) {
>                 spin_unlock_irq(&data->txlock);
>                 data->suspend_count--;
>                 return -EBUSY;
>         }
> +       spin_unlock_irq(&data->txlock);
> +
> +       if (PMSG_IS_AUTO(message) && delayed_work_pending(&hdev->le_scan_disable)) {
> +               data->suspend_count--;
> +               return -EBUSY;
> +       }



> +
> +       set_bit(BTUSB_SUSPENDING, &data->flags);
>
>         cancel_work_sync(&data->work);
>
> --
> 2.25.1
>


-- 
Luiz Augusto von Dentz