Re: [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work
Luiz Augusto von Dentz <[email protected]> Fri, 31 Jul 2026 15:25:29 -0400
| Newsgroups | org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CABBYNZJx46usjp0_snfRQ7pvWrxPE8NZqSyqyM0rMEsoHgZnow@mail.gmail.com> |
Hi Chengfeng, On Thu, Jul 30, 2026 at 6:41=E2=80=AFAM Chengfeng Ye <[email protected]= > 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() =3D 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: [email protected] > Signed-off-by: Chengfeng Ye <[email protected]> > --- > 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 *wor= k) > 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 +=3D 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 =3D 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 =3D 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 *hd= ev, 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 con= nection. > -- > 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. --=20 Luiz Augusto von Dentz