Bluetooth: SCO: Fix possible circular locking dependency on sco_connect_cfm

This attempts to fix the following trace:

======================================================
WARNING: possible circular locking dependency detected
6.3.0-rc2-g0b93eeba4454 #4703 Not tainted
------------------------------------------------------
kworker/u3:0/46 is trying to acquire lock:
ffff888001fd9130 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}, at:
sco_connect_cfm+0x118/0x4a0

but task is already holding lock:
ffffffff831e3340 (hci_cb_list_lock){+.+.}-{3:3}, at:
hci_sync_conn_complete_evt+0x1ad/0x3d0

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+0x13b/0xcc0
       hci_sync_conn_complete_evt+0x1ad/0x3d0
       hci_event_packet+0x55c/0x7c0
       hci_rx_work+0x34c/0xa00
       process_one_work+0x575/0x910
       worker_thread+0x89/0x6f0
       kthread+0x14e/0x180
       ret_from_fork+0x2b/0x50

-> #1 (&hdev->lock){+.+.}-{3:3}:
       __mutex_lock+0x13b/0xcc0
       sco_sock_connect+0xfc/0x630
       __sys_connect+0x197/0x1b0
       __x64_sys_connect+0x37/0x50
       do_syscall_64+0x42/0x90
       entry_SYSCALL_64_after_hwframe+0x70/0xda

-> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_SCO){+.+.}-{0:0}:
       __lock_acquire+0x18cc/0x3740
       lock_acquire+0x151/0x3a0
       lock_sock_nested+0x32/0x80
       sco_connect_cfm+0x118/0x4a0
       hci_sync_conn_complete_evt+0x1e6/0x3d0
       hci_event_packet+0x55c/0x7c0
       hci_rx_work+0x34c/0xa00
       process_one_work+0x575/0x910
       worker_thread+0x89/0x6f0
       kthread+0x14e/0x180
       ret_from_fork+0x2b/0x50

other info that might help us debug this:

Chain exists of:
  sk_lock-AF_BLUETOOTH-BTPROTO_SCO --> &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_SCO);

 *** DEADLOCK ***

4 locks held by kworker/u3:0/46:
 #0: ffff8880028d1130 ((wq_completion)hci0#2){+.+.}-{0:0}, at:
 process_one_work+0x4c0/0x910
 #1: ffff8880013dfde0 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0},
 at: process_one_work+0x4c0/0x910
 #2: ffff8880025d8070 (&hdev->lock){+.+.}-{3:3}, at:
 hci_sync_conn_complete_evt+0xa6/0x3d0
 #3: ffffffffb79e3340 (hci_cb_list_lock){+.+.}-{3:3}, at:
 hci_sync_conn_complete_evt+0x1ad/0x3d0
Signed-off-by: default avatarLuiz Augusto von Dentz <luiz.von.dentz@intel.com>
parent b76abe46
...@@ -235,27 +235,41 @@ static int sco_chan_add(struct sco_conn *conn, struct sock *sk, ...@@ -235,27 +235,41 @@ static int sco_chan_add(struct sco_conn *conn, struct sock *sk,
return err; return err;
} }
static int sco_connect(struct hci_dev *hdev, struct sock *sk) static int sco_connect(struct sock *sk)
{ {
struct sco_conn *conn; struct sco_conn *conn;
struct hci_conn *hcon; struct hci_conn *hcon;
struct hci_dev *hdev;
int err, type; int err, type;
BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst); BT_DBG("%pMR -> %pMR", &sco_pi(sk)->src, &sco_pi(sk)->dst);
hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
if (!hdev)
return -EHOSTUNREACH;
hci_dev_lock(hdev);
if (lmp_esco_capable(hdev) && !disable_esco) if (lmp_esco_capable(hdev) && !disable_esco)
type = ESCO_LINK; type = ESCO_LINK;
else else
type = SCO_LINK; type = SCO_LINK;
if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT && if (sco_pi(sk)->setting == BT_VOICE_TRANSPARENT &&
(!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) (!lmp_transp_capable(hdev) || !lmp_esco_capable(hdev))) {
return -EOPNOTSUPP; err = -EOPNOTSUPP;
goto unlock;
}
hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst, hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
sco_pi(sk)->setting, &sco_pi(sk)->codec); sco_pi(sk)->setting, &sco_pi(sk)->codec);
if (IS_ERR(hcon)) if (IS_ERR(hcon)) {
return PTR_ERR(hcon); err = PTR_ERR(hcon);
goto unlock;
}
hci_dev_unlock(hdev);
hci_dev_put(hdev);
conn = sco_conn_add(hcon); conn = sco_conn_add(hcon);
if (!conn) { if (!conn) {
...@@ -263,13 +277,15 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk) ...@@ -263,13 +277,15 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk)
return -ENOMEM; return -ENOMEM;
} }
/* Update source addr of the socket */
bacpy(&sco_pi(sk)->src, &hcon->src);
err = sco_chan_add(conn, sk, NULL); err = sco_chan_add(conn, sk, NULL);
if (err) if (err)
return err; return err;
lock_sock(sk);
/* Update source addr of the socket */
bacpy(&sco_pi(sk)->src, &hcon->src);
if (hcon->state == BT_CONNECTED) { if (hcon->state == BT_CONNECTED) {
sco_sock_clear_timer(sk); sco_sock_clear_timer(sk);
sk->sk_state = BT_CONNECTED; sk->sk_state = BT_CONNECTED;
...@@ -278,6 +294,13 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk) ...@@ -278,6 +294,13 @@ static int sco_connect(struct hci_dev *hdev, struct sock *sk)
sco_sock_set_timer(sk, sk->sk_sndtimeo); sco_sock_set_timer(sk, sk->sk_sndtimeo);
} }
release_sock(sk);
return err;
unlock:
hci_dev_unlock(hdev);
hci_dev_put(hdev);
return err; return err;
} }
...@@ -565,7 +588,6 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen ...@@ -565,7 +588,6 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
{ {
struct sockaddr_sco *sa = (struct sockaddr_sco *) addr; struct sockaddr_sco *sa = (struct sockaddr_sco *) addr;
struct sock *sk = sock->sk; struct sock *sk = sock->sk;
struct hci_dev *hdev;
int err; int err;
BT_DBG("sk %p", sk); BT_DBG("sk %p", sk);
...@@ -574,37 +596,26 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen ...@@ -574,37 +596,26 @@ static int sco_sock_connect(struct socket *sock, struct sockaddr *addr, int alen
addr->sa_family != AF_BLUETOOTH) addr->sa_family != AF_BLUETOOTH)
return -EINVAL; return -EINVAL;
lock_sock(sk); if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND)
if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND) { return -EBADFD;
err = -EBADFD;
goto done;
}
if (sk->sk_type != SOCK_SEQPACKET) { if (sk->sk_type != SOCK_SEQPACKET)
err = -EINVAL; err = -EINVAL;
goto done;
}
hdev = hci_get_route(&sa->sco_bdaddr, &sco_pi(sk)->src, BDADDR_BREDR);
if (!hdev) {
err = -EHOSTUNREACH;
goto done;
}
hci_dev_lock(hdev);
lock_sock(sk);
/* Set destination address and psm */ /* Set destination address and psm */
bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr); bacpy(&sco_pi(sk)->dst, &sa->sco_bdaddr);
release_sock(sk);
err = sco_connect(hdev, sk); err = sco_connect(sk);
hci_dev_unlock(hdev);
hci_dev_put(hdev);
if (err) if (err)
goto done; return err;
lock_sock(sk);
err = bt_sock_wait_state(sk, BT_CONNECTED, err = bt_sock_wait_state(sk, BT_CONNECTED,
sock_sndtimeo(sk, flags & O_NONBLOCK)); sock_sndtimeo(sk, flags & O_NONBLOCK));
done:
release_sock(sk); release_sock(sk);
return err; return err;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment