public inbox for linux-bluetooth@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: fix use-after-free in hci_conn_drop
@ 2026-02-09 10:02 Masahiro Kawada
  2026-02-09 11:16 ` bluez.test.bot
  2026-02-09 13:25 ` [PATCH] " Hillf Danton
  0 siblings, 2 replies; 6+ messages in thread
From: Masahiro Kawada @ 2026-02-09 10:02 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: luiz.dentz, marcel, johan.hedberg, linux-kernel, Masahiro Kawada,
	syzbot+3609b9b48e68e1fe47fd

Fix a use-after-free in hci_conn_drop triggered via hci_cmd_sync_work.

In hci_conn_del(), hci_cmd_sync_dequeue() is called after
hci_conn_cleanup() which may have already freed the conn pointer.
Fix by moving the dequeue before cleanup.

Additionally, le_read_features_complete() calls hci_conn_drop(conn)
without checking whether conn is still valid. When
hci_le_read_remote_features_sync() blocks waiting for an HCI event,
another thread can free conn through hci_conn_del(). Fix by adding
a hci_conn_valid() check before calling hci_conn_drop().

Fixes: 881559af5f5c ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
Fixes: a106e50be74b ("Bluetooth: HCI: Add support for LL Extended Feature Set")
Reported-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3609b9b48e68e1fe47fd
Tested-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
Signed-off-by: Masahiro Kawada <youjingxiaogao2@gmail.com>
---
 net/bluetooth/hci_conn.c | 6 +++---
 net/bluetooth/hci_sync.c | 3 +++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 0795818963a..aa3607327ad 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -1232,15 +1232,15 @@ void hci_conn_del(struct hci_conn *conn)
 	skb_queue_purge(&conn->data_q);
 	skb_queue_purge(&conn->tx_q.queue);
 
+	/* Dequeue callbacks using connection pointer as data */
+	hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
+
 	/* Remove the connection from the list and cleanup its remaining
 	 * state. This is a separate function since for some cases like
 	 * BT_CONNECT_SCAN we *only* want the cleanup part without the
 	 * rest of hci_conn_del.
 	 */
 	hci_conn_cleanup(conn);
-
-	/* Dequeue callbacks using connection pointer as data */
-	hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
 }
 
 struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index f04a90bce4a..f31086c187f 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7371,6 +7371,9 @@ static void le_read_features_complete(struct hci_dev *hdev, void *data, int err)
 	if (err == -ECANCELED)
 		return;
 
+	if (!hci_conn_valid(hdev, conn))
+		return;
+
 	hci_conn_drop(conn);
 }
 
-- 
2.43.0

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

* RE: Bluetooth: fix use-after-free in hci_conn_drop
  2026-02-09 10:02 [PATCH] Bluetooth: fix use-after-free in hci_conn_drop Masahiro Kawada
@ 2026-02-09 11:16 ` bluez.test.bot
  2026-02-09 13:25 ` [PATCH] " Hillf Danton
  1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2026-02-09 11:16 UTC (permalink / raw)
  To: linux-bluetooth, youjingxiaogao2

[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1052221

---Test result---

Test Summary:
CheckPatch                    PENDING   0.47 seconds
GitLint                       PENDING   0.33 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      25.77 seconds
CheckAllWarning               PASS      28.45 seconds
CheckSparse                   PASS      31.85 seconds
BuildKernel32                 PASS      24.99 seconds
TestRunnerSetup               PASS      560.46 seconds
TestRunner_l2cap-tester       PASS      28.11 seconds
TestRunner_iso-tester         PASS      91.87 seconds
TestRunner_bnep-tester        PASS      6.46 seconds
TestRunner_mgmt-tester        FAIL      117.55 seconds
TestRunner_rfcomm-tester      PASS      9.58 seconds
TestRunner_sco-tester         FAIL      14.80 seconds
TestRunner_ioctl-tester       PASS      10.10 seconds
TestRunner_mesh-tester        FAIL      12.52 seconds
TestRunner_smp-tester         PASS      8.69 seconds
TestRunner_userchan-tester    PASS      6.63 seconds
IncrementalBuild              PENDING   1.02 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.106 seconds
##############################
Test: TestRunner_sco-tester - FAIL
Desc: Run sco-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
BUG: sleeping function called from invalid context at net/core/sock.c:3782
Total: 30, Passed: 30 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.774 seconds
Mesh - Send cancel - 2                               Timed out    1.992 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth


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

* Re: [PATCH] Bluetooth: fix use-after-free in hci_conn_drop
  2026-02-09 10:02 [PATCH] Bluetooth: fix use-after-free in hci_conn_drop Masahiro Kawada
  2026-02-09 11:16 ` bluez.test.bot
@ 2026-02-09 13:25 ` Hillf Danton
  2026-02-10  6:08   ` kawada
  1 sibling, 1 reply; 6+ messages in thread
From: Hillf Danton @ 2026-02-09 13:25 UTC (permalink / raw)
  To: Masahiro Kawada
  Cc: linux-bluetooth, luiz.dentz, marcel, johan.hedberg, linux-kernel,
	syzbot+3609b9b48e68e1fe47fd

On Mon,  9 Feb 2026 19:02:11 +0900 Masahiro Kawada wrote:
> Fix a use-after-free in hci_conn_drop triggered via hci_cmd_sync_work.
> 
> In hci_conn_del(), hci_cmd_sync_dequeue() is called after
> hci_conn_cleanup() which may have already freed the conn pointer.
> Fix by moving the dequeue before cleanup.
> 
> Additionally, le_read_features_complete() calls hci_conn_drop(conn)
> without checking whether conn is still valid. When
> hci_le_read_remote_features_sync() blocks waiting for an HCI event,
> another thread can free conn through hci_conn_del(). Fix by adding
> a hci_conn_valid() check before calling hci_conn_drop().
> 
> Fixes: 881559af5f5c ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
> Fixes: a106e50be74b ("Bluetooth: HCI: Add support for LL Extended Feature Set")
> Reported-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=3609b9b48e68e1fe47fd
> Tested-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
> Signed-off-by: Masahiro Kawada <youjingxiaogao2@gmail.com>
> ---
>  net/bluetooth/hci_conn.c | 6 +++---
>  net/bluetooth/hci_sync.c | 3 +++
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 0795818963a..aa3607327ad 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -1232,15 +1232,15 @@ void hci_conn_del(struct hci_conn *conn)
>  	skb_queue_purge(&conn->data_q);
>  	skb_queue_purge(&conn->tx_q.queue);
>  
> +	/* Dequeue callbacks using connection pointer as data */
> +	hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
> +
>  	/* Remove the connection from the list and cleanup its remaining
>  	 * state. This is a separate function since for some cases like
>  	 * BT_CONNECT_SCAN we *only* want the cleanup part without the
>  	 * rest of hci_conn_del.
>  	 */
>  	hci_conn_cleanup(conn);
> -
> -	/* Dequeue callbacks using connection pointer as data */
> -	hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
>  }
>  
>  struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index f04a90bce4a..f31086c187f 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -7371,6 +7371,9 @@ static void le_read_features_complete(struct hci_dev *hdev, void *data, int err)
>  	if (err == -ECANCELED)
>  		return;
>  
> +	if (!hci_conn_valid(hdev, conn))
> +		return;
> +
>  	hci_conn_drop(conn);
>  }
>  
> -- 
> 2.43.0
>
The uaf [1] is due to the following race,

	cpu1				cpu2
	hci_cmd_sync_work()		hci_rx_work()
	mutex_lock(&hdev->cmd_sync_work_lock);
	entry = list_first_entry_or_null(&hdev->cmd_sync_work_list,
					 struct hci_cmd_sync_work_entry,
					 list);
	if (entry)
		list_del(&entry->list);
	mutex_unlock(&hdev->cmd_sync_work_lock);

					hci_conn_del()
					hci_conn_hash_del(hdev, conn);
					hci_conn_cleanup(conn)	// free conn
					hci_cmd_sync_dequeue()
					mutex_lock(&hdev->cmd_sync_work_lock);
					while ((entry = _hci_cmd_sync_lookup_entry(hdev, func, data,
						   destroy))) {
						_hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED);
						ret = true;
					}
					mutex_unlock(&hdev->cmd_sync_work_lock);

	hci_req_sync_lock(hdev);
	err = entry->func(hdev, entry->data);
	if (entry->destroy)
		entry->destroy(hdev, entry->data, err);
		hci_conn_drop(conn)	// uaf
	hci_req_sync_unlock(hdev);

but the race still exists after this patch.

	cpu1				cpu2
	hci_conn_valid(hdev, conn)
					hci_conn_hash_del(hdev, conn);
					hci_cmd_sync_dequeue()
					hci_conn_cleanup(conn)	// free conn
	hci_conn_drop(conn); // uaf

[1] Subject: [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in hci_conn_drop (3)
https://lore.kernel.org/lkml/69301edd.a70a0220.2ea503.00cf.GAE@google.com/

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

* Re: [PATCH] Bluetooth: fix use-after-free in hci_conn_drop
  2026-02-09 13:25 ` [PATCH] " Hillf Danton
@ 2026-02-10  6:08   ` kawada
  2026-02-10 10:08     ` Hillf Danton
  0 siblings, 1 reply; 6+ messages in thread
From: kawada @ 2026-02-10  6:08 UTC (permalink / raw)
  To: Hillf Danton
  Cc: linux-bluetooth, luiz.dentz, marcel, johan.hedberg, linux-kernel,
	syzbot+3609b9b48e68e1fe47fd

> but the race still exists after this patch.

I agree that the TOCTOU issue remains. I looked into all the functions
that can be called as entry->destroy in hci_cmd_sync_work where the
second argument (data) is used as hci_conn*. I found five such
functions across two files:

In hci_sync.c:
  - create_le_conn_complete
  - create_pa_complete
  - create_big_complete
  - le_read_features_complete

In hci_conn.c:
  - create_big_complete (separate static function)

Of these, create_le_conn_complete and create_pa_complete already use
hci_dev_lock, which effectively prevents this TOCTOU issue.

The remaining three are vulnerable:
  - create_big_complete in hci_sync.c calls hci_conn_valid without
    holding hci_dev_lock (TOCTOU)
  - le_read_features_complete calls hci_conn_drop with no validity
    check at all
  - create_big_complete in hci_conn.c calls hci_connect_cfm and
    hci_conn_del with no validity check at all

Given this, I believe the following set of patches would be
appropriate:

1. Reorder hci_cmd_sync_dequeue before hci_conn_cleanup in
   hci_conn_del
2. Wrap all three vulnerable callbacks with hci_dev_lock and
   hci_conn_valid, following the same pattern used by
   create_le_conn_complete and create_pa_complete

If this approach sounds reasonable, I will prepare updated patches.


2026年2月9日(月) 22:25 Hillf Danton <hdanton@sina.com>:
>
> On Mon,  9 Feb 2026 19:02:11 +0900 Masahiro Kawada wrote:
> > Fix a use-after-free in hci_conn_drop triggered via hci_cmd_sync_work.
> >
> > In hci_conn_del(), hci_cmd_sync_dequeue() is called after
> > hci_conn_cleanup() which may have already freed the conn pointer.
> > Fix by moving the dequeue before cleanup.
> >
> > Additionally, le_read_features_complete() calls hci_conn_drop(conn)
> > without checking whether conn is still valid. When
> > hci_le_read_remote_features_sync() blocks waiting for an HCI event,
> > another thread can free conn through hci_conn_del(). Fix by adding
> > a hci_conn_valid() check before calling hci_conn_drop().
> >
> > Fixes: 881559af5f5c ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
> > Fixes: a106e50be74b ("Bluetooth: HCI: Add support for LL Extended Feature Set")
> > Reported-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3609b9b48e68e1fe47fd
> > Tested-by: syzbot+3609b9b48e68e1fe47fd@syzkaller.appspotmail.com
> > Signed-off-by: Masahiro Kawada <youjingxiaogao2@gmail.com>
> > ---
> >  net/bluetooth/hci_conn.c | 6 +++---
> >  net/bluetooth/hci_sync.c | 3 +++
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 0795818963a..aa3607327ad 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -1232,15 +1232,15 @@ void hci_conn_del(struct hci_conn *conn)
> >       skb_queue_purge(&conn->data_q);
> >       skb_queue_purge(&conn->tx_q.queue);
> >
> > +     /* Dequeue callbacks using connection pointer as data */
> > +     hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
> > +
> >       /* Remove the connection from the list and cleanup its remaining
> >        * state. This is a separate function since for some cases like
> >        * BT_CONNECT_SCAN we *only* want the cleanup part without the
> >        * rest of hci_conn_del.
> >        */
> >       hci_conn_cleanup(conn);
> > -
> > -     /* Dequeue callbacks using connection pointer as data */
> > -     hci_cmd_sync_dequeue(hdev, NULL, conn, NULL);
> >  }
> >
> >  struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index f04a90bce4a..f31086c187f 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -7371,6 +7371,9 @@ static void le_read_features_complete(struct hci_dev *hdev, void *data, int err)
> >       if (err == -ECANCELED)
> >               return;
> >
> > +     if (!hci_conn_valid(hdev, conn))
> > +             return;
> > +
> >       hci_conn_drop(conn);
> >  }
> >
> > --
> > 2.43.0
> >
> The uaf [1] is due to the following race,
>
>         cpu1                            cpu2
>         hci_cmd_sync_work()             hci_rx_work()
>         mutex_lock(&hdev->cmd_sync_work_lock);
>         entry = list_first_entry_or_null(&hdev->cmd_sync_work_list,
>                                          struct hci_cmd_sync_work_entry,
>                                          list);
>         if (entry)
>                 list_del(&entry->list);
>         mutex_unlock(&hdev->cmd_sync_work_lock);
>
>                                         hci_conn_del()
>                                         hci_conn_hash_del(hdev, conn);
>                                         hci_conn_cleanup(conn)  // free conn
>                                         hci_cmd_sync_dequeue()
>                                         mutex_lock(&hdev->cmd_sync_work_lock);
>                                         while ((entry = _hci_cmd_sync_lookup_entry(hdev, func, data,
>                                                    destroy))) {
>                                                 _hci_cmd_sync_cancel_entry(hdev, entry, -ECANCELED);
>                                                 ret = true;
>                                         }
>                                         mutex_unlock(&hdev->cmd_sync_work_lock);
>
>         hci_req_sync_lock(hdev);
>         err = entry->func(hdev, entry->data);
>         if (entry->destroy)
>                 entry->destroy(hdev, entry->data, err);
>                 hci_conn_drop(conn)     // uaf
>         hci_req_sync_unlock(hdev);
>
> but the race still exists after this patch.
>
>         cpu1                            cpu2
>         hci_conn_valid(hdev, conn)
>                                         hci_conn_hash_del(hdev, conn);
>                                         hci_cmd_sync_dequeue()
>                                         hci_conn_cleanup(conn)  // free conn
>         hci_conn_drop(conn); // uaf
>
> [1] Subject: [syzbot] [bluetooth?] KASAN: slab-use-after-free Write in hci_conn_drop (3)
> https://lore.kernel.org/lkml/69301edd.a70a0220.2ea503.00cf.GAE@google.com/

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

* Re: [PATCH] Bluetooth: fix use-after-free in hci_conn_drop
  2026-02-10  6:08   ` kawada
@ 2026-02-10 10:08     ` Hillf Danton
  2026-02-10 14:50       ` kawada
  0 siblings, 1 reply; 6+ messages in thread
From: Hillf Danton @ 2026-02-10 10:08 UTC (permalink / raw)
  To: kawada
  Cc: linux-bluetooth, luiz.dentz, marcel, johan.hedberg, linux-kernel,
	syzbot+3609b9b48e68e1fe47fd

[ hm... top reply looks no good ]

On Tue, 10 Feb 2026 15:08:12 +0900 Masahiro Kawada wrote:
> > but the race still exists after this patch.
> 
> I agree that the TOCTOU issue remains. I looked into all the functions
> that can be called as entry->destroy in hci_cmd_sync_work where the
> second argument (data) is used as hci_conn*. I found five such
> functions across two files:
> 
> In hci_sync.c:
>   - create_le_conn_complete
>   - create_pa_complete
>   - create_big_complete
>   - le_read_features_complete
> 
> In hci_conn.c:
>   - create_big_complete (separate static function)
> 
> Of these, create_le_conn_complete and create_pa_complete already use
> hci_dev_lock, which effectively prevents this TOCTOU issue.
> 
> The remaining three are vulnerable:
>   - create_big_complete in hci_sync.c calls hci_conn_valid without
>     holding hci_dev_lock (TOCTOU)
>   - le_read_features_complete calls hci_conn_drop with no validity
>     check at all
>   - create_big_complete in hci_conn.c calls hci_connect_cfm and
>     hci_conn_del with no validity check at all
> 
> Given this, I believe the following set of patches would be
> appropriate:
> 
> 1. Reorder hci_cmd_sync_dequeue before hci_conn_cleanup in
>    hci_conn_del
> 2. Wrap all three vulnerable callbacks with hci_dev_lock and
>    hci_conn_valid, following the same pattern used by
>    create_le_conn_complete and create_pa_complete
> 
> If this approach sounds reasonable, I will prepare updated patches.
>
Better not before spotting the reason why conn->refcnt failed to make the
entry->destroy callback safe, given the complexity of the race.

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

* Re: [PATCH] Bluetooth: fix use-after-free in hci_conn_drop
  2026-02-10 10:08     ` Hillf Danton
@ 2026-02-10 14:50       ` kawada
  0 siblings, 0 replies; 6+ messages in thread
From: kawada @ 2026-02-10 14:50 UTC (permalink / raw)
  To: Hillf Danton
  Cc: linux-bluetooth, luiz.dentz, marcel, johan.hedberg, linux-kernel,
	syzbot+3609b9b48e68e1fe47fd

Apologies for the top-posting.

> Better not before spotting the reason why conn->refcnt
> failed to make the entry->destroy callback safe.

hci_conn_hold() increments conn->refcnt, but the actual
freeing of conn is done by put_device() called from
hci_conn_del(). hci_conn_del() does not check conn->refcnt
at all, so the hold/drop refcount cannot prevent the UAF
in entry->destroy callbacks.

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

end of thread, other threads:[~2026-02-10 14:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09 10:02 [PATCH] Bluetooth: fix use-after-free in hci_conn_drop Masahiro Kawada
2026-02-09 11:16 ` bluez.test.bot
2026-02-09 13:25 ` [PATCH] " Hillf Danton
2026-02-10  6:08   ` kawada
2026-02-10 10:08     ` Hillf Danton
2026-02-10 14:50       ` kawada

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