Bluetooth: Serialize TX scheduling with teardown

The TX scheduler drops its RCU read lock before using the selected channel or connection. Teardown on the separate request workqueue can then free that object while transmission is still using it.

This series fixes two distinct lifetime bugs using the existing device mutex:

1. Protect channel selection, transmission and priority recalculation in the ACL and LE schedulers against channel deletion.
2. Protect connection selection and transmission in the SCO and ISO schedulers against connection teardown.

Please apply the patches in order. Patch 2 depends on the ACL/LE locking introduced by patch 1: it uses a lock-held SCO helper for their nested SCO calls, while direct SCO calls from the TX worker use a locking wrapper. This preserves packet scheduling order without recursively taking the device mutex. Timeout checks remain outside the critical sections.

The transmit path can sleep, so extending ordinary RCU across transmission is not a suitable substitute for the mutex.

Validation: rebuilt hci_core.o after each patch and completed a full kernel build with CONFIG_BT, CONFIG_BT_BREDR and CONFIG_BT_LE enabled. Both patches pass strict checkpatch and apply in order to the stated base. Runtime PoC replay and runtime lockdep testing have not been performed for this series.

Chengfeng Ye (2):
Bluetooth: hci_core: Serialize ACL scheduling with channel deletion
Bluetooth: hci_core: Serialize SCO and ISO scheduling with teardown

 net/bluetooth/hci_core.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14

2.43.0

hci_chan_sent() selects a channel under RCU but drops the read lock before accessing chan->conn and returning the channel. The ACL and LE schedulers then use its packet queue and update its transmit counters without any protection against channel deletion.

After TX selects a channel and releases RCU, a disconnect command timeout on the separate request workqueue can run hci_conn_failed() and l2cap_conn_del(). hci_chan_del() can then unlink the channel, complete synchronize_rcu(), purge its queue and free it before TX resumes. This causes use-after-free both in hci_chan_sent() and in its callers.

KASAN reported:

  BUG: KASAN: slab-use-after-free in hci_chan_sent+0x892/0x9b0
  Workqueue: hci0 hci_tx_work
  Call Trace:
   hci_chan_sent+0x892/0x9b0
   hci_tx_work+0x5e6/0xb70
  Allocated by task 91:
   hci_chan_create+0xe3/0x350
   l2cap_conn_add.part.0+0x12/0xa30
   l2cap_chan_connect+0x110d/0x1b60
   l2cap_sock_connect+0x310/0x530
  Freed by task 99:
   hci_chan_del+0x11f/0x170
   l2cap_conn_del+0x4f1/0x800
   l2cap_connect_cfm+0x88c/0xd30
   hci_conn_failed+0x150/0x250
   hci_abort_conn_sync+0x3e3/0x800
   hci_cmd_sync_run+0x7e/0xc0
   hci_abort_conn+0x105/0x1f0
   disconnect_sync+0x157/0x290
   hci_cmd_sync_work+0x13c/0x290

Hold the existing device mutex across channel selection and transmission in both schedulers to serialize them with channel teardown. Keep timeout handling outside the critical sections because hci_link_tx_to() acquires the same mutex. This also permits the transmit path to sleep, unlike extending the RCU read-side critical section across packet submission.

---
 net/bluetooth/hci_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

index d183efaf9063..24b46ccd4da2 100644
@@ -3659,6 +3659,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
 
 	__check_timeout(hdev, cnt, ACL_LINK);
 
+	hci_dev_lock(hdev);
+
 	while (hdev->acl_cnt &&
 	       (chan = hci_chan_sent(hdev, ACL_LINK, &quote))) {
 		u32 priority = (skb_peek(&chan->data_q))->priority;

@@ -3690,6 +3692,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
 
 	if (cnt != hdev->acl_cnt)
 		hci_prio_recalculate(hdev, ACL_LINK);
+
+	hci_dev_unlock(hdev);
 }
 
 static void hci_sched_acl(struct hci_dev *hdev)

@@ -3718,6 +3722,8 @@ static void hci_sched_le(struct hci_dev *hdev)
 
 	__check_timeout(hdev, *cnt, LE_LINK);
 
+	hci_dev_lock(hdev);
+
 	tmp = *cnt;
 	while (*cnt && (chan = hci_chan_sent(hdev, LE_LINK, &quote))) {
 		u32 priority = (skb_peek(&chan->data_q))->priority;

@@ -3746,6 +3752,8 @@ static void hci_sched_le(struct hci_dev *hdev)
 
 	if (*cnt != tmp)
 		hci_prio_recalculate(hdev, LE_LINK);
+
+	hci_dev_unlock(hdev);
 }
 
 /* Schedule iso */

2.43.0

hci_low_sent() selects a connection under RCU but drops the read lock before calculating its quota and returning it to the scheduler. The connection is then used without lifetime protection by hci_sched_iso() and hci_sched_sco().

After the TX worker drops the RCU read lock, hci_abort_conn_sync() on hdev->req_workqueue can remove the connection from the hash, complete synchronize_rcu(), purge its queues and release it. The TX worker on hdev->workqueue can then access the freed connection in hci_quote_sent() or while dequeuing packets and updating conn->sent.

KASAN reported:

  BUG: KASAN: slab-use-after-free in hci_low_sent+0x730/0x840
  Workqueue: hci0 hci_tx_work
  Call Trace:
   hci_low_sent+0x730/0x840
   hci_sched_iso+0x25e/0x4d0
   hci_tx_work+0x239/0xcb0

  Allocated by task 93:
   __hci_conn_add+0x16f/0x1b40
   hci_bind_bis+0x782/0x17b0
   hci_connect_bis+0xa0/0x510
   iso_sock_connect+0x589/0x1050

  Freed by task 88:
   kfree+0x131/0x3c0
   device_release+0xc8/0x240
   kobject_put+0x14d/0x280
   hci_conn_del+0x55a/0xe80
   hci_disconnect_sync+0x156/0x180
   hci_abort_conn_sync+0x3e7/0x940
   hci_cmd_sync_work+0x13c/0x290

Hold hci_dev_lock() across connection selection and transmission in both SCO and ISO scheduling, serializing them with connection teardown. This also prevents queuing completion timestamps after the connection queues have been purged. Extending RCU across transmission would be unsafe because the transmit path can sleep. Keep the ISO timeout check outside the mutex since hci_link_tx_to() takes it itself.

The preceding channel fix already holds this mutex in the ACL and LE schedulers, which call the SCO scheduler between packets. Move the SCO body to __hci_sched_sco(), assert that its caller holds the mutex, and use it directly from these locked paths. Keep a locking hci_sched_sco() wrapper for the direct calls from hci_tx_work(). This avoids recursively acquiring the device mutex while preserving the scheduling order.

Remove the obsolete claim that connection removal disables TX.

---
 net/bluetooth/hci_core.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

index 24b46ccd4da2..985c58dc6da8 100644
@@ -3402,9 +3402,6 @@ static struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type,
 	struct hci_conn *conn = NULL, *c;
 	unsigned int num = 0, min = ~0;
 
-	/* We don't have to lock device here. Connections are always
-	 * added and removed with TX task disabled. */
-
 	rcu_read_lock();
 
 	list_for_each_entry_rcu(c, &h->list, list) {

@@ -3609,13 +3606,15 @@ static void __check_timeout(struct hci_dev *hdev, unsigned int cnt, u8 type)
 }
 
 /* Schedule SCO */
-static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
+static void __hci_sched_sco(struct hci_dev *hdev, __u8 type)
 {
 	struct hci_conn *conn;
 	struct sk_buff *skb;
 	int quote, *cnt;
 	unsigned int pkts = hdev->sco_pkts;
 
+	lockdep_assert_held(&hdev->lock);
+
 	bt_dev_dbg(hdev, "type %u", type);
 
 	if (!hci_conn_num(hdev, type) || !pkts)

@@ -3650,6 +3649,13 @@ static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
 		queue_work(hdev->workqueue, &hdev->tx_work);
 }
 
+static void hci_sched_sco(struct hci_dev *hdev, __u8 type)
+{
+	hci_dev_lock(hdev);
+	__hci_sched_sco(hdev, type);
+	hci_dev_unlock(hdev);
+}
+
 static void hci_sched_acl_pkt(struct hci_dev *hdev)
 {
 	unsigned int cnt = hdev->acl_cnt;

@@ -3685,8 +3691,8 @@ static void hci_sched_acl_pkt(struct hci_dev *hdev)
 			chan->conn->sent++;
 
 			/* Send pending SCO packets right away */
-			hci_sched_sco(hdev, SCO_LINK);
-			hci_sched_sco(hdev, ESCO_LINK);
+			__hci_sched_sco(hdev, SCO_LINK);
+			__hci_sched_sco(hdev, ESCO_LINK);
 		}
 	}
 

@@ -3745,8 +3751,8 @@ static void hci_sched_le(struct hci_dev *hdev)
 			chan->conn->sent++;
 
 			/* Send pending SCO packets right away */
-			hci_sched_sco(hdev, SCO_LINK);
-			hci_sched_sco(hdev, ESCO_LINK);
+			__hci_sched_sco(hdev, SCO_LINK);
+			__hci_sched_sco(hdev, ESCO_LINK);
 		}
 	}
 

@@ -3772,6 +3778,8 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type)
 
 	__check_timeout(hdev, *cnt, type);
 
+	hci_dev_lock(hdev);
+
 	while (*cnt && (conn = hci_low_sent(hdev, type, &quote))) {
 		while (quote-- && (skb = skb_dequeue(&conn->data_q))) {
 			BT_DBG("skb %p len %d", skb, skb->len);

@@ -3785,6 +3793,8 @@ static void hci_sched_iso(struct hci_dev *hdev, __u8 type)
 			(*cnt)--;
 		}
 	}
+
+	hci_dev_unlock(hdev);
 }
 
 static void hci_tx_work(struct work_struct *work)

2.43.0

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/series/1174569/

---Test result---

Test Summary:

CheckPatch                    PASS      1.17 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       PASS      0.47 seconds
SubjectPrefix                 PASS      0.89 seconds
BuildKernel                   PASS      29.82 seconds
CheckAllWarning               PASS      34.19 seconds
CheckSparse                   PASS      36.80 seconds
BuildKernel32                 PASS      29.72 seconds
CheckKernelLLVM               PASS      33.97 seconds
TestRunnerSetup               PASS      793.36 seconds
TestRunner_l2cap-tester       PASS      16.58 seconds
TestRunner_iso-tester         PASS      26.90 seconds
TestRunner_bnep-tester        PASS      3.07 seconds
TestRunner_mgmt-tester        PASS      61.18 seconds
TestRunner_rfcomm-tester      PASS      4.40 seconds
TestRunner_sco-tester         PASS      7.56 seconds
TestRunner_ioctl-tester       PASS      4.55 seconds
TestRunner_mesh-tester        FAIL      7.76 seconds
TestRunner_smp-tester         PASS      4.27 seconds
TestRunner_userchan-tester    PASS      3.12 seconds
TestRunner_6lowpan-tester     PASS      4.37 seconds
IncrementalBuild              PASS      37.19 seconds

Details
##############################
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.419 seconds
Mesh - Send cancel - 2                               Timed out    1.999 seconds

https://github.com/bluez/bluetooth-next/pull/823

---
Regards,
Linux Bluetooth