Commit c86cc5a3 authored by Luiz Augusto von Dentz's avatar Luiz Augusto von Dentz Committed by Marcel Holtmann

Bluetooth: hci_event: Fix checking for invalid handle on error status

Commit d5ebaa7c introduces checks for handle range
(e.g HCI_CONN_HANDLE_MAX) but controllers like Intel AX200 don't seem
to respect the valid range int case of error status:

> HCI Event: Connect Complete (0x03) plen 11
        Status: Page Timeout (0x04)
        Handle: 65535
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
	Sound Products Inc)
        Link type: ACL (0x01)
        Encryption: Disabled (0x00)
[1644965.827560] Bluetooth: hci0: Ignoring HCI_Connection_Complete for invalid handle

Because of it is impossible to cleanup the connections properly since
the stack would attempt to cancel the connection which is no longer in
progress causing the following trace:

< HCI Command: Create Connection Cancel (0x01|0x0008) plen 6
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
	Sound Products Inc)
= bluetoothd: src/profile.c:record_cb() Unable to get Hands-Free Voice
	gateway SDP record: Connection timed out
> HCI Event: Command Complete (0x0e) plen 10
      Create Connection Cancel (0x01|0x0008) ncmd 1
        Status: Unknown Connection Identifier (0x02)
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
	Sound Products Inc)
< HCI Command: Create Connection Cancel (0x01|0x0008) plen 6
        Address: 94:DB:56:XX:XX:XX (Sony Home Entertainment&
	Sound Products Inc)

Fixes: d5ebaa7c ("Bluetooth: hci_event: Ignore multiple conn complete events")
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: default avatarMarcel Holtmann <marcel@holtmann.org>
parent acb16b39
...@@ -578,6 +578,7 @@ enum { ...@@ -578,6 +578,7 @@ enum {
#define HCI_ERROR_CONNECTION_TIMEOUT 0x08 #define HCI_ERROR_CONNECTION_TIMEOUT 0x08
#define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d #define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d
#define HCI_ERROR_REJ_BAD_ADDR 0x0f #define HCI_ERROR_REJ_BAD_ADDR 0x0f
#define HCI_ERROR_INVALID_PARAMETERS 0x12
#define HCI_ERROR_REMOTE_USER_TERM 0x13 #define HCI_ERROR_REMOTE_USER_TERM 0x13
#define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14 #define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14
#define HCI_ERROR_REMOTE_POWER_OFF 0x15 #define HCI_ERROR_REMOTE_POWER_OFF 0x15
......
...@@ -3067,13 +3067,9 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -3067,13 +3067,9 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
{ {
struct hci_ev_conn_complete *ev = data; struct hci_ev_conn_complete *ev = data;
struct hci_conn *conn; struct hci_conn *conn;
u8 status = ev->status;
if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) { bt_dev_dbg(hdev, "status 0x%2.2x", status);
bt_dev_err(hdev, "Ignoring HCI_Connection_Complete for invalid handle");
return;
}
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
hci_dev_lock(hdev); hci_dev_lock(hdev);
...@@ -3122,8 +3118,14 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -3122,8 +3118,14 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
goto unlock; goto unlock;
} }
if (!ev->status) { if (!status) {
conn->handle = __le16_to_cpu(ev->handle); conn->handle = __le16_to_cpu(ev->handle);
if (conn->handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
conn->handle, HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
goto done;
}
if (conn->type == ACL_LINK) { if (conn->type == ACL_LINK) {
conn->state = BT_CONFIG; conn->state = BT_CONFIG;
...@@ -3164,18 +3166,18 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -3164,18 +3166,18 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
hci_send_cmd(hdev, HCI_OP_CHANGE_CONN_PTYPE, sizeof(cp), hci_send_cmd(hdev, HCI_OP_CHANGE_CONN_PTYPE, sizeof(cp),
&cp); &cp);
} }
} else {
conn->state = BT_CLOSED;
if (conn->type == ACL_LINK)
mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
} }
if (conn->type == ACL_LINK) if (conn->type == ACL_LINK)
hci_sco_setup(conn, ev->status); hci_sco_setup(conn, ev->status);
if (ev->status) { done:
hci_connect_cfm(conn, ev->status); if (status) {
conn->state = BT_CLOSED;
if (conn->type == ACL_LINK)
mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, status);
hci_connect_cfm(conn, status);
hci_conn_del(conn); hci_conn_del(conn);
} else if (ev->link_type == SCO_LINK) { } else if (ev->link_type == SCO_LINK) {
switch (conn->setting & SCO_AIRMODE_MASK) { switch (conn->setting & SCO_AIRMODE_MASK) {
...@@ -3185,7 +3187,7 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -3185,7 +3187,7 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
break; break;
} }
hci_connect_cfm(conn, ev->status); hci_connect_cfm(conn, status);
} }
unlock: unlock:
...@@ -4676,6 +4678,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -4676,6 +4678,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
{ {
struct hci_ev_sync_conn_complete *ev = data; struct hci_ev_sync_conn_complete *ev = data;
struct hci_conn *conn; struct hci_conn *conn;
u8 status = ev->status;
switch (ev->link_type) { switch (ev->link_type) {
case SCO_LINK: case SCO_LINK:
...@@ -4690,12 +4693,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -4690,12 +4693,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
return; return;
} }
if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) { bt_dev_dbg(hdev, "status 0x%2.2x", status);
bt_dev_err(hdev, "Ignoring HCI_Sync_Conn_Complete for invalid handle");
return;
}
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
hci_dev_lock(hdev); hci_dev_lock(hdev);
...@@ -4729,9 +4727,17 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -4729,9 +4727,17 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
goto unlock; goto unlock;
} }
switch (ev->status) { switch (status) {
case 0x00: case 0x00:
conn->handle = __le16_to_cpu(ev->handle); conn->handle = __le16_to_cpu(ev->handle);
if (conn->handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
conn->handle, HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
conn->state = BT_CLOSED;
break;
}
conn->state = BT_CONNECTED; conn->state = BT_CONNECTED;
conn->type = ev->link_type; conn->type = ev->link_type;
...@@ -4775,8 +4781,8 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data, ...@@ -4775,8 +4781,8 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
} }
} }
hci_connect_cfm(conn, ev->status); hci_connect_cfm(conn, status);
if (ev->status) if (status)
hci_conn_del(conn); hci_conn_del(conn);
unlock: unlock:
...@@ -5527,11 +5533,6 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, ...@@ -5527,11 +5533,6 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
struct smp_irk *irk; struct smp_irk *irk;
u8 addr_type; u8 addr_type;
if (handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Ignoring HCI_LE_Connection_Complete for invalid handle");
return;
}
hci_dev_lock(hdev); hci_dev_lock(hdev);
/* All controllers implicitly stop advertising in the event of a /* All controllers implicitly stop advertising in the event of a
...@@ -5603,6 +5604,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status, ...@@ -5603,6 +5604,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
conn->dst_type = ev_bdaddr_type(hdev, conn->dst_type, NULL); conn->dst_type = ev_bdaddr_type(hdev, conn->dst_type, NULL);
if (handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x", handle,
HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
}
if (status) { if (status) {
hci_le_conn_failed(conn, status); hci_le_conn_failed(conn, status);
goto unlock; goto unlock;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment