Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Pauli Virtanen <pav@iki.fi>
To:     linux-bluetooth@vger.kernel.org
Cc:     Pauli Virtanen <pav@iki.fi>
Subject: [PATCH v2 1/4] Bluetooth: hci_sync: fix canceling LE scanning / CIS create conn state
Date:   Sun,  6 Aug 2023 23:18:37 +0300
Message-ID: <b2ed850478c905299c40154c1d6dfc304a93d77a.1691352503.git.pav@iki.fi>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

In hci_abort_conn_sync, some errors from hci_le_connect_cancel_sync
indicate the connection can be terminated immediately.  However,
hci_abort_conn_sync passes these errors through, making
hci_disconnect_all_sync stop processing in these cases.

Fix by using a separate flag to indicate if canceled connection can be
deleted directly.

Fixes: 0f2efc1d02d5 ("Bluetooth: hci_conn: Consolidate code for aborting connections")
Fixes: dd1f6778b17b ("Bluetooth: hci_sync: delete CIS in BT_OPEN/CONNECT/BOUND when aborting")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    v2: no change

 net/bluetooth/hci_sync.c | 48 ++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 41a8e57d8267..51ff682f66e0 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -5253,13 +5253,14 @@ static int hci_disconnect_sync(struct hci_dev *hdev, struct hci_conn *conn,
 }
 
 static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
-				      struct hci_conn *conn, u8 reason)
+				      struct hci_conn *conn, u8 reason,
+				      bool *del)
 {
-	/* Return reason if scanning since the connection shall probably be
-	 * cleanup directly.
-	 */
-	if (test_bit(HCI_CONN_SCANNING, &conn->flags))
-		return reason;
+	/* If scanning, connection can be just deleted */
+	if (test_bit(HCI_CONN_SCANNING, &conn->flags)) {
+		*del = true;
+		return 0;
+	}
 
 	if (conn->role == HCI_ROLE_SLAVE ||
 	    test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
@@ -5270,10 +5271,10 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
 }
 
 static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
-				   u8 reason)
+				   u8 reason, bool *del)
 {
 	if (conn->type == LE_LINK)
-		return hci_le_connect_cancel_sync(hdev, conn, reason);
+		return hci_le_connect_cancel_sync(hdev, conn, reason, del);
 
 	if (conn->type == ISO_LINK) {
 		/* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
@@ -5287,9 +5288,9 @@ static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn,
 		if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags))
 			return hci_disconnect_sync(hdev, conn, reason);
 
-		/* CIS with no Create CIS sent have nothing to cancel */
+		/* CIS with no Create CIS sent can be just deleted */
 		if (bacmp(&conn->dst, BDADDR_ANY))
-			return HCI_ERROR_LOCAL_HOST_TERM;
+			*del = true;
 
 		/* There is no way to cancel a BIS without terminating the BIG
 		 * which is done later on connection cleanup.
@@ -5370,6 +5371,7 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 {
 	int err = 0;
 	u16 handle = conn->handle;
+	bool del = false;
 
 	switch (conn->state) {
 	case BT_CONNECTED:
@@ -5377,17 +5379,15 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 		err = hci_disconnect_sync(hdev, conn, reason);
 		break;
 	case BT_CONNECT:
-		err = hci_connect_cancel_sync(hdev, conn, reason);
+		err = hci_connect_cancel_sync(hdev, conn, reason, &del);
 		break;
 	case BT_CONNECT2:
 		err = hci_reject_conn_sync(hdev, conn, reason);
 		break;
 	case BT_OPEN:
 	case BT_BOUND:
-		hci_dev_lock(hdev);
-		hci_conn_failed(conn, reason);
-		hci_dev_unlock(hdev);
-		return 0;
+		del = true;
+		break;
 	default:
 		conn->state = BT_CLOSED;
 		return 0;
@@ -5398,18 +5398,15 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason)
 	 * or in case of LE it was still scanning so it can be cleanup
 	 * safely.
 	 */
-	if (err) {
-		struct hci_conn *c;
+	if (err || del) {
+		hci_dev_lock(hdev);
 
 		/* Check if the connection hasn't been cleanup while waiting
 		 * commands to complete.
 		 */
-		c = hci_conn_hash_lookup_handle(hdev, handle);
-		if (!c || c != conn)
-			return 0;
+		if (hci_conn_hash_lookup_handle(hdev, handle) == conn)
+			hci_conn_failed(conn, del ? reason : err);
 
-		hci_dev_lock(hdev);
-		hci_conn_failed(conn, err);
 		hci_dev_unlock(hdev);
 	}
 
@@ -6311,8 +6308,11 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
 				       conn->conn_timeout, NULL);
 
 done:
-	if (err == -ETIMEDOUT)
-		hci_le_connect_cancel_sync(hdev, conn, 0x00);
+	if (err == -ETIMEDOUT) {
+		bool __maybe_unused del;
+
+		hci_le_connect_cancel_sync(hdev, conn, 0x00, &del);
+	}
 
 	/* Re-enable advertising after the connection attempt is finished. */
 	hci_resume_advertising_sync(hdev);
-- 
2.41.0