Re: [PATCH] Bluetooth: hci_sync: Fix accept list UAF during suspend

[Luiz Augusto von Dentz]

Hi Chengfeng,

On Thu, Jul 30, 2026 at 5:23 AM Chengfeng Ye <nicoyip.dev@xxxxxxxxx> wrote:
>
> hci_update_event_filter_sync() walks hdev->accept_list while sending a
> synchronous HCI command for each remote-wakeup device.  The suspend path
> holds hdev->req_lock, but accept-list updates are serialized by hdev->lock.
> Consequently, remove_device() can free the current list entry during the
> controller wait.
>
> The following interleaving causes the use-after-free:
>
>   hci_update_event_filter_sync()    remove_device()
>   fetch accept-list entry
>   hci_set_event_filter_sync()
>     wait for controller response    hci_dev_lock()
>                                     list_del()
>                                     kfree()
>                                     hci_dev_unlock()
>   read the freed list.next
>
> KASAN reported:
>
>   BUG: KASAN: slab-use-after-free in hci_suspend_sync+0x835/0x910
>   Read of size 8 at addr ffff88810bec8440 by task kworker/0:1/10
>   Workqueue: events vhci_suspend_work
>   Call Trace:
>    hci_suspend_sync+0x835/0x910
>    hci_suspend_dev+0x182/0x450
>    process_one_work+0x661/0x1090
>    worker_thread+0x45b/0xd10
>
>   Allocated by task 86:
>    hci_bdaddr_list_add_with_flags+0x1a8/0x400
>    add_device+0x381/0x820
>    hci_sock_sendmsg+0x1033/0x1ea0
>
>   Freed by task 91:
>    kfree+0x131/0x3c0
>    remove_device+0x429/0xb70
>    hci_sock_sendmsg+0x1033/0x1ea0
>
> Snapshot the remote-wakeup addresses under hdev->lock.  Release the lock
> before sending HCI commands.  This preserves list order and avoids
> retaining an accept-list node across a controller wait.
>
> Fixes: 182ee45da083 ("Bluetooth: hci_sync: Rework hci_suspend_notifier")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> ---
>  net/bluetooth/hci_sync.c | 38 +++++++++++++++++++++++++++++++-------
>  1 file changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index c0b1fc293b49..540da19d1d64 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -6250,6 +6250,8 @@ static int hci_pause_discovery_sync(struct hci_dev *hdev)
>  static int hci_update_event_filter_sync(struct hci_dev *hdev)
>  {
>         struct bdaddr_list_with_flags *b;
> +       bdaddr_t *accept_list = NULL;
> +       size_t i, num_entries = 0;
>         u8 scan = SCAN_DISABLED;
>         bool scanning = test_bit(HCI_PSCAN, &hdev->flags);
>         int err;
> @@ -6263,26 +6265,48 @@ static int hci_update_event_filter_sync(struct hci_dev *hdev)
>         if (hci_test_quirk(hdev, HCI_QUIRK_BROKEN_FILTER_CLEAR_ALL))
>                 return 0;
>
> +       hci_dev_lock(hdev);
> +
> +       list_for_each_entry(b, &hdev->accept_list, list)
> +               if (b->flags & HCI_CONN_FLAG_REMOTE_WAKEUP)
> +                       num_entries++;


If !num_entries we can probably bail out here, or we do need to call
hci_clear_event_filter_sync? If so, perhaps that should be called
first.

> +       if (num_entries) {
> +               accept_list = kmalloc_array(num_entries, sizeof(*accept_list),
> +                                           GFP_KERNEL);
> +               if (!accept_list) {
> +                       hci_dev_unlock(hdev);
> +                       return -ENOMEM;
> +               }
> +       }
> +
> +       i = 0;
> +       list_for_each_entry(b, &hdev->accept_list, list)
> +               if (b->flags & HCI_CONN_FLAG_REMOTE_WAKEUP)
> +                       bacpy(&accept_list[i++], &b->bdaddr);
> +
> +       hci_dev_unlock(hdev);

While this works, iterating twice if there are no entries is wasteful;
we should bail out or rework this.

>         /* Always clear event filter when starting */
>         hci_clear_event_filter_sync(hdev);
>
> -       list_for_each_entry(b, &hdev->accept_list, list) {
> -               if (!(b->flags & HCI_CONN_FLAG_REMOTE_WAKEUP))
> -                       continue;
> -
> -               bt_dev_dbg(hdev, "Adding event filters for %pMR", &b->bdaddr);
> +       for (i = 0; i < num_entries; i++) {
> +               bt_dev_dbg(hdev, "Adding event filters for %pMR",
> +                          &accept_list[i]);
>
>                 err =  hci_set_event_filter_sync(hdev, HCI_FLT_CONN_SETUP,
>                                                  HCI_CONN_SETUP_ALLOW_BDADDR,
> -                                                &b->bdaddr,
> +                                                &accept_list[i],
>                                                  HCI_CONN_SETUP_AUTO_ON);
>                 if (err)
>                         bt_dev_err(hdev, "Failed to set event filter for %pMR",
> -                                  &b->bdaddr);
> +                                  &accept_list[i]);
>                 else
>                         scan = SCAN_PAGE;
>         }
>
> +       kfree(accept_list);
> +
>         if (scan && !scanning)
>                 hci_write_scan_enable_sync(hdev, scan);
>         else if (!scan && scanning)
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz