[PATCH v2] Bluetooth: l2cap: defer conn param update to avoid conn->lock/hdev->lock inversion

[Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>]

When a BLE peripheral sends an L2CAP Connection Parameter Update Request
the processing path is:

  process_pending_rx()          [takes conn->lock]
    l2cap_le_sig_channel()
      l2cap_conn_param_update_req()
        hci_le_conn_update()    [takes hdev->lock]

Meanwhile other code paths take the locks in the opposite order:

  l2cap_chan_connect()          [takes hdev->lock]
    ...
      mutex_lock(&conn->lock)

  l2cap_conn_ready()            [hdev->lock via hci_cb_list_lock]
    ...
      mutex_lock(&conn->lock)

This is a classic AB/BA deadlock which lockdep reports as a circular
locking dependency when connecting a BLE MIDI keyboard (Carry-On FC-49).

Fix this by introducing l2cap_conn_param_update_sync() which is scheduled
via hci_cmd_sync_queue() to run on the hci_cmd_sync workqueue, outside
any of the involved locks.  The necessary connection parameters are
captured into a heap-allocated struct conn_param_update_data and the
sync callback performs the hci_conn_params update, sends the HCI LE
Connection Update command, and notifies mgmt of the new parameters.

To guard against a theoretical handle-reuse race (the connection could
drop and its handle be reassigned between queuing and execution), the
sync callback verifies the connection still exists and its destination
address matches the original request before proceeding.

The allocation is performed before sending the L2CAP_CONN_PARAM_ACCEPTED
response so that on OOM the peer receives a REJECTED response instead of
an inconsistent state where the peer applies new parameters but the local
controller does not.

Fixes: f044eb0524a0 ("Bluetooth: Store latency and supervision timeout in connection params")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---

Changes in v2 (Paul Menzel, Sashiko/Gemini AI review):
- Allocate before sending ACCEPTED response to avoid state mismatch on OOM
- Verify connection handle and address in sync callback against reuse race
- Expand commit message with implementation details

 net/bluetooth/l2cap_core.c | 93 ++++++++++++++++++++++++++++++++++----
 1 file changed, 85 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 95c65fece39b..94dfef9df498 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4670,6 +4670,70 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn,
 	return 0;
 }
 
+struct conn_param_update_data {
+	u16	handle;
+	bdaddr_t dst;
+	u8	dst_type;
+	u16	min;
+	u16	max;
+	u16	latency;
+	u16	to_multiplier;
+};
+
+static int l2cap_conn_param_update_sync(struct hci_dev *hdev, void *data)
+{
+	struct conn_param_update_data *d = data;
+	struct hci_conn *conn;
+	struct hci_conn_params *params;
+	struct hci_cp_le_conn_update cp;
+	u8 store_hint = 0x00;
+
+	/* Verify the connection is still alive and matches the original
+	 * request, since the handle could theoretically be reused after
+	 * a disconnect/reconnect cycle.
+	 */
+	hci_dev_lock(hdev);
+
+	conn = hci_conn_hash_lookup_handle(hdev, d->handle);
+	if (!conn || bacmp(&conn->dst, &d->dst)) {
+		hci_dev_unlock(hdev);
+		return 0;
+	}
+
+	params = hci_conn_params_lookup(hdev, &d->dst, d->dst_type);
+	if (params) {
+		params->conn_min_interval = d->min;
+		params->conn_max_interval = d->max;
+		params->conn_latency = d->latency;
+		params->supervision_timeout = d->to_multiplier;
+		store_hint = 0x01;
+	}
+
+	hci_dev_unlock(hdev);
+
+	memset(&cp, 0, sizeof(cp));
+	cp.handle		= cpu_to_le16(d->handle);
+	cp.conn_interval_min	= cpu_to_le16(d->min);
+	cp.conn_interval_max	= cpu_to_le16(d->max);
+	cp.conn_latency		= cpu_to_le16(d->latency);
+	cp.supervision_timeout	= cpu_to_le16(d->to_multiplier);
+	cp.min_ce_len		= cpu_to_le16(0x0000);
+	cp.max_ce_len		= cpu_to_le16(0x0000);
+
+	hci_send_cmd(hdev, HCI_OP_LE_CONN_UPDATE, sizeof(cp), &cp);
+
+	mgmt_new_conn_param(hdev, &d->dst, d->dst_type, store_hint,
+			    d->min, d->max, d->latency, d->to_multiplier);
+
+	return 0;
+}
+
+static void l2cap_conn_param_update_destroy(struct hci_dev *hdev, void *data,
+					    int err)
+{
+	kfree(data);
+}
+
 static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 					      struct l2cap_cmd_hdr *cmd,
 					      u16 cmd_len, u8 *data)
@@ -4677,6 +4741,7 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 	struct hci_conn *hcon = conn->hcon;
 	struct l2cap_conn_param_update_req *req;
 	struct l2cap_conn_param_update_rsp rsp;
+	struct conn_param_update_data *d;
 	u16 min, max, latency, to_multiplier;
 	int err;
 
@@ -4703,18 +4768,30 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
 	else
 		rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_ACCEPTED);
 
+	if (!err) {
+		d = kmalloc(sizeof(*d), GFP_KERNEL);
+		if (!d) {
+			rsp.result = cpu_to_le16(L2CAP_CONN_PARAM_REJECTED);
+			err = -ENOMEM;
+		}
+	}
+
 	l2cap_send_cmd(conn, cmd->ident, L2CAP_CONN_PARAM_UPDATE_RSP,
 		       sizeof(rsp), &rsp);
 
 	if (!err) {
-		u8 store_hint;
-
-		store_hint = hci_le_conn_update(hcon, min, max, latency,
-						to_multiplier);
-		mgmt_new_conn_param(hcon->hdev, &hcon->dst, hcon->dst_type,
-				    store_hint, min, max, latency,
-				    to_multiplier);
-
+		d->handle = hcon->handle;
+		bacpy(&d->dst, &hcon->dst);
+		d->dst_type = hcon->dst_type;
+		d->min = min;
+		d->max = max;
+		d->latency = latency;
+		d->to_multiplier = to_multiplier;
+
+		if (hci_cmd_sync_queue(hcon->hdev,
+				       l2cap_conn_param_update_sync, d,
+				       l2cap_conn_param_update_destroy) < 0)
+			kfree(d);
 	}
 
 	return 0;
-- 
2.53.0