Skip to content

Commit 0013017

Browse files
Vudentzgregkh
Vudentz
authored and
gregkh
committed
Bluetooth: L2CAP: Fix deadlock
commit f1a8f40 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: 89e856e ("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>
1 parent b13982c commit 0013017

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
@@ -35,6 +35,8 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
3535
int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
3636
const void *param, u8 event, u32 timeout,
3737
struct sock *sk);
38+
int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
39+
const void *param, u32 timeout);
3840

3941
void hci_cmd_sync_init(struct hci_dev *hdev);
4042
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)
@@ -733,6 +689,7 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
733689
{
734690
struct hci_dev *hdev;
735691
struct hci_dev_req dr;
692+
__le16 policy;
736693
int err = 0;
737694

738695
if (copy_from_user(&dr, arg, sizeof(dr)))
@@ -764,8 +721,8 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
764721

765722
switch (cmd) {
766723
case HCISETAUTH:
767-
err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt,
768-
HCI_INIT_TIMEOUT, NULL);
724+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_AUTH_ENABLE,
725+
1, &dr.dev_opt, HCI_CMD_TIMEOUT);
769726
break;
770727

771728
case HCISETENCRYPT:
@@ -776,19 +733,23 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
776733

777734
if (!test_bit(HCI_AUTH, &hdev->flags)) {
778735
/* Auth must be enabled first */
779-
err = hci_req_sync(hdev, hci_auth_req, dr.dev_opt,
780-
HCI_INIT_TIMEOUT, NULL);
736+
err = __hci_cmd_sync_status(hdev,
737+
HCI_OP_WRITE_AUTH_ENABLE,
738+
1, &dr.dev_opt,
739+
HCI_CMD_TIMEOUT);
781740
if (err)
782741
break;
783742
}
784743

785-
err = hci_req_sync(hdev, hci_encrypt_req, dr.dev_opt,
786-
HCI_INIT_TIMEOUT, NULL);
744+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_ENCRYPT_MODE,
745+
1, &dr.dev_opt,
746+
HCI_CMD_TIMEOUT);
787747
break;
788748

789749
case HCISETSCAN:
790-
err = hci_req_sync(hdev, hci_scan_req, dr.dev_opt,
791-
HCI_INIT_TIMEOUT, NULL);
750+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_SCAN_ENABLE,
751+
1, &dr.dev_opt,
752+
HCI_CMD_TIMEOUT);
792753

793754
/* Ensure that the connectable and discoverable states
794755
* get correctly modified as this was a non-mgmt change.
@@ -798,8 +759,11 @@ int hci_dev_cmd(unsigned int cmd, void __user *arg)
798759
break;
799760

800761
case HCISETLINKPOL:
801-
err = hci_req_sync(hdev, hci_linkpol_req, dr.dev_opt,
802-
HCI_INIT_TIMEOUT, NULL);
762+
policy = cpu_to_le16(dr.dev_opt);
763+
764+
err = __hci_cmd_sync_status(hdev, HCI_OP_WRITE_DEF_LINK_POLICY,
765+
2, &policy,
766+
HCI_CMD_TIMEOUT);
803767
break;
804768

805769
case HCISETLINKMODE:

‎net/bluetooth/hci_sync.c‎

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

282+
int hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
283+
const void *param, u32 timeout)
284+
{
285+
int err;
286+
287+
hci_req_sync_lock(hdev);
288+
err = __hci_cmd_sync_status(hdev, opcode, plen, param, timeout);
289+
hci_req_sync_unlock(hdev);
290+
291+
return err;
292+
}
293+
EXPORT_SYMBOL(hci_cmd_sync_status);
294+
282295
static void hci_cmd_sync_work(struct work_struct *work)
283296
{
284297
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
@@ -7798,6 +7798,8 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
77987798

77997799
BT_DBG("chan %p, len %d", chan, skb->len);
78007800

7801+
l2cap_chan_lock(chan);
7802+
78017803
if (chan->state != BT_BOUND && chan->state != BT_CONNECTED)
78027804
goto drop;
78037805

@@ -7814,6 +7816,7 @@ static void l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm,
78147816
}
78157817

78167818
drop:
7819+
l2cap_chan_unlock(chan);
78177820
l2cap_chan_put(chan);
78187821
free_skb:
78197822
kfree_skb(skb);

‎net/bluetooth/l2cap_sock.c‎

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

1526-
/* To avoid race with sock_release, a chan lock needs to be added here
1527-
* to synchronize the sock.
1528-
*/
1529-
l2cap_chan_hold(chan);
1530-
l2cap_chan_lock(chan);
15311526
sk = chan->data;
1532-
1533-
if (!sk) {
1534-
l2cap_chan_unlock(chan);
1535-
l2cap_chan_put(chan);
1527+
if (!sk)
15361528
return -ENXIO;
1537-
}
15381529

15391530
pi = l2cap_pi(sk);
15401531
lock_sock(sk);
@@ -1586,8 +1577,6 @@ static int l2cap_sock_recv_cb(struct l2cap_chan *chan, struct sk_buff *skb)
15861577

15871578
done:
15881579
release_sock(sk);
1589-
l2cap_chan_unlock(chan);
1590-
l2cap_chan_put(chan);
15911580

15921581
return err;
15931582
}

0 commit comments

Comments
 (0)