From 229c1b5f617945cedce0dac4f64f2a7b24d19634 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Thu, 16 May 2024 14:11:58 +0000 Subject: [PATCH] Revert "Bluetooth: hci_conn: Consolidate code for aborting connections" This reverts commit 6083089ab00631617f9eac678df3ab050a9d837a which is commit a13f316e90fdb1fb6df6582e845aa9b3270f3581 upstream. It breaks the Android kernel abi and can be brought back in the future in an abi-safe way if it is really needed. Bug: 161946584 Change-Id: Ic482b9be644466e06baa4bc0f01a7cbf63b905fb Signed-off-by: Greg Kroah-Hartman --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_conn.c | 158 ++++++++++++++++++++++++------- net/bluetooth/hci_sync.c | 23 ++--- net/bluetooth/mgmt.c | 15 ++- 4 files changed, 149 insertions(+), 49 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index bab16a3fd647..357d4f5a38ac 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -745,7 +745,6 @@ struct hci_conn { unsigned long flags; enum conn_reasons conn_reason; - __u8 abort_reason; __u32 clock; __u16 clock_accuracy; @@ -765,6 +764,7 @@ struct hci_conn { struct delayed_work auto_accept_work; struct delayed_work idle_work; struct delayed_work le_conn_timeout; + struct work_struct le_scan_cleanup; struct device dev; struct dentry *debugfs; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index f752a9f9bb9c..12d36875358b 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -175,6 +175,57 @@ static void hci_conn_cleanup(struct hci_conn *conn) hci_dev_put(hdev); } +static void le_scan_cleanup(struct work_struct *work) +{ + struct hci_conn *conn = container_of(work, struct hci_conn, + le_scan_cleanup); + struct hci_dev *hdev = conn->hdev; + struct hci_conn *c = NULL; + + BT_DBG("%s hcon %p", hdev->name, conn); + + hci_dev_lock(hdev); + + /* Check that the hci_conn is still around */ + rcu_read_lock(); + list_for_each_entry_rcu(c, &hdev->conn_hash.list, list) { + if (c == conn) + break; + } + rcu_read_unlock(); + + if (c == conn) { + hci_connect_le_scan_cleanup(conn, 0x00); + hci_conn_cleanup(conn); + } + + hci_dev_unlock(hdev); + hci_dev_put(hdev); + hci_conn_put(conn); +} + +static void hci_connect_le_scan_remove(struct hci_conn *conn) +{ + BT_DBG("%s hcon %p", conn->hdev->name, conn); + + /* We can't call hci_conn_del/hci_conn_cleanup here since that + * could deadlock with another hci_conn_del() call that's holding + * hci_dev_lock and doing cancel_delayed_work_sync(&conn->disc_work). + * Instead, grab temporary extra references to the hci_dev and + * hci_conn and perform the necessary cleanup in a separate work + * callback. + */ + + hci_dev_hold(conn->hdev); + hci_conn_get(conn); + + /* Even though we hold a reference to the hdev, many other + * things might get cleaned up meanwhile, including the hdev's + * own workqueue, so we can't use that for scheduling. + */ + schedule_work(&conn->le_scan_cleanup); +} + static void hci_acl_create_connection(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; @@ -621,6 +672,13 @@ static void hci_conn_timeout(struct work_struct *work) if (refcnt > 0) return; + /* LE connections in scanning state need special handling */ + if (conn->state == BT_CONNECT && conn->type == LE_LINK && + test_bit(HCI_CONN_SCANNING, &conn->flags)) { + hci_connect_le_scan_remove(conn); + return; + } + hci_abort_conn(conn, hci_proto_disconn_ind(conn)); } @@ -992,6 +1050,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, INIT_DELAYED_WORK(&conn->auto_accept_work, hci_conn_auto_accept); INIT_DELAYED_WORK(&conn->idle_work, hci_conn_idle); INIT_DELAYED_WORK(&conn->le_conn_timeout, le_conn_timeout); + INIT_WORK(&conn->le_scan_cleanup, le_scan_cleanup); atomic_set(&conn->refcnt, 0); @@ -2778,46 +2837,81 @@ u32 hci_conn_get_phy(struct hci_conn *conn) return phys; } -static int abort_conn_sync(struct hci_dev *hdev, void *data) -{ - struct hci_conn *conn; - u16 handle = PTR_ERR(data); - - conn = hci_conn_hash_lookup_handle(hdev, handle); - if (!conn) - return 0; - - return hci_abort_conn_sync(hdev, conn, conn->abort_reason); -} - int hci_abort_conn(struct hci_conn *conn, u8 reason) { - struct hci_dev *hdev = conn->hdev; + int r = 0; - /* If abort_reason has already been set it means the connection is - * already being aborted so don't attempt to overwrite it. - */ - if (conn->abort_reason) + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) return 0; - bt_dev_dbg(hdev, "handle 0x%2.2x reason 0x%2.2x", conn->handle, reason); + switch (conn->state) { + case BT_CONNECTED: + case BT_CONFIG: + if (conn->type == AMP_LINK) { + struct hci_cp_disconn_phy_link cp; - conn->abort_reason = reason; + cp.phy_handle = HCI_PHY_HANDLE(conn->handle); + cp.reason = reason; + r = hci_send_cmd(conn->hdev, HCI_OP_DISCONN_PHY_LINK, + sizeof(cp), &cp); + } else { + struct hci_cp_disconnect dc; - /* If the connection is pending check the command opcode since that - * might be blocking on hci_cmd_sync_work while waiting its respective - * event so we need to hci_cmd_sync_cancel to cancel it. - */ - if (conn->state == BT_CONNECT && hdev->req_status == HCI_REQ_PEND) { - switch (hci_skb_event(hdev->sent_cmd)) { - case HCI_EV_LE_CONN_COMPLETE: - case HCI_EV_LE_ENHANCED_CONN_COMPLETE: - case HCI_EVT_LE_CIS_ESTABLISHED: - hci_cmd_sync_cancel(hdev, -ECANCELED); - break; + dc.handle = cpu_to_le16(conn->handle); + dc.reason = reason; + r = hci_send_cmd(conn->hdev, HCI_OP_DISCONNECT, + sizeof(dc), &dc); } + + conn->state = BT_DISCONN; + + break; + case BT_CONNECT: + if (conn->type == LE_LINK) { + if (test_bit(HCI_CONN_SCANNING, &conn->flags)) + break; + r = hci_send_cmd(conn->hdev, + HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL); + } else if (conn->type == ACL_LINK) { + if (conn->hdev->hci_ver < BLUETOOTH_VER_1_2) + break; + r = hci_send_cmd(conn->hdev, + HCI_OP_CREATE_CONN_CANCEL, + 6, &conn->dst); + } + break; + case BT_CONNECT2: + if (conn->type == ACL_LINK) { + struct hci_cp_reject_conn_req rej; + + bacpy(&rej.bdaddr, &conn->dst); + rej.reason = reason; + + r = hci_send_cmd(conn->hdev, + HCI_OP_REJECT_CONN_REQ, + sizeof(rej), &rej); + } else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) { + struct hci_cp_reject_sync_conn_req rej; + + bacpy(&rej.bdaddr, &conn->dst); + + /* SCO rejection has its own limited set of + * allowed error values (0x0D-0x0F) which isn't + * compatible with most values passed to this + * function. To be safe hard-code one of the + * values that's suitable for SCO. + */ + rej.reason = HCI_ERROR_REJ_LIMITED_RESOURCES; + + r = hci_send_cmd(conn->hdev, + HCI_OP_REJECT_SYNC_CONN_REQ, + sizeof(rej), &rej); + } + break; + default: + conn->state = BT_CLOSED; + break; } - return hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle), - NULL); + return r; } diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c03729c10fdd..31dd064d77a4 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5230,27 +5230,22 @@ 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) { - /* Return reason if scanning since the connection shall probably be - * cleanup directly. - */ if (test_bit(HCI_CONN_SCANNING, &conn->flags)) - return reason; + return 0; - if (conn->role == HCI_ROLE_SLAVE || - test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags)) return 0; return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL, 0, NULL, HCI_CMD_TIMEOUT); } -static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn, - u8 reason) +static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn) { if (conn->type == LE_LINK) - return hci_le_connect_cancel_sync(hdev, conn, reason); + return hci_le_connect_cancel_sync(hdev, conn); if (hdev->hci_ver < BLUETOOTH_VER_1_2) return 0; @@ -5303,11 +5298,9 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) case BT_CONFIG: return hci_disconnect_sync(hdev, conn, reason); case BT_CONNECT: - err = hci_connect_cancel_sync(hdev, conn, reason); + err = hci_connect_cancel_sync(hdev, conn); /* Cleanup hci_conn object if it cannot be cancelled as it - * likelly means the controller and host stack are out of sync - * or in case of LE it was still scanning so it can be cleanup - * safely. + * likelly means the controller and host stack are out of sync. */ if (err) { hci_dev_lock(hdev); @@ -6222,7 +6215,7 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn) done: if (err == -ETIMEDOUT) - hci_le_connect_cancel_sync(hdev, conn, 0x00); + hci_le_connect_cancel_sync(hdev, conn); /* Re-enable advertising after the connection attempt is finished. */ hci_resume_advertising_sync(hdev); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index efeea7994d22..922938a7fd15 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -3583,6 +3583,18 @@ unlock: return err; } +static int abort_conn_sync(struct hci_dev *hdev, void *data) +{ + struct hci_conn *conn; + u16 handle = PTR_ERR(data); + + conn = hci_conn_hash_lookup_handle(hdev, handle); + if (!conn) + return 0; + + return hci_abort_conn_sync(hdev, conn, HCI_ERROR_REMOTE_USER_TERM); +} + static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data, u16 len) { @@ -3633,7 +3645,8 @@ static int cancel_pair_device(struct sock *sk, struct hci_dev *hdev, void *data, le_addr_type(addr->type)); if (conn->conn_reason == CONN_REASON_PAIR_DEVICE) - hci_abort_conn(conn, HCI_ERROR_REMOTE_USER_TERM); + hci_cmd_sync_queue(hdev, abort_conn_sync, ERR_PTR(conn->handle), + NULL); unlock: hci_dev_unlock(hdev);