From: Luiz Augusto von Dentz <[email protected]>
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 <[email protected]>
---
net/bluetooth/iso.c | 61 ++++++++++++++++++++++++++++-----------------
1 file changed, 38 insertions(+), 23 deletions(-)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index e23aabf4e0cf..035bb5d25f85 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -261,13 +261,13 @@ static int iso_connect_bis(struct sock *sk)
if (!bis_capable(hdev)) {
err = -EOPNOTSUPP;
- goto done;
+ goto unlock;
}
/* Fail if out PHYs are marked as disabled */
if (!iso_pi(sk)->qos.out.phy) {
err = -EINVAL;
- goto done;
+ goto unlock;
}
hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst,
@@ -276,22 +276,27 @@ static int iso_connect_bis(struct sock *sk)
iso_pi(sk)->base);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
- goto done;
+ goto unlock;
}
conn = iso_conn_add(hcon);
if (!conn) {
hci_conn_drop(hcon);
err = -ENOMEM;
- goto done;
+ goto unlock;
}
+ hci_dev_unlock(hdev);
+ hci_dev_put(hdev);
+
+ lock_sock(sk);
+
/* Update source addr of the socket */
bacpy(&iso_pi(sk)->src, &hcon->src);
err = iso_chan_add(conn, sk, NULL);
if (err)
- goto done;
+ goto release;
if (hcon->state == BT_CONNECTED) {
iso_sock_clear_timer(sk);
@@ -301,7 +306,11 @@ static int iso_connect_bis(struct sock *sk)
iso_sock_set_timer(sk, sk->sk_sndtimeo);
}
-done:
+release:
+ release_sock(sk);
+ return err;
+
+unlock:
hci_dev_unlock(hdev);
hci_dev_put(hdev);
return err;
@@ -325,13 +334,13 @@ static int iso_connect_cis(struct sock *sk)
if (!cis_central_capable(hdev)) {
err = -EOPNOTSUPP;
- goto done;
+ goto unlock;
}
/* Fail if either PHYs are marked as disabled */
if (!iso_pi(sk)->qos.in.phy && !iso_pi(sk)->qos.out.phy) {
err = -EINVAL;
- goto done;
+ goto unlock;
}
/* Just bind if DEFER_SETUP has been set */
@@ -341,7 +350,7 @@ static int iso_connect_cis(struct sock *sk)
&iso_pi(sk)->qos);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
- goto done;
+ goto unlock;
}
} else {
hcon = hci_connect_cis(hdev, &iso_pi(sk)->dst,
@@ -349,7 +358,7 @@ static int iso_connect_cis(struct sock *sk)
&iso_pi(sk)->qos);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
- goto done;
+ goto unlock;
}
}
@@ -357,15 +366,20 @@ static int iso_connect_cis(struct sock *sk)
if (!conn) {
hci_conn_drop(hcon);
err = -ENOMEM;
- goto done;
+ goto unlock;
}
+ hci_dev_unlock(hdev);
+ hci_dev_put(hdev);
+
+ lock_sock(sk);
+
/* Update source addr of the socket */
bacpy(&iso_pi(sk)->src, &hcon->src);
err = iso_chan_add(conn, sk, NULL);
if (err)
- goto done;
+ goto release;
if (hcon->state == BT_CONNECTED) {
iso_sock_clear_timer(sk);
@@ -378,7 +392,11 @@ static int iso_connect_cis(struct sock *sk)
iso_sock_set_timer(sk, sk->sk_sndtimeo);
}
-done:
+release:
+ release_sock(sk);
+ return err;
+
+unlock:
hci_dev_unlock(hdev);
hci_dev_put(hdev);
return err;
@@ -832,20 +850,23 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr *addr,
bacpy(&iso_pi(sk)->dst, &sa->iso_bdaddr);
iso_pi(sk)->dst_type = sa->iso_bdaddr_type;
+ release_sock(sk);
+
if (bacmp(&iso_pi(sk)->dst, BDADDR_ANY))
err = iso_connect_cis(sk);
else
err = iso_connect_bis(sk);
if (err)
- goto done;
+ return err;
+
+ lock_sock(sk);
if (!test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
err = bt_sock_wait_state(sk, BT_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK));
}
-done:
release_sock(sk);
return err;
}
@@ -1101,28 +1122,22 @@ static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
{
struct sock *sk = sock->sk;
struct iso_pinfo *pi = iso_pi(sk);
- int err;
BT_DBG("sk %p", sk);
- lock_sock(sk);
-
if (test_and_clear_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
switch (sk->sk_state) {
case BT_CONNECT2:
+ lock_sock(sk);
iso_conn_defer_accept(pi->conn->hcon);
sk->sk_state = BT_CONFIG;
release_sock(sk);
return 0;
case BT_CONNECT:
- err = iso_connect_cis(sk);
- release_sock(sk);
- return err;
+ return iso_connect_cis(sk);
}
}
- release_sock(sk);
-
return bt_sock_recvmsg(sock, msg, len, flags);
}
--
2.37.3
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=702363
---Test result---
Test Summary:
CheckPatch PASS 0.59 seconds
GitLint PASS 0.29 seconds
SubjectPrefix PASS 0.08 seconds
BuildKernel PASS 35.19 seconds
BuildKernel32 PASS 31.79 seconds
TestRunnerSetup PASS 437.89 seconds
TestRunner_l2cap-tester PASS 16.29 seconds
TestRunner_iso-tester PASS 16.04 seconds
TestRunner_bnep-tester PASS 5.57 seconds
TestRunner_mgmt-tester PASS 108.97 seconds
TestRunner_rfcomm-tester PASS 9.58 seconds
TestRunner_sco-tester PASS 8.98 seconds
TestRunner_ioctl-tester PASS 10.23 seconds
TestRunner_mesh-tester PASS 7.03 seconds
TestRunner_smp-tester PASS 8.72 seconds
TestRunner_userchan-tester PASS 5.83 seconds
IncrementalBuild PASS 32.32 seconds
---
Regards,
Linux Bluetooth
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Tue, 6 Dec 2022 17:35:01 -0800 you wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 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
>
> [...]
Here is the summary with links:
- Bluetooth: ISO: Avoid circular locking dependency
https://git.kernel.org/bluetooth/bluetooth-next/c/9e5cfe86387a
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html