Skip to content

Commit c524f95

Browse files
Vudentzgregkh
Vudentz
authored and
gregkh
committed
Bluetooth: ISO: Avoid circular locking dependency
[ Upstream commit 241f519 ] This attempts to avoid circular locking dependency between sock_lock and hdev_lock: WARNING: possible circular locking dependency detected 6.0.0-rc7-03728-g18dd8ab0a783 #3 Not tainted ------------------------------------------------------ kworker/u3:2/53 is trying to acquire lock: ffff888000254130 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0}, at: iso_conn_del+0xbd/0x1d0 but task is already holding lock: ffffffff9f39a080 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_le_cis_estabilished_evt+0x1b5/0x500 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (hci_cb_list_lock){+.+.}-{3:3}: __mutex_lock+0x10e/0xfe0 hci_le_remote_feat_complete_evt+0x17f/0x320 hci_event_packet+0x39c/0x7d0 hci_rx_work+0x2bf/0x950 process_one_work+0x569/0x980 worker_thread+0x2a3/0x6f0 kthread+0x153/0x180 ret_from_fork+0x22/0x30 -> #1 (&hdev->lock){+.+.}-{3:3}: __mutex_lock+0x10e/0xfe0 iso_connect_cis+0x6f/0x5a0 iso_sock_connect+0x1af/0x710 __sys_connect+0x17e/0x1b0 __x64_sys_connect+0x37/0x50 do_syscall_64+0x43/0x90 entry_SYSCALL_64_after_hwframe+0x62/0xcc -> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0}: __lock_acquire+0x1b51/0x33d0 lock_acquire+0x16f/0x3b0 lock_sock_nested+0x32/0x80 iso_conn_del+0xbd/0x1d0 iso_connect_cfm+0x226/0x680 hci_le_cis_estabilished_evt+0x1ed/0x500 hci_event_packet+0x39c/0x7d0 hci_rx_work+0x2bf/0x950 process_one_work+0x569/0x980 worker_thread+0x2a3/0x6f0 kthread+0x153/0x180 ret_from_fork+0x22/0x30 other info that might help us debug this: Chain exists of: sk_lock-AF_BLUETOOTH-BTPROTO_ISO --> &hdev->lock --> hci_cb_list_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(hci_cb_list_lock); lock(&hdev->lock); lock(hci_cb_list_lock); lock(sk_lock-AF_BLUETOOTH-BTPROTO_ISO); *** DEADLOCK *** 4 locks held by kworker/u3:2/53: #0: ffff8880021d9130 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work+0x4ad/0x980 #1: ffff888002387de0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work+0x4ad/0x980 #2: ffff888001ac0070 (&hdev->lock){+.+.}-{3:3}, at: hci_le_cis_estabilished_evt+0xc3/0x500 #3: ffffffff9f39a080 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_le_cis_estabilished_evt+0x1b5/0x500 Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Stable-dep-of: 6a5ad25 ("Bluetooth: ISO: Fix possible circular locking dependency") Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 8ac6043 commit c524f95

File tree

1 file changed

+38
-23
lines changed

1 file changed

+38
-23
lines changed

‎net/bluetooth/iso.c‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -261,36 +261,41 @@ static int iso_connect_bis(struct sock *sk)
261261

262262
if (!bis_capable(hdev)) {
263263
err = -EOPNOTSUPP;
264-
goto done;
264+
goto unlock;
265265
}
266266

267267
/* Fail if out PHYs are marked as disabled */
268268
if (!iso_pi(sk)->qos.out.phy) {
269269
err = -EINVAL;
270-
goto done;
270+
goto unlock;
271271
}
272272

273273
hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst, iso_pi(sk)->dst_type,
274274
&iso_pi(sk)->qos, iso_pi(sk)->base_len,
275275
iso_pi(sk)->base);
276276
if (IS_ERR(hcon)) {
277277
err = PTR_ERR(hcon);
278-
goto done;
278+
goto unlock;
279279
}
280280

281281
conn = iso_conn_add(hcon);
282282
if (!conn) {
283283
hci_conn_drop(hcon);
284284
err = -ENOMEM;
285-
goto done;
285+
goto unlock;
286286
}
287287

288+
hci_dev_unlock(hdev);
289+
hci_dev_put(hdev);
290+
291+
lock_sock(sk);
292+
288293
/* Update source addr of the socket */
289294
bacpy(&iso_pi(sk)->src, &hcon->src);
290295

291296
err = iso_chan_add(conn, sk, NULL);
292297
if (err)
293-
goto done;
298+
goto release;
294299

295300
if (hcon->state == BT_CONNECTED) {
296301
iso_sock_clear_timer(sk);
@@ -300,7 +305,11 @@ static int iso_connect_bis(struct sock *sk)
300305
iso_sock_set_timer(sk, sk->sk_sndtimeo);
301306
}
302307

303-
done:
308+
release:
309+
release_sock(sk);
310+
return err;
311+
312+
unlock:
304313
hci_dev_unlock(hdev);
305314
hci_dev_put(hdev);
306315
return err;
@@ -324,13 +333,13 @@ static int iso_connect_cis(struct sock *sk)
324333

325334
if (!cis_central_capable(hdev)) {
326335
err = -EOPNOTSUPP;
327-
goto done;
336+
goto unlock;
328337
}
329338

330339
/* Fail if either PHYs are marked as disabled */
331340
if (!iso_pi(sk)->qos.in.phy && !iso_pi(sk)->qos.out.phy) {
332341
err = -EINVAL;
333-
goto done;
342+
goto unlock;
334343
}
335344

336345
/* Just bind if DEFER_SETUP has been set */
@@ -340,31 +349,36 @@ static int iso_connect_cis(struct sock *sk)
340349
&iso_pi(sk)->qos);
341350
if (IS_ERR(hcon)) {
342351
err = PTR_ERR(hcon);
343-
goto done;
352+
goto unlock;
344353
}
345354
} else {
346355
hcon = hci_connect_cis(hdev, &iso_pi(sk)->dst,
347356
le_addr_type(iso_pi(sk)->dst_type),
348357
&iso_pi(sk)->qos);
349358
if (IS_ERR(hcon)) {
350359
err = PTR_ERR(hcon);
351-
goto done;
360+
goto unlock;
352361
}
353362
}
354363

355364
conn = iso_conn_add(hcon);
356365
if (!conn) {
357366
hci_conn_drop(hcon);
358367
err = -ENOMEM;
359-
goto done;
368+
goto unlock;
360369
}
361370

371+
hci_dev_unlock(hdev);
372+
hci_dev_put(hdev);
373+
374+
lock_sock(sk);
375+
362376
/* Update source addr of the socket */
363377
bacpy(&iso_pi(sk)->src, &hcon->src);
364378

365379
err = iso_chan_add(conn, sk, NULL);
366380
if (err)
367-
goto done;
381+
goto release;
368382

369383
if (hcon->state == BT_CONNECTED) {
370384
iso_sock_clear_timer(sk);
@@ -377,7 +391,11 @@ static int iso_connect_cis(struct sock *sk)
377391
iso_sock_set_timer(sk, sk->sk_sndtimeo);
378392
}
379393

380-
done:
394+
release:
395+
release_sock(sk);
396+
return err;
397+
398+
unlock:
381399
hci_dev_unlock(hdev);
382400
hci_dev_put(hdev);
383401
return err;
@@ -831,20 +849,23 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr *addr,
831849
bacpy(&iso_pi(sk)->dst, &sa->iso_bdaddr);
832850
iso_pi(sk)->dst_type = sa->iso_bdaddr_type;
833851

852+
release_sock(sk);
853+
834854
if (bacmp(&iso_pi(sk)->dst, BDADDR_ANY))
835855
err = iso_connect_cis(sk);
836856
else
837857
err = iso_connect_bis(sk);
838858

839859
if (err)
840-
goto done;
860+
return err;
861+
862+
lock_sock(sk);
841863

842864
if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
843865
err = bt_sock_wait_state(sk, BT_CONNECTED,
844866
sock_sndtimeo(sk, flags & O_NONBLOCK));
845867
}
846868

847-
done:
848869
release_sock(sk);
849870
return err;
850871
}
@@ -1099,28 +1120,22 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
10991120
{
11001121
struct sock *sk = sock->sk;
11011122
struct iso_pinfo *pi = iso_pi(sk);
1102-
int err;
11031123

11041124
BT_DBG("sk %p", sk);
11051125

1106-
lock_sock(sk);
1107-
11081126
if (test_and_clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
11091127
switch (sk->sk_state) {
11101128
case BT_CONNECT2:
1129+
lock_sock(sk);
11111130
iso_conn_defer_accept(pi->conn->hcon);
11121131
sk->sk_state = BT_CONFIG;
11131132
release_sock(sk);
11141133
return 0;
11151134
case BT_CONNECT:
1116-
err = iso_connect_cis(sk);
1117-
release_sock(sk);
1118-
return err;
1135+
return iso_connect_cis(sk);
11191136
}
11201137
}
11211138

1122-
release_sock(sk);
1123-
11241139
return bt_sock_recvmsg(sock, msg, len, flags);
11251140
}
11261141

0 commit comments

Comments
 (0)