The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work
@ 2026-07-30 10:41 Chengfeng Ye
  2026-07-31 19:25 ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 4+ messages in thread
From: Chengfeng Ye @ 2026-07-30 10:41 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, Johan Hedberg
  Cc: linux-bluetooth, linux-kernel, Chengfeng Ye, stable

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@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work
  2026-07-30 10:41 [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work Chengfeng Ye
@ 2026-07-31 19:25 ` Luiz Augusto von Dentz
  2026-08-01 14:54   ` Chengfeng Ye
  0 siblings, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-07-31 19:25 UTC (permalink / raw)
  To: Chengfeng Ye
  Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel,
	stable

Hi Chengfeng,

On Thu, Jul 30, 2026 at 6:41 AM Chengfeng Ye <nicoyip.dev@gmail.com> 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@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
>  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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work
  2026-07-31 19:25 ` Luiz Augusto von Dentz
@ 2026-08-01 14:54   ` Chengfeng Ye
  2026-08-01 14:57     ` Chengfeng Ye
  0 siblings, 1 reply; 4+ messages in thread
From: Chengfeng Ye @ 2026-08-01 14:54 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel,
	stable

Hi Luiz,

On Sat, Aug 1, 2026 at 3:25 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> 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.

Thanks for the review, I have just sent a v2 patch to remove the
racing timer completely and let the hci_le_directed_advertising_sync
synchronously wait for the connection. Let me know if anything should
be further adjusted.

Best regards,
Chengfeng

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work
  2026-08-01 14:54   ` Chengfeng Ye
@ 2026-08-01 14:57     ` Chengfeng Ye
  0 siblings, 0 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-08-01 14:57 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel,
	stable

The v2 patch is [PATCH v2] Bluetooth: hci_sync: wait for directed
advertising completion.

Best regards,
Chengfeng

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-01 14:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 10:41 [PATCH] Bluetooth: hci_conn: hold conn for LE timeout work Chengfeng Ye
2026-07-31 19:25 ` Luiz Augusto von Dentz
2026-08-01 14:54   ` Chengfeng Ye
2026-08-01 14:57     ` Chengfeng Ye

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox