Lee, Chun-Yi 2e32a7
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Lee, Chun-Yi 2e32a7
Date: Fri, 22 Apr 2022 12:58:16 -0700
Lee, Chun-Yi 2e32a7
Subject: Bluetooth: hci_event: Fix checking for invalid handle on error status
Lee, Chun-Yi 2e32a7
Patch-mainline: v5.18-rc5
Lee, Chun-Yi 2e32a7
Git-commit: c86cc5a3ec70f5644f1fa21610b943d0441bc1f7
Lee, Chun-Yi 2e32a7
References: jsc#PED-1407
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
Commit d5ebaa7c5f6f6 introduces checks for handle range
Lee, Chun-Yi 2e32a7
(e.g HCI_CONN_HANDLE_MAX) but controllers like Intel AX200 don't seem
Lee, Chun-Yi 2e32a7
to respect the valid range int case of error status:
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
> HCI Event: Connect Complete (0x03) plen 11
Lee, Chun-Yi 2e32a7
        Status: Page Timeout (0x04)
Lee, Chun-Yi 2e32a7
        Handle: 65535
Lee, Chun-Yi 2e32a7
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
Lee, Chun-Yi 2e32a7
	Sound Products Inc)
Lee, Chun-Yi 2e32a7
        Link type: ACL (0x01)
Lee, Chun-Yi 2e32a7
        Encryption: Disabled (0x00)
Lee, Chun-Yi 2e32a7
[1644965.827560] Bluetooth: hci0: Ignoring HCI_Connection_Complete for invalid handle
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
Because of it is impossible to cleanup the connections properly since
Lee, Chun-Yi 2e32a7
the stack would attempt to cancel the connection which is no longer in
Lee, Chun-Yi 2e32a7
progress causing the following trace:
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
< HCI Command: Create Connection Cancel (0x01|0x0008) plen 6
Lee, Chun-Yi 2e32a7
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
Lee, Chun-Yi 2e32a7
	Sound Products Inc)
Lee, Chun-Yi 2e32a7
= bluetoothd: src/profile.c:record_cb() Unable to get Hands-Free Voice
Lee, Chun-Yi 2e32a7
	gateway SDP record: Connection timed out
Lee, Chun-Yi 2e32a7
> HCI Event: Command Complete (0x0e) plen 10
Lee, Chun-Yi 2e32a7
      Create Connection Cancel (0x01|0x0008) ncmd 1
Lee, Chun-Yi 2e32a7
        Status: Unknown Connection Identifier (0x02)
Lee, Chun-Yi 2e32a7
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
Lee, Chun-Yi 2e32a7
	Sound Products Inc)
Lee, Chun-Yi 2e32a7
< HCI Command: Create Connection Cancel (0x01|0x0008) plen 6
Lee, Chun-Yi 2e32a7
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
Lee, Chun-Yi 2e32a7
	Sound Products Inc)
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
Fixes: d5ebaa7c5f6f6 ("Bluetooth: hci_event: Ignore multiple conn complete events")
Lee, Chun-Yi 2e32a7
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Lee, Chun-Yi 2e32a7
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
Lee, Chun-Yi 2e32a7
Acked-by: Lee, Chun-Yi <jlee@suse.com>
Lee, Chun-Yi 2e32a7
---
Lee, Chun-Yi 2e32a7
 include/net/bluetooth/hci.h |    1 
Lee, Chun-Yi 2e32a7
 net/bluetooth/hci_event.c   |   65 ++++++++++++++++++++++++--------------------
Lee, Chun-Yi 2e32a7
 2 files changed, 37 insertions(+), 29 deletions(-)
Lee, Chun-Yi 2e32a7
Lee, Chun-Yi 2e32a7
--- a/include/net/bluetooth/hci.h
Lee, Chun-Yi 2e32a7
+++ b/include/net/bluetooth/hci.h
Lee, Chun-Yi 2e32a7
@@ -578,6 +578,7 @@ enum {
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_CONNECTION_TIMEOUT	0x08
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_REJ_LIMITED_RESOURCES	0x0d
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_REJ_BAD_ADDR		0x0f
Lee, Chun-Yi 2e32a7
+#define HCI_ERROR_INVALID_PARAMETERS	0x12
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_REMOTE_USER_TERM	0x13
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_REMOTE_LOW_RESOURCES	0x14
Lee, Chun-Yi 2e32a7
 #define HCI_ERROR_REMOTE_POWER_OFF	0x15
Lee, Chun-Yi 2e32a7
--- a/net/bluetooth/hci_event.c
Lee, Chun-Yi 2e32a7
+++ b/net/bluetooth/hci_event.c
Lee, Chun-Yi 2e32a7
@@ -3067,13 +3067,9 @@ static void hci_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 {
Lee, Chun-Yi 2e32a7
 	struct hci_ev_conn_complete *ev = data;
Lee, Chun-Yi 2e32a7
 	struct hci_conn *conn;
Lee, Chun-Yi 2e32a7
+	u8 status = ev->status;
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
-		bt_dev_err(hdev, "Ignoring HCI_Connection_Complete for invalid handle");
Lee, Chun-Yi 2e32a7
-		return;
Lee, Chun-Yi 2e32a7
-	}
Lee, Chun-Yi 2e32a7
-
Lee, Chun-Yi 2e32a7
-	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
Lee, Chun-Yi 2e32a7
+	bt_dev_dbg(hdev, "status 0x%2.2x", status);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	hci_dev_lock(hdev);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
@@ -3122,8 +3118,14 @@ static void hci_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 		goto unlock;
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	if (!ev->status) {
Lee, Chun-Yi 2e32a7
+	if (!status) {
Lee, Chun-Yi 2e32a7
 		conn->handle = __le16_to_cpu(ev->handle);
Lee, Chun-Yi 2e32a7
+		if (conn->handle > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
+			bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
Lee, Chun-Yi 2e32a7
+				   conn->handle, HCI_CONN_HANDLE_MAX);
Lee, Chun-Yi 2e32a7
+			status = HCI_ERROR_INVALID_PARAMETERS;
Lee, Chun-Yi 2e32a7
+			goto done;
Lee, Chun-Yi 2e32a7
+		}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 		if (conn->type == ACL_LINK) {
Lee, Chun-Yi 2e32a7
 			conn->state = BT_CONFIG;
Lee, Chun-Yi 2e32a7
@@ -3164,18 +3166,18 @@ static void hci_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 			hci_send_cmd(hdev, HCI_OP_CHANGE_CONN_PTYPE, sizeof(cp),
Lee, Chun-Yi 2e32a7
 				     &cp;;
Lee, Chun-Yi 2e32a7
 		}
Lee, Chun-Yi 2e32a7
-	} else {
Lee, Chun-Yi 2e32a7
-		conn->state = BT_CLOSED;
Lee, Chun-Yi 2e32a7
-		if (conn->type == ACL_LINK)
Lee, Chun-Yi 2e32a7
-			mgmt_connect_failed(hdev, &conn->dst, conn->type,
Lee, Chun-Yi 2e32a7
-					    conn->dst_type, ev->status);
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	if (conn->type == ACL_LINK)
Lee, Chun-Yi 2e32a7
 		hci_sco_setup(conn, ev->status);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	if (ev->status) {
Lee, Chun-Yi 2e32a7
-		hci_connect_cfm(conn, ev->status);
Lee, Chun-Yi 2e32a7
+done:
Lee, Chun-Yi 2e32a7
+	if (status) {
Lee, Chun-Yi 2e32a7
+		conn->state = BT_CLOSED;
Lee, Chun-Yi 2e32a7
+		if (conn->type == ACL_LINK)
Lee, Chun-Yi 2e32a7
+			mgmt_connect_failed(hdev, &conn->dst, conn->type,
Lee, Chun-Yi 2e32a7
+					    conn->dst_type, status);
Lee, Chun-Yi 2e32a7
+		hci_connect_cfm(conn, status);
Lee, Chun-Yi 2e32a7
 		hci_conn_del(conn);
Lee, Chun-Yi 2e32a7
 	} else if (ev->link_type == SCO_LINK) {
Lee, Chun-Yi 2e32a7
 		switch (conn->setting & SCO_AIRMODE_MASK) {
Lee, Chun-Yi 2e32a7
@@ -3185,7 +3187,7 @@ static void hci_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 			break;
Lee, Chun-Yi 2e32a7
 		}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-		hci_connect_cfm(conn, ev->status);
Lee, Chun-Yi 2e32a7
+		hci_connect_cfm(conn, status);
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 unlock:
Lee, Chun-Yi 2e32a7
@@ -4676,6 +4678,7 @@ static void hci_sync_conn_complete_evt(s
Lee, Chun-Yi 2e32a7
 {
Lee, Chun-Yi 2e32a7
 	struct hci_ev_sync_conn_complete *ev = data;
Lee, Chun-Yi 2e32a7
 	struct hci_conn *conn;
Lee, Chun-Yi 2e32a7
+	u8 status = ev->status;
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	switch (ev->link_type) {
Lee, Chun-Yi 2e32a7
 	case SCO_LINK:
Lee, Chun-Yi 2e32a7
@@ -4690,12 +4693,7 @@ static void hci_sync_conn_complete_evt(s
Lee, Chun-Yi 2e32a7
 		return;
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
-		bt_dev_err(hdev, "Ignoring HCI_Sync_Conn_Complete for invalid handle");
Lee, Chun-Yi 2e32a7
-		return;
Lee, Chun-Yi 2e32a7
-	}
Lee, Chun-Yi 2e32a7
-
Lee, Chun-Yi 2e32a7
-	bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
Lee, Chun-Yi 2e32a7
+	bt_dev_dbg(hdev, "status 0x%2.2x", status);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	hci_dev_lock(hdev);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
@@ -4729,9 +4727,17 @@ static void hci_sync_conn_complete_evt(s
Lee, Chun-Yi 2e32a7
 		goto unlock;
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	switch (ev->status) {
Lee, Chun-Yi 2e32a7
+	switch (status) {
Lee, Chun-Yi 2e32a7
 	case 0x00:
Lee, Chun-Yi 2e32a7
 		conn->handle = __le16_to_cpu(ev->handle);
Lee, Chun-Yi 2e32a7
+		if (conn->handle > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
+			bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
Lee, Chun-Yi 2e32a7
+				   conn->handle, HCI_CONN_HANDLE_MAX);
Lee, Chun-Yi 2e32a7
+			status = HCI_ERROR_INVALID_PARAMETERS;
Lee, Chun-Yi 2e32a7
+			conn->state = BT_CLOSED;
Lee, Chun-Yi 2e32a7
+			break;
Lee, Chun-Yi 2e32a7
+		}
Lee, Chun-Yi 2e32a7
+
Lee, Chun-Yi 2e32a7
 		conn->state  = BT_CONNECTED;
Lee, Chun-Yi 2e32a7
 		conn->type   = ev->link_type;
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
@@ -4775,8 +4781,8 @@ static void hci_sync_conn_complete_evt(s
Lee, Chun-Yi 2e32a7
 		}
Lee, Chun-Yi 2e32a7
 	}
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	hci_connect_cfm(conn, ev->status);
Lee, Chun-Yi 2e32a7
-	if (ev->status)
Lee, Chun-Yi 2e32a7
+	hci_connect_cfm(conn, status);
Lee, Chun-Yi 2e32a7
+	if (status)
Lee, Chun-Yi 2e32a7
 		hci_conn_del(conn);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 unlock:
Lee, Chun-Yi 2e32a7
@@ -5527,11 +5533,6 @@ static void le_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 	struct smp_irk *irk;
Lee, Chun-Yi 2e32a7
 	u8 addr_type;
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
-	if (handle > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
-		bt_dev_err(hdev, "Ignoring HCI_LE_Connection_Complete for invalid handle");
Lee, Chun-Yi 2e32a7
-		return;
Lee, Chun-Yi 2e32a7
-	}
Lee, Chun-Yi 2e32a7
-
Lee, Chun-Yi 2e32a7
 	hci_dev_lock(hdev);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	/* All controllers implicitly stop advertising in the event of a
Lee, Chun-Yi 2e32a7
@@ -5603,6 +5604,12 @@ static void le_conn_complete_evt(struct
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
 	conn->dst_type = ev_bdaddr_type(hdev, conn->dst_type, NULL);
Lee, Chun-Yi 2e32a7
 
Lee, Chun-Yi 2e32a7
+	if (handle > HCI_CONN_HANDLE_MAX) {
Lee, Chun-Yi 2e32a7
+		bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x", handle,
Lee, Chun-Yi 2e32a7
+			   HCI_CONN_HANDLE_MAX);
Lee, Chun-Yi 2e32a7
+		status = HCI_ERROR_INVALID_PARAMETERS;
Lee, Chun-Yi 2e32a7
+	}
Lee, Chun-Yi 2e32a7
+
Lee, Chun-Yi 2e32a7
 	if (status) {
Lee, Chun-Yi 2e32a7
 		hci_le_conn_failed(conn, status);
Lee, Chun-Yi 2e32a7
 		goto unlock;