Skip to content

Commit 172a251

Browse files
Vudentzsmb49
Vudentz
authored and
smb49
committed
Bluetooth: L2CAP: Fix deadlock
BugLink: https://bugs.launchpad.net/bugs/2078304 commit f1a8f402f13f94263cf349216c257b2985100927 upstream. This fixes the following deadlock introduced by 39a92a55be13 ("bluetooth/l2cap: sync sock recv cb and release") ============================================ WARNING: possible recursive locking detected 6.10.0-rc3-g4029dba6b6f1 #6823 Not tainted -------------------------------------------- kworker/u5:0/35 is trying to acquire lock: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_sock_recv_cb+0x44/0x1e0 but task is already holding lock: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_get_chan_by_scid+0xaf/0xd0 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&chan->lock#2/1); lock(&chan->lock#2/1); *** DEADLOCK *** May be due to missing lock nesting notation 3 locks held by kworker/u5:0/35: #0: ffff888002b8a940 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work+0x750/0x930 #1: ffff888002c67dd0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work+0x44e/0x930 #2: ffff888002ec2510 (&chan->lock#2/1){+.+.}-{3:3}, at: l2cap_get_chan_by_scid+0xaf/0xd0 To fix the original problem this introduces l2cap_chan_lock at l2cap_conless_channel to ensure that l2cap_sock_recv_cb is called with chan->lock held. Fixes: 89e856e124f9 ("bluetooth/l2cap: sync sock recv cb and release") Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by: Portia Stephens <portia.stephens@canonical.com> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
1 parent a4f8864 commit 172a251

File tree

5 files changed

+37
-66
lines changed

5 files changed

+37
-66
lines changed

‎include/net/bluetooth/hci_sync.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
3838
int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
3939
const void *param, u8 event, u32 timeout,
4040
struct sock *sk);
41+
int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
42+
const void *param, u32 timeout);
4143

4244
void hci_cmd_sync_init(struct hci_dev *hdev);
4345
void hci_cmd_sync_clear(struct hci_dev *hdev);

‎net/bluetooth/hci_core.c

Lines changed: 18 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -63,50 +63,6 @@ DEFINE_MUTEX(hci_cb_list_lock);
6363
/* HCI ID Numbering */
6464
static DEFINE_IDA(hci_index_ida);
6565

66-
static int hci_scan_req(struct hci_request *req, unsigned long opt)
67-
{
68-
__u8 scan = opt;
69-
70-
BT_DBG("%s %x", req->hdev->name, scan);
71-
72-
/* Inquiry and Page scans */
73-
hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
74-
return 0;
75-
}
76-
77-
static int hci_auth_req(struct hci_request *req, unsigned long opt)
78-
{
79-
__u8 auth = opt;
80-
81-
BT_DBG("%s %x", req->hdev->name, auth);
82-
83-
/* Authentication */
84-
hci_req_add(req, HCI_OP_WRITE_AUTH_ENABLE, 1, &auth);
85-
return 0;
86-
}
87-
88-
static int hci_encrypt_req(struct hci_request *req, unsigned long opt)
89-
{
90-
__u8 encrypt = opt;
91-
92-
BT_DBG("%s %x", req->hdev->name, encrypt);
93-
94-
/* Encryption */
95-
hci_req_add(req, HCI_OP_WRITE_ENCRYPT_MODE, 1, &encrypt);
96-
return 0;
97-
}
98-
99-
static int hci_linkpol_req(struct hci_request *req, unsigned long opt)
100-
{
101-
__le16 policy = cpu_to_le16(opt);
102-
103-
BT_DBG("%s %x", req->hdev->name, policy);
104-
105-
/* Default link policy */
106-
hci_req_add(req, HCI_OP_WRITE_DEF_LINK_POLICY, 2, &policy);
107-
return 0;
108-
}
109-
11066
/* Get HCI device by index.
11167
* Device is held on return. */
11268
struct hci_dev *hci_dev_get(int index)
@@ -728,6 +684,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
728684
{
729685
struct hci_dev *hdev;
730686
struct hci_dev_req dr;
687+
__le16 policy;
731688
int err = 0;
732689

733690
if (copy_from_user(&dr, arg, sizeof(dr)))
@@ -754,8 +711,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
754711

755712
switch (cmd) {
756713
case HCISETAUTH:
757-
err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt,
758-
HCI_INIT_TIMEOUT, NULL);
714+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_AUTH_ENABLE,
715+
1, &dr.dev_opt, HCI_CMD_TIMEOUT);
759716
break;
760717

761718
case HCISETENCRYPT:
@@ -766,19 +723,23 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
766723

767724
if (!test_bit(HCI_AUTH, &hdev->flags)) {
768725
/* Auth must be enabled first */
769-
err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt,
770-
HCI_INIT_TIMEOUT, NULL);
726+
err = __hci_cmd_sync_status(hdev,
727+
HCI_OP_WRITE_AUTH_ENABLE,
728+
1, &dr.dev_opt,
729+
HCI_CMD_TIMEOUT);
771730
if (err)
772731
break;
773732
}
774733

775-
err = hci_req_sync(hdev, hci_encrypt_req, dr.dev_opt,
776-
HCI_INIT_TIMEOUT, NULL);
734+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_ENCRYPT_MODE,
735+
1, &dr.dev_opt,
736+
HCI_CMD_TIMEOUT);
777737
break;
778738

779739
case HCISETSCAN:
780-
err = hci_req_sync(hdev, hci_scan_req, dr.dev_opt,
781-
HCI_INIT_TIMEOUT, NULL);
740+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SCAN_ENABLE,
741+
1, &dr.dev_opt,
742+
HCI_CMD_TIMEOUT);
782743

783744
/* Ensure that the connectable and discoverable states
784745
* get correctly modified as this was a non-mgmt change.
@@ -788,8 +749,11 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
788749
break;
789750

790751
case HCISETLINKPOL:
791-
err = hci_req_sync(hdev, hci_linkpol_req, dr.dev_opt,
792-
HCI_INIT_TIMEOUT, NULL);
752+
policy = cpu_to_le16(dr.dev_opt);
753+
754+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_DEF_LINK_POLICY,
755+
2, &policy,
756+
HCI_CMD_TIMEOUT);
793757
break;
794758

795759
case HCISETLINKMODE:

‎net/bluetooth/hci_sync.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,19 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
280280
}
281281
EXPORT_SYMBOL(__hci_cmd_sync_status);
282282

283+
int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
284+
const void *param, u32 timeout)
285+
{
286+
int err;
287+
288+
hci_req_sync_lock(hdev);
289+
err = __hci_cmd_sync_status(hdev, opcode, plen, param, timeout);
290+
hci_req_sync_unlock(hdev);
291+
292+
return err;
293+
}
294+
EXPORT_SYMBOL(hci_cmd_sync_status);
295+
283296
static void hci_cmd_sync_work(struct work_struct *work)
284297
{
285298
struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_sync_work);

‎net/bluetooth/l2cap_core.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6762,6 +6762,8 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
67626762

67636763
BT_DBG("chan %p, len %d", chan, skb->len);
67646764

6765+
l2cap_chan_lock(chan);
6766+
67656767
if (chan->state != BT_BOUND && chan->state != BT_CONNECTED)
67666768
goto drop;
67676769

@@ -6778,6 +6780,7 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
67786780
}
67796781

67806782
drop:
6783+
l2cap_chan_unlock(chan);
67816784
l2cap_chan_put(chan);
67826785
free_skb:
67836786
kfree_skb(skb);

‎net/bluetooth/l2cap_sock.c

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1489,18 +1489,9 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
14891489
struct l2cap_pinfo *pi;
14901490
int err;
14911491

1492-
/* To avoid race with sock_release, a chan lock needs to be added here
1493-
* to synchronize the sock.
1494-
*/
1495-
l2cap_chan_hold(chan);
1496-
l2cap_chan_lock(chan);
14971492
sk = chan->data;
1498-
1499-
if (!sk) {
1500-
l2cap_chan_unlock(chan);
1501-
l2cap_chan_put(chan);
1493+
if (!sk)
15021494
return -ENXIO;
1503-
}
15041495

15051496
pi = l2cap_pi(sk);
15061497
lock_sock(sk);
@@ -1552,8 +1543,6 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
15521543

15531544
done:
15541545
release_sock(sk);
1555-
l2cap_chan_unlock(chan);
1556-
l2cap_chan_put(chan);
15571546

15581547
return err;
15591548
}

0 commit comments

Comments
 (0)