From: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
To: marcel@holtmann.org, luiz.dentz@gmail.com
Cc: linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Subject: [PATCH] Bluetooth: l2cap: defer conn param update to avoid conn->lock/hdev->lock inversion
Date: Fri, 10 Apr 2026 03:21:22 +0500 [thread overview]
Message-ID: <20260409222122.21394-1-mikhail.v.gavrilov@gmail.com> (raw)
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 deferring the hci_le_conn_update() and mgmt_new_conn_param()
calls to the hci_cmd_sync workqueue via hci_cmd_sync_queue(), which runs
outside any of the involved locks.
Fixes: f044eb0524a0 ("Bluetooth: Store latency and supervision timeout in connection params")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
---
net/bluetooth/l2cap_core.c | 76 ++++++++++++++++++++++++++++++++++----
1 file changed, 69 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 95c65fece39b..e59d3af250ef 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4670,6 +4670,59 @@ 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_params *params;
+ struct hci_cp_le_conn_update cp;
+ u8 store_hint = 0x00;
+
+ hci_dev_lock(hdev);
+
+ 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 +4730,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;
@@ -4707,14 +4761,22 @@ static inline int l2cap_conn_param_update_req(struct l2cap_conn *conn,
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 = kmalloc(sizeof(*d), GFP_KERNEL);
+ if (!d)
+ return 0;
+ 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
next reply other threads:[~2026-04-09 22:21 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 22:21 Mikhail Gavrilov [this message]
2026-04-09 23:18 ` Bluetooth: l2cap: defer conn param update to avoid conn->lock/hdev->lock inversion bluez.test.bot
2026-04-10 5:45 ` [PATCH] " Paul Menzel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260409222122.21394-1-mikhail.v.gavrilov@gmail.com \
--to=mikhail.v.gavrilov@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luiz.dentz@gmail.com \
--cc=marcel@holtmann.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox