Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-2344-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Iulia Tanasescu <iulia.tanasescu@nxp.com>
To: linux-bluetooth@vger.kernel.org
Cc: claudia.rosu@nxp.com,
	mihai-octavian.urzica@nxp.com,
	silviu.barbulescu@nxp.com,
	vlad.pruteanu@nxp.com,
	andrei.istodorescu@nxp.com,
	luiz.dentz@gmail.com,
	Iulia Tanasescu <iulia.tanasescu@nxp.com>
Subject: [PATCH v3 1/2] Bluetooth: ISO: Fix circular locking dependency warning
Date: Thu,  7 Mar 2024 15:58:33 +0200
Message-Id: <20240307135834.3941-2-iulia.tanasescu@nxp.com>
In-Reply-To: <20240307135834.3941-1-iulia.tanasescu@nxp.com>
References: <20240307135834.3941-1-iulia.tanasescu@nxp.com>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?S22DAvdmnvVGkrr9HJsoMTBBHU+KF3kzESVW6hDNqs6q74wOnXjqOPuyV5nq?=
 =?us-ascii?Q?YglfVg3EnRw182Qcr9qfQ1SZv+EyLKhqe9tQTW9O6TC/aNqKyV1z5UI+h3Z4?=
 =?us-ascii?Q?Lg6C5ZD3MIUHQyXZJtm4aVBXp0U6ulcQyLuNtwoi2yWodlnR4Lo7RTYxFplS?=
 =?us-ascii?Q?9udLEFnMxY7oHmjc5ZuJ9++yGozwk5+k0KZCYVc2LT9E5eO5mXDE/h9qt/mc?=
 =?us-ascii?Q?WqDCTTA8GOzuEcpWlQ2LnMzUf566bhHuy191vnGHH9gJdV6BXFCfb6Y7rEvN?=
 =?us-ascii?Q?l+vu+j8hYEDaup4YNPuci+zXgppV+dnl2E3MFsSyLvtlPWonal//B/dUNve9?=
 =?us-ascii?Q?ZnbE1wRbjDlNljnGG4Cl+K3xEkwIYtP9EJEAJIiI7arWkabq88QxkA9jQL1F?=
 =?us-ascii?Q?N8VrL1rGQbtBnbn2KjIMcbFLWJwmdUejvwUFqw92LezxlpL3FynXO+tGe2yA?=
 =?us-ascii?Q?YE3SHtCpxo/oAlN+pEQpDKY9w5zobG6v6LN3iOauoQJeBMVpHXCyS5wTGYAK?=
 =?us-ascii?Q?H4DtIp/inooMjSIKUmCwx+frGRcqX8NycRNS8e7tFo1Y+9WrN8Ok0EV0GBnT?=
 =?us-ascii?Q?jd6Z0I0XcxhE0/H623gHO+4Q73ZMf6Z7ijx5tWIWsUER0g2+2pMybmqvBlrr?=
 =?us-ascii?Q?ZWUbNr2Ma3tRsWWK4/9vAjWPuLKrPe6JFqjaFdR9BEpL02Z/4Y1iOFhH9C8T?=
 =?us-ascii?Q?59WPAdraZhYm2uOYE2Ygttpen9aIG1REw46u8BhkLC2boRou1Zqw4ZvZG0mO?=
 =?us-ascii?Q?UoH6nbcJE4TyWh7kScYYPsaWUGTdDWrb+oNAMnYMqihjGttRm9lbY+jEYQSF?=
 =?us-ascii?Q?32llIJ7g5lcpmgWq3BtcQT87Ph+z9PXONJE1i2C63DyAilu8MWnpE65dUPGx?=
 =?us-ascii?Q?lWQsrfIj7d5WtfxqI90FVA/7/JPK6+zhUDVdODBaGxs33xggRPvl2p4z9oYF?=
 =?us-ascii?Q?RLObucfRt2Bo+ap7TPoczGZvNEaANYA2hG9IyDQbmP1IFA8MnJf1AI01nEh6?=
 =?us-ascii?Q?8OQ1NOKbRRr34Ad91AFwID84dKV+BwBbUkhOOoWYRAy0mNqjd3FpC1L1G10y?=
 =?us-ascii?Q?0ALHOKMl5TS6iiTjo53Si1VSunQ0oJp9qXv5h7VBpE6XOe4KM9kuUrxXTJjV?=
 =?us-ascii?Q?LHrK+wiq9z+o7xv0RhuBbf4bkKjxd39dGj4dvciNfmh8IOEwY62vWFhwMqJK?=
 =?us-ascii?Q?osmZyp3rsELXy/f1Bvh8prXvzJwl4fv9ujyMOHfKqZGc5ZXqWDjkSYwKAmNV?=
 =?us-ascii?Q?mw8Tu8lEb/tYRnGuv7aDhYtRg3hOBGOURQ4P0DMrXCKZCsUguRYOK+Vi8Odq?=
 =?us-ascii?Q?TFBB32DiwTezKYN/d98uGUbvHeT/jAjVSQzvWe1RXtFoBPVfNebjfzYIms20?=
 =?us-ascii?Q?hCfO461miuYqw6UFkK+Qve6me0P8AobBUJj0JpA1mBeGlxprhyK7h6eKL/DR?=
 =?us-ascii?Q?6hBa85GTXcCoS6EsBc2LV8z6sy/IGzTEm20s0J6+WIaVZGTXRwzQMRLMVW8G?=
 =?us-ascii?Q?gW4xlofCbbUssukogRvo+v00tD4q7SZiBgPFh4H0DKlHU6NAmeNl6tmTAL9J?=
 =?us-ascii?Q?DGVV3jtsH8aYt2qL9VuluMhVPsu4n5cuoXNkuhhO9nXMSvlyTupDBCqIy9GN?=
 =?us-ascii?Q?4A=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f46b422d-58a6-43af-2873-08dc3eaeb4e6
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB8909.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 13:58:46.0903
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: XhjukVzflwDaummlBskzEPOJW0dWhwikFFC61P6X7n/HBUauNL9Ud1Z4WoBzqJskjqnNXEXchyvSl5ggxmceyQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR04MB8424

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 --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;
 }
 
-- 
2.39.2