This fixes circular locking dependency warnings, by ensuring
the hci_dev_lock -> lock_sk order for all ISO functions.
Below is an example of a warning generated because of locking
dependencies:
[ 75.307983] ======================================================
[ 75.307984] WARNING: possible circular locking dependency detected
[ 75.307985] 6.12.0-rc6+ #22 Not tainted
[ 75.307987] ------------------------------------------------------
[ 75.307987] kworker/u81:2/2623 is trying to acquire lock:
[ 75.307988] ffff8fde1769da58 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO)
at: iso_connect_cfm+0x253/0x840 [bluetooth]
[ 75.308021]
but task is already holding lock:
[ 75.308022] ffff8fdd61a10078 (&hdev->lock)
at: hci_le_per_adv_report_evt+0x47/0x2f0 [bluetooth]
[ 75.308053]
which lock already depends on the new lock.
[ 75.308054]
the existing dependency chain (in reverse order) is:
[ 75.308055]
-> #1 (&hdev->lock){+.+.}-{3:3}:
[ 75.308057] __mutex_lock+0xad/0xc50
[ 75.308061] mutex_lock_nested+0x1b/0x30
[ 75.308063] iso_sock_listen+0x143/0x5c0 [bluetooth]
[ 75.308085] __sys_listen_socket+0x49/0x60
[ 75.308088] __x64_sys_listen+0x4c/0x90
[ 75.308090] x64_sys_call+0x2517/0x25f0
[ 75.308092] do_syscall_64+0x87/0x150
[ 75.308095] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 75.308098]
-> #0 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0}:
[ 75.308100] __lock_acquire+0x155e/0x25f0
[ 75.308103] lock_acquire+0xc9/0x300
[ 75.308105] lock_sock_nested+0x32/0x90
[ 75.308107] iso_connect_cfm+0x253/0x840 [bluetooth]
[ 75.308128] hci_connect_cfm+0x6c/0x190 [bluetooth]
[ 75.308155] hci_le_per_adv_report_evt+0x27b/0x2f0 [bluetooth]
[ 75.308180] hci_le_meta_evt+0xe7/0x200 [bluetooth]
[ 75.308206] hci_event_packet+0x21f/0x5c0 [bluetooth]
[ 75.308230] hci_rx_work+0x3ae/0xb10 [bluetooth]
[ 75.308254] process_one_work+0x212/0x740
[ 75.308256] worker_thread+0x1bd/0x3a0
[ 75.308258] kthread+0xe4/0x120
[ 75.308259] ret_from_fork+0x44/0x70
[ 75.308261] ret_from_fork_asm+0x1a/0x30
[ 75.308263]
other info that might help us debug this:
[ 75.308264] Possible unsafe locking scenario:
[ 75.308264] CPU0 CPU1
[ 75.308265] ---- ----
[ 75.308265] lock(&hdev->lock);
[ 75.308267] lock(sk_lock-
AF_BLUETOOTH-BTPROTO_ISO);
[ 75.308268] lock(&hdev->lock);
[ 75.308269] lock(sk_lock-AF_BLUETOOTH-BTPROTO_ISO);
[ 75.308270]
*** DEADLOCK ***
[ 75.308271] 4 locks held by kworker/u81:2/2623:
[ 75.308272] #0: ffff8fdd66e52148 ((wq_completion)hci0#2){+.+.}-{0:0},
at: process_one_work+0x443/0x740
[ 75.308276] #1: ffffafb488b7fe48 ((work_completion)(&hdev->rx_work)),
at: process_one_work+0x1ce/0x740
[ 75.308280] #2: ffff8fdd61a10078 (&hdev->lock){+.+.}-{3:3}
at: hci_le_per_adv_report_evt+0x47/0x2f0 [bluetooth]
[ 75.308304] #3: ffffffffb6ba4900 (rcu_read_lock){....}-{1:2},
at: hci_connect_cfm+0x29/0x190 [bluetooth]
Signed-off-by: Iulia Tanasescu <iulia.tanasescu@xxxxxxx>
---
net/bluetooth/iso.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 8ed818254dc8..cb004b678d65 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1102,6 +1102,7 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr *addr,
return err;
}
+/* This function requires the caller to hold sk lock */
static int iso_listen_bis(struct sock *sk)
{
struct hci_dev *hdev;
@@ -1128,7 +1129,15 @@ static int iso_listen_bis(struct sock *sk)
if (!hdev)
return -EHOSTUNREACH;
+ /* Prevent sk from being freed whilst unlocked */
+ sock_hold(sk);
+
+ /* To avoid circular locking dependencies,
+ * hdev should be locked first before sk.
+ */
+ release_sock(sk);
hci_dev_lock(hdev);
+ lock_sock(sk);
/* Fail if user set invalid QoS */
if (iso_pi(sk)->qos_user_set && !check_bcast_qos(&iso_pi(sk)->qos)) {
@@ -1161,7 +1170,13 @@ static int iso_listen_bis(struct sock *sk)
hci_dev_put(hdev);
unlock:
+ /* Unlock order should be in reverse from lock order. */
+ release_sock(sk);
hci_dev_unlock(hdev);
+ lock_sock(sk);
+
+ sock_put(sk);
+
return err;
}
@@ -1417,6 +1432,7 @@ static void iso_conn_defer_accept(struct hci_conn *conn)
hci_send_cmd(hdev, HCI_OP_LE_ACCEPT_CIS, sizeof(cp), &cp);
}
+/* This function requires the caller to hold sk lock */
static void iso_conn_big_sync(struct sock *sk)
{
int err;
@@ -1428,6 +1444,14 @@ static void iso_conn_big_sync(struct sock *sk)
if (!hdev)
return;
+ /* Prevent sk from being freed whilst unlocked */
+ sock_hold(sk);
+
+ /* To avoid circular locking dependencies, hdev should be
+ * locked first before sk.
+ */
+ release_sock(sk);
+
/* hci_le_big_create_sync requires hdev lock to be held, since
* it enqueues the HCI LE BIG Create Sync command via
* hci_cmd_sync_queue_once, which checks hdev flags that might
@@ -1435,6 +1459,8 @@ static void iso_conn_big_sync(struct sock *sk)
*/
hci_dev_lock(hdev);
+ lock_sock(sk);
+
if (!test_and_set_bit(BT_SK_BIG_SYNC, &iso_pi(sk)->flags)) {
err = hci_le_big_create_sync(hdev, iso_pi(sk)->conn->hcon,
&iso_pi(sk)->qos,
@@ -1446,7 +1472,12 @@ static void iso_conn_big_sync(struct sock *sk)
err);
}
+ /* Unlock order should be in reverse from lock order. */
+ release_sock(sk);
hci_dev_unlock(hdev);
+ lock_sock(sk);
+
+ sock_put(sk);
}
static int iso_sock_recvmsg(struct socket *sock, struct msghdr *msg,
--
2.40.1
