[1/3] Bluetooth: hci_sync: fix canceling LE scanning / CIS create conn state

Commit Message

Pauli Virtanen Aug. 6, 2023, 3:27 p.m. UTC

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 <[e-mail]>
---
 net/bluetooth/hci_sync.c | 48 ++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

Comments

[e-mail] Aug. 6, 2023, 4 p.m. UTC | #1

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=773436

---Test result---

Test Summary:
CheckPatch                    FAIL      2.27 seconds
GitLint                       FAIL      1.19 seconds
SubjectPrefix                 PASS      0.35 seconds
BuildKernel                   PASS      32.20 seconds
CheckAllWarning               PASS      35.20 seconds
CheckSparse                   PASS      40.12 seconds
CheckSmatch                   PASS      111.99 seconds
BuildKernel32                 PASS      31.53 seconds
TestRunnerSetup               PASS      472.33 seconds
TestRunner_l2cap-tester       PASS      22.17 seconds
TestRunner_iso-tester         PASS      43.64 seconds
TestRunner_bnep-tester        PASS      10.03 seconds
TestRunner_mgmt-tester        PASS      211.21 seconds
TestRunner_rfcomm-tester      PASS      15.03 seconds
TestRunner_sco-tester         PASS      18.23 seconds
TestRunner_ioctl-tester       PASS      17.04 seconds
TestRunner_mesh-tester        PASS      12.45 seconds
TestRunner_smp-tester         PASS      13.56 seconds
TestRunner_userchan-tester    PASS      10.49 seconds
IncrementalBuild              PASS      40.72 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[2/3] Bluetooth: hci_sync: fix use-after-free in hci_disconnect_all_sync
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#100: 
CPU: 0 PID: 124 Comm: kworker/u9:0 Tainted: G        W          6.5.0-rc1+ #10

total: 0 errors, 1 warnings, 0 checks, 50 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13342873.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[2/3] Bluetooth: hci_sync: fix use-after-free in hci_disconnect_all_sync

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
13: B1 Line exceeds max length (99>80): "BUG: KASAN: slab-use-after-free in hci_set_powered_sync (net/bluetooth/hci_sync.c:5424) [bluetooth]"
17: B1 Line exceeds max length (81>80): "Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-1.fc38 04/01/2014"


---
Regards,
Linux Bluetooth

Patch

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);