Bluetooth: hci_sync: wait for directed advertising completion

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

Remove le_conn_timeout instead of adding another connection reference. Have the directed-advertising enable commands complete normally, then wait for the appropriate LE Connection Complete event with HCI_OP_NOP, matching other Command Complete then later-event sequences such as PAST. The command-sync entry already holds a connection reference until its completion callback returns.

LE Set Advertising Enable and LE Set Extended Advertising Enable return Command Complete rather than Command Status, so the later event cannot be attached to the enable command itself without changing generic command completion. Waiting with HCI_OP_NOP keeps that completion path unchanged.

Mark directed advertising as an in-flight connection attempt so teardown can cancel the wait. Disable advertising synchronously when that wait fails, and preserve HCI_ERROR_ADVERTISING_TIMEOUT for a software timeout. Clear the instance-0 extended advertising state when advertising is stopped or a connection completes so resuming paused advertising does not restart the directed advertising instance.

There is then no delayed callback that can race with connection deletion.

---
Changes in v5:
- Drop the generic hci_cmd_complete_evt() change from v3/v4. Keeping a request pending on successful Command Complete when a later event was requested broke BlueZ CI PAST, Read Exp Feature, and Mesh Send cancel.
- Wait for directed advertising completion with HCI_OP_NOP after the enable command completes, matching PAST, instead of changing generic command-complete semantics.

Changes in v4:
- Rebase onto bluetooth-next/master so the patch applies to current Bluetooth CI HEAD.

Changes in v3:
- Keep command-sync requests pending on successful Command Complete when the caller requested a later event, matching the existing Command Status behavior and fixing the first Sashiko/Luiz report.
- Clear HCI_LE_ADV_0 when all extended advertising instances are disabled and when LE connection complete implicitly stops advertising, fixing the second Sashiko report about resuming stale directed advertising.

Changes in v2:
- Remove le_conn_timeout instead of adding references around delayed work.
- Wait for LE Connection Complete from both directed-advertising enable paths.
- Make the wait cancellable and disable advertising after a failed wait.

 include/net/bluetooth/hci_core.h |  1 -
 net/bluetooth/hci_conn.c         | 45 --------------------------------
 net/bluetooth/hci_event.c        | 33 +++++------------------
 net/bluetooth/hci_sync.c         | 38 ++++++++++++++++++++++-----
 4 files changed, 38 insertions(+), 79 deletions(-)

index c12cd6873f65..d7a2df82ff38 100644
@@ -769,7 +769,6 @@ struct hci_conn {
 	struct delayed_work disc_work;
 	struct delayed_work auto_accept_work;
 	struct delayed_work idle_work;
-	struct delayed_work le_conn_timeout;
 
 	struct device	dev;
 	struct dentry	*debugfs;

index 2f5e21ff9752..b0af5635f831 100644
@@ -1595,22 +1595,10 @@ static u8 hci_cc_le_set_adv_enable(struct hci_dev *hdev, void *data,
 
 	hci_dev_lock(hdev);
 
-	/* If we're doing connection initiation as peripheral. Set a
-	 * timeout in case something goes wrong.
-	 */
-	if (*sent) {
-		struct hci_conn *conn;
-
+	if (*sent)
 		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);
-	} else {
+	else
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);
-	}
 
 	hci_dev_unlock(hdev);
 

@@ -1642,20 +1630,12 @@ static u8 hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, void *data,
 		adv = hci_find_adv_instance(hdev, set->handle);
 
 	if (cp->enable) {
-		struct hci_conn *conn;
-
 		hci_dev_set_flag(hdev, HCI_LE_ADV);
 
 		if (adv)
 			adv->enabled = true;
 		else if (!set->handle)
 			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);
 	} else {
 		if (cp->num_of_sets) {
 			if (adv)

@@ -1676,6 +1656,7 @@ static u8 hci_cc_le_set_ext_adv_enable(struct hci_dev *hdev, void *data,
 			list_for_each_entry_safe(adv, n, &hdev->adv_instances,
 						 list)
 				adv->enabled = false;
+			hci_dev_clear_flag(hdev, HCI_LE_ADV_0);
 		}
 
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);

@@ -5764,10 +5745,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
 	hci_store_wake_reason(hdev, bdaddr, bdaddr_type);
 
 	/* Advertising stops when a connection is created. On a failed
-	 * connection it keeps running, so leave the state bit alone.
+	 * connection it keeps running, so leave the state bits alone.
 	 */
-	if (!status)
+	if (!status) {
 		hci_dev_clear_flag(hdev, HCI_LE_ADV);
+		hci_dev_clear_flag(hdev, HCI_LE_ADV_0);
+	}
 
 	/* Check for existing connection:
 	 *

@@ -5814,8 +5797,6 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
 							  &conn->init_addr_type);
 			}
 		}
-	} else {
-		cancel_delayed_work(&conn->le_conn_timeout);
 	}
 
 	/* The HCI_LE_Connection_Complete event is only sent once per connection.

index 7150037a864b..7e092e827ab8 100644
@@ -6639,6 +6639,19 @@ static bool conn_use_rpa(struct hci_conn *conn)
 	return hci_dev_test_flag(hdev, HCI_PRIVACY);
 }
 
+static int hci_le_wait_directed_adv_complete_sync(struct hci_dev *hdev,
+						  struct hci_conn *conn)
+{
+	/* LE Set (Extended) Advertising Enable returns a command complete
+	 * event, so it cannot wait for LE Connection Complete.
+	 */
+	return __hci_cmd_sync_status_sk(hdev, HCI_OP_NOP, 0, NULL,
+					use_enhanced_conn_complete(hdev) ?
+					HCI_EV_LE_ENHANCED_CONN_COMPLETE :
+					HCI_EV_LE_CONN_COMPLETE,
+					conn->conn_timeout, NULL);
+}
+
 static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
 						struct hci_conn *conn)
 {

@@ -6704,7 +6717,11 @@ static int hci_le_ext_directed_advertising_sync(struct hci_dev *hdev,
 			return err;
 	}
 
-	return hci_enable_ext_advertising_sync(hdev, 0x00);
+	err = hci_enable_ext_advertising_sync(hdev, 0x00);
+	if (err)
+		return err;
+
+	return hci_le_wait_directed_adv_complete_sync(hdev, conn);
 }
 
 static int hci_le_directed_advertising_sync(struct hci_dev *hdev,

@@ -6755,8 +6772,13 @@ static int hci_le_directed_advertising_sync(struct hci_dev *hdev,
 
 	enable = 0x01;
 
-	return __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_ENABLE,
-				     sizeof(enable), &enable, HCI_CMD_TIMEOUT);
+	status = __hci_cmd_sync_status(hdev, HCI_OP_LE_SET_ADV_ENABLE,
+				       sizeof(enable), &enable,
+				       HCI_CMD_TIMEOUT);
+	if (status)
+		return status;
+
+	return hci_le_wait_directed_adv_complete_sync(hdev, conn);
 }
 
 static void set_ext_conn_params(struct hci_conn *conn,

@@ -6860,6 +6882,7 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 		/* Pause advertising while doing directed advertising. */
 		hci_pause_advertising_sync(hdev);
 
+		set_bit(HCI_CONN_CREATE, &conn->flags);
 		err = hci_le_directed_advertising_sync(hdev, conn);
 		goto done;
 	}

@@ -6946,7 +6969,9 @@ static int hci_le_create_conn_sync(struct hci_dev *hdev, void *data)
 done:
 	clear_bit(HCI_CONN_CREATE, &conn->flags);
 
-	if (err == -ETIMEDOUT)
+	if (err && conn->role == HCI_ROLE_SLAVE)
+		hci_disable_advertising_sync(hdev);
+	else if (err == -ETIMEDOUT)
 		hci_le_connect_cancel_sync(hdev, conn, 0x00);
 
 	/* Re-enable advertising after the connection attempt is finished. */

@@ -7283,9 +7308,8 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
 	if (conn != hci_lookup_le_connect(hdev))
 		goto unlock;
 
-	/* Flush to make sure we send create conn cancel command if needed */
-	flush_delayed_work(&conn->le_conn_timeout);
-	hci_conn_failed(conn, bt_status(err));
+	hci_conn_failed(conn, conn->role == HCI_ROLE_SLAVE && err == -ETIMEDOUT ?
+			 HCI_ERROR_ADVERTISING_TIMEOUT : bt_status(err));
 
 unlock:
 	hci_dev_unlock(hdev);

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/project/bluetooth/list/?series=1150937

---Test result---

Test Summary:

CheckPatch                    PASS      1.14 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.20 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      24.41 seconds
CheckAllWarning               PASS      27.42 seconds
CheckSparse                   PASS      24.44 seconds
BuildKernel32                 PASS      28.23 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      427.64 seconds
TestRunner_l2cap-tester       PASS      62.80 seconds
TestRunner_iso-tester         FAIL      92.02 seconds
TestRunner_bnep-tester        PASS      17.87 seconds
TestRunner_mgmt-tester        FAIL      206.09 seconds
TestRunner_rfcomm-tester      PASS      23.10 seconds
TestRunner_sco-tester         PASS      27.28 seconds
TestRunner_ioctl-tester       PASS      23.03 seconds
TestRunner_mesh-tester        FAIL      23.68 seconds
TestRunner_smp-tester         PASS      21.01 seconds
TestRunner_userchan-tester    PASS      18.25 seconds
TestRunner_6lowpan-tester     PASS      21.11 seconds
IncrementalBuild              PASS      22.13 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis Output:
Clang not found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 141, Passed: 136 (96.5%), Failed: 5, Not Run: 0

Failed Test Cases

ISO Broadcaster PAST Info - Success                  Timed out    2.680 seconds
ISO Broadcaster PAST Info RPA - Success              Timed out    1.987 seconds
ISO Broadcaster PAST Sender - Success                Timed out    2.006 seconds
ISO Broadcaster PAST Receiver - Success              Timed out    2.811 seconds
ISO Broadcaster PAST Receiver Sync Lost - Success    Timed out    2.525 seconds

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

Failed Test Cases
Read Exp Feature - Success Failed 0.236 seconds
##############################
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.545 seconds
Mesh - Send cancel - 2                               Timed out    1.992 seconds

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

---
Regards,
Linux Bluetooth