Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-2280-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
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: [1/2] Bluetooth: ISO: Fix circular locking dependency warning
Date: Mon,  4 Mar 2024 18:50:49 +0200
Message-Id: <20240304165050.3430-2-iulia.tanasescu@nxp.com>
In-Reply-To: <20240304165050.3430-1-iulia.tanasescu@nxp.com>
References: <20240304165050.3430-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?98imclkTF9DQfongwKayj0rxeGgw6EoPmNm0o1joLfuNZ9A0S1acCaygq+Ic?=
 =?us-ascii?Q?ENXPrx2w4q3z1lv8MYdbA5kn37l9nDFHfdiY8uN2UCRXQP67w/+NC58J7Gmf?=
 =?us-ascii?Q?xtluON/IMHe42Qg7xGTzzG5KKufe7SmTd7n6wPf3/1A28wjmI7VGeJhj7cgR?=
 =?us-ascii?Q?ihojb/AWl0XMj08LWOTRZB8aW8Pw5bBy3Pv7YMpvIsRMmRfv5VCd5aiBIvGH?=
 =?us-ascii?Q?6Ho0tB+qrEXU9Uu60/691kBAMUTg+Z1WnCOMitoq5FmmuTMc0vbPezKB+9va?=
 =?us-ascii?Q?HQNpJZ6djLXBK+ROprxBM6LibP4yME5HTEIiNhQ1EzC+ChbwAAGppCxc/Mtp?=
 =?us-ascii?Q?DkZZ0uMn2q/oyXv/tT+c4rn6Xc81uObxE/RUGX7OLPWWW5mWsEkSdY4KwSNa?=
 =?us-ascii?Q?YvUA8R4iIfPIGnX3hL8mAu4QHKG3+xnJZlj2b68ZHQpSBy3IOFcOF59rOK9b?=
 =?us-ascii?Q?DzeiyIP/IKuP+XRGfW2JJ1qfD7KQwNmxhuwyezAcJUGSCLz2NUDkgoMtiRw9?=
 =?us-ascii?Q?T2uDiL6n8Wb/Xtn9pPQDCe29iF6UrgxhYcPs4fIhblD1fmRxf67RXoviiTsW?=
 =?us-ascii?Q?0Df50KOxT9BTYQyLoNsSlfjDZRdNBjhxd3ieEfbalaWsXS/x25nIXaUkNybY?=
 =?us-ascii?Q?CUNOQWyv/dkS7Mm0lAc7Qw+g4uvxslwOFexK1KzCy1Y1DT/QwW/ngQPEUbu4?=
 =?us-ascii?Q?SpBq4LuNpOWxdwVSdbpyhS733Rdu02RjrXtMybkBGU6/1IBkFPZaInOybgEe?=
 =?us-ascii?Q?6HsvzepZdnQoM2foKNESAmwiqK4iz/6RqcSZ+qi6D/LRfYI8avU1LsM57rzN?=
 =?us-ascii?Q?RtZVYH5l7147so/WQS0yVA0XH6pBPDMu9950/N8QI0cL2or21GpGXWZ9rPeS?=
 =?us-ascii?Q?bQBsjspFyOj12rSFi/B7wrzZHAjO+0aixqD6chOoZ38O+pFET32YFd6QmoGr?=
 =?us-ascii?Q?HoH/Ie4xhZq9IWaOSGCg3Y+2cHYptJ1hL0RL0WJbQwWakmQulPl6XikQhJ0d?=
 =?us-ascii?Q?hA0Vzaf30CmaF+cmiRxDfdzWMDmBd0aWU9NW6mqJ69ZXFgzy1pM+NcGRsA7a?=
 =?us-ascii?Q?9FO2zMGHb3PxBipff+uOfXe7lv+r+Fq1P9NDYsXE0GDuHYG999izpHwGPyVu?=
 =?us-ascii?Q?UW39a4MwPpQgRD3CcKCVAxjy5zULJheXFhN9DmvmGxxXyV+Z+t+8liiP20DN?=
 =?us-ascii?Q?jszjntSMUHXynfy2C8GPOhPsjr4kg1+Om5HFLGiB9P/ZLLGWcJsuokR20EgR?=
 =?us-ascii?Q?7W1j/1d6GbBBM+1jB8TFJFIBrK62oFeeyLR2goM0FP4JuHMJG53JOhzPdyKi?=
 =?us-ascii?Q?9Pw8Ij8vuDBsxeweAaNLIPaZuBM2LKYz+Ib6IczCosbOqOnfcCBhXpmpOF0h?=
 =?us-ascii?Q?vG5S5aMudxM6te/hieSfc280uUynTbInFCmMOYQx/7ISzOZxGbdU/DFyzSVt?=
 =?us-ascii?Q?SDburrP+/xG+k+t2jggV233vIrnnkAhx3cs0z2O8jkRA7ruYDwANxxRQv8UA?=
 =?us-ascii?Q?/e/9aOuvlpIDDyqm/uj1CBNgcGp/676iBiRTnU/BLeeSlak8L592dns2WRNk?=
 =?us-ascii?Q?PjKR4R7iDwLFNvQeWkRAkqGwTMLbs0t8GifxvoWCkRCmJe8Pf5CIyKJZgIRn?=
 =?us-ascii?Q?tA=3D=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fc6230d7-5a4c-4647-47cb-08dc3c6b4901
X-MS-Exchange-CrossTenant-AuthSource: AS8PR04MB8898.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Mar 2024 16:51:06.4754
 (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: vFUXQXkfBaoE34sAdCBrcZNI2oowatmQWEDP1vqmIm6tbQktl8GkB8odbEyjGSgChuTXRGxh0xado+gtmp3/EA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM7PR04MB7064

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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 30c777c469f9..163b07db575d 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,12 @@ static int iso_listen_bis(struct sock *sk)
 	if (!hdev)
 		return -EHOSTUNREACH;
 
+	/* 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 +1134,10 @@ 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);
 	return err;
 }
 
-- 
2.39.2