diff mbox series

[v3,1/2] Bluetooth: ISO: Fix circular locking dependency warning

Message ID 20240307135834.3941-2-iulia.tanasescu@nxp.com
State New
Headers show
Series Bluetooth: ISO: iso_listen_bis fixes | expand

Commit Message

Iulia Tanasescu March 7, 2024, 1:58 p.m. UTC 
This fixes the circular locking dependency warning caused
by iso_listen_bis acquiring the hdev lock while the socket
has been locked in the caller function.

======================================================
WARNING: possible circular locking dependency detected
6.8.0-rc5+ #1 Not tainted
------------------------------------------------------
iso-tester/2950 is trying to acquire lock:
ffff88817a048080 (&hdev->lock){+.+.}-{3:3}, at: iso_sock_listen+0x305/0x8d0

               but task is already holding lock:
ffff888197c39278 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0},
               at: iso_sock_listen+0x91/0x8d0

               which lock already depends on the new lock.

               the existing dependency chain (in reverse order) is:

 -> #1 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0}:
        lock_sock_nested+0x3b/0xb0
        iso_connect_cis+0x185/0x540
        iso_sock_connect+0x445/0x7e0
        __sys_connect_file+0xd5/0x100
        __sys_connect+0x11e/0x150
        __x64_sys_connect+0x42/0x60
        do_syscall_64+0x8d/0x150
        entry_SYSCALL_64_after_hwframe+0x6e/0x76

-> #0 (&hdev->lock){+.+.}-{3:3}:
        __lock_acquire+0x208f/0x3720
        lock_acquire+0x16d/0x3f0
        __mutex_lock+0x155/0x1310
        mutex_lock_nested+0x1b/0x30
        iso_sock_listen+0x305/0x8d0
        __sys_listen+0x106/0x190
        __x64_sys_listen+0x30/0x40
        do_syscall_64+0x8d/0x150
        entry_SYSCALL_64_after_hwframe+0x6e/0x76

        other info that might help us debug this:

Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(sk_lock-AF_BLUETOOTH-BTPROTO_ISO);
                                lock(&hdev->lock);
                                lock(sk_lock-AF_BLUETOOTH-BTPROTO_ISO);
   lock(&hdev->lock);

                *** DEADLOCK ***

1 lock held by iso-tester/2950:
0: ffff888197c39278 (sk_lock-AF_BLUETOOTH-BTPROTO_ISO){+.+.}-{0:0},
                at: iso_sock_listen+0x91/0x8d0

Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com>
---
 net/bluetooth/iso.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
diff mbox series

Patch

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 8af75d37b14c..d1608387fd85 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -1069,6 +1069,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;
@@ -1095,7 +1096,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)) {
@@ -1128,7 +1137,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;
 }