[PW_SID:1079574] [v2] Bluetooth: l2cap: defer conn param update to avoid conn->lock/hdev->lock inversion#59
[PW_SID:1079574] [v2] Bluetooth: l2cap: defer conn param update to avoid conn->lock/hdev->lock inversion#59BluezTestBot wants to merge 1 commit intoworkflowfrom
Conversation
…ock inversion
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: f044eb0 ("Bluetooth: Store latency and supervision timeout in connection params")
Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
|
CheckPatch |
|
GitLint |
|
SubjectPrefix |
|
BuildKernel |
|
CheckAllWarning |
|
CheckSparse |
|
BuildKernel32 |
|
TestRunnerSetup |
|
TestRunner_l2cap-tester |
|
TestRunner_iso-tester |
|
TestRunner_bnep-tester |
|
TestRunner_mgmt-tester |
|
TestRunner_rfcomm-tester |
|
TestRunner_sco-tester |
|
TestRunner_ioctl-tester |
|
TestRunner_mesh-tester |
|
TestRunner_smp-tester |
|
TestRunner_userchan-tester |
|
TestRunner_6lowpan-tester |
|
IncrementalBuild |
2 participants
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: f044eb0 ("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):
net/bluetooth/l2cap_core.c | 93 ++++++++++++++++++++++++++++++++++----
1 file changed, 85 insertions(+), 8 deletions(-)