Re: [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work

[Luiz Augusto von Dentz]

Hi Chengfeng,

On Thu, Jul 30, 2026 at 6:41 AM Chengfeng Ye <nicoyip.dev@xxxxxxxxx> wrote:
>
> le_conn_timeout is embedded in struct hci_conn, but queuing the work
> does not hold a reference to the connection. hci_conn_del() uses
> cancel_delayed_work() because synchronous cancellation would deadlock
> when le_conn_timeout() itself calls hci_conn_del() while holding
> hdev->lock.
>
> This leaves the following interleaving possible:
>
>   CPU 0                               CPU 1
>   le_conn_timeout()
>                                       hci_conn_del()
>                                         cancel_delayed_work() = false
>                                         hci_conn_cleanup()
>                                           put_device()
>                                             kfree(conn)
>   hci_conn_failed(conn, ...)
>
> The callback then dereferences the released connection. KASAN reported:
>
>   BUG: KASAN: slab-use-after-free in hci_conn_failed+0x232/0x250
>   Read of size 8 at addr ffff8881180e8e20 by task kworker/u33:1/111
>   Workqueue: hci0 le_conn_timeout
>   Call Trace:
>     hci_conn_failed+0x232/0x250
>     le_conn_timeout+0x23e/0x2c0
>     process_one_work+0x61b/0xf50
>     worker_thread+0x45b/0xd10
>
>   Allocated by task 115:
>     __hci_conn_add+0x1758/0x1b90
>     hci_connect_le+0x523/0x780
>     l2cap_chan_connect+0xfca/0x1bd0
>     l2cap_sock_connect+0x310/0x530
>
>   Freed by task 110:
>     kfree+0x149/0x330
>     device_release+0xc8/0x240
>     kobject_put+0x14d/0x280
>     hci_conn_del+0x561/0xe70
>     hci_abort_conn_sync+0x3e3/0x800
>
> Take a connection device reference before queuing le_conn_timeout. Drop
> it when the callback finishes, when queuing fails, or when cancellation
> removes a pending instance. If cancellation races an executing callback,
> the callback retains the reference until its final access, avoiding the
> use-after-free without waiting under hdev->lock.
>
> Fixes: 980ffc0a2cec ("Bluetooth: Fix LE connection timeout deadlock")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chengfeng Ye <nicoyip.dev@xxxxxxxxx>
> ---
>  net/bluetooth/hci_conn.c  |  8 ++++++--
>  net/bluetooth/hci_event.c | 25 ++++++++++++++++---------
>  2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 1966cd153d97..501a2dd2543b 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -723,10 +723,13 @@ static void le_conn_timeout(struct work_struct *work)
>                 hci_dev_lock(hdev);
>                 hci_conn_failed(conn, HCI_ERROR_ADVERTISING_TIMEOUT);
>                 hci_dev_unlock(hdev);
> -               return;
> +               goto done;
>         }
>
>         hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM);
> +
> +done:
> +       hci_conn_put(conn);
>  }
>
>  struct iso_list_data {
> @@ -1267,7 +1270,8 @@ void hci_conn_del(struct hci_conn *conn)
>                         hdev->acl_cnt += conn->sent;
>                 break;
>         case LE_LINK:
> -               cancel_delayed_work(&conn->le_conn_timeout);
> +               if (cancel_delayed_work(&conn->le_conn_timeout))
> +                       hci_conn_put(conn);
>
>                 if (hdev->le_pkts) {
>                         if (!hci_conn_num(hdev, LE_LINK) ||
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 741d658e9630..e1e6b5092ce8 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -1566,10 +1566,13 @@ static u8 hci_cc_le_set_adv_enable(struct hci_dev *hdev, void *data,
>                 hci_dev_set_flag(hdev, HCI_LE_ADV);
>
>                 conn = hci_lookup_le_connect(hdev);
> -               if (conn)
> -                       queue_delayed_work(hdev->workqueue,
> -                                          &conn->le_conn_timeout,
> -                                          conn->conn_timeout);
> +               if (conn) {
> +                       hci_conn_get(conn);
> +                       if (!queue_delayed_work(hdev->workqueue,
> +                                               &conn->le_conn_timeout,
> +                                               conn->conn_timeout))
> +                               hci_conn_put(conn);
> +               }
>         } else {
>                 hci_dev_clear_flag(hdev, HCI_LE_ADV);
>         }
> @@ -1614,10 +1617,13 @@ static u8 hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, void *data,
>                         hci_dev_set_flag(hdev, HCI_LE_ADV_0);
>
>                 conn = hci_lookup_le_connect(hdev);
> -               if (conn)
> -                       queue_delayed_work(hdev->workqueue,
> -                                          &conn->le_conn_timeout,
> -                                          conn->conn_timeout);
> +               if (conn) {
> +                       hci_conn_get(conn);
> +                       if (!queue_delayed_work(hdev->workqueue,
> +                                               &conn->le_conn_timeout,
> +                                               conn->conn_timeout))
> +                               hci_conn_put(conn);
> +               }
>         } else {
>                 if (cp->num_of_sets) {
>                         if (adv)
> @@ -5771,7 +5777,8 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
>                         }
>                 }
>         } else {
> -               cancel_delayed_work(&conn->le_conn_timeout);
> +               if (cancel_delayed_work(&conn->le_conn_timeout))
> +                       hci_conn_put(conn);
>         }
>
>         /* The HCI_LE_Connection_Complete event is only sent once per connection.
> --
> 2.43.0

We might be better off removing the le_conn_timeout completely and
just make hci_le_create_conn_sync -> hci_le_directed_advertising_sync
wait on the connection complete directly rather then using yet another
work that can then race against the likes of hci_conn_del.

-- 
Luiz Augusto von Dentz