| Message ID | 20240305140939.5090-2-iulia.tanasescu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bluetooth: ISO: iso_listen_bis fixes | expand |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=832552 ---Test result--- Test Summary: CheckPatch PASS 1.29 seconds GitLint PASS 0.62 seconds SubjectPrefix PASS 0.24 seconds BuildKernel PASS 28.06 seconds CheckAllWarning PASS 30.78 seconds CheckSparse PASS 36.42 seconds CheckSmatch PASS 99.56 seconds BuildKernel32 PASS 27.63 seconds TestRunnerSetup PASS 504.40 seconds TestRunner_l2cap-tester PASS 20.25 seconds TestRunner_iso-tester FAIL 37.26 seconds TestRunner_bnep-tester PASS 4.87 seconds TestRunner_mgmt-tester FAIL 110.92 seconds TestRunner_rfcomm-tester PASS 7.43 seconds TestRunner_sco-tester PASS 17.06 seconds TestRunner_ioctl-tester PASS 7.97 seconds TestRunner_mesh-tester PASS 5.97 seconds TestRunner_smp-tester PASS 7.04 seconds TestRunner_userchan-tester PASS 5.22 seconds IncrementalBuild PASS 31.97 seconds Details ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: Total: 117, Passed: 115 (98.3%), Failed: 2, Not Run: 0 Failed Test Cases ISO Connect Suspend - Success Failed 4.186 seconds ISO Connect2 Suspend - Success Failed 4.211 seconds ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2 Failed Test Cases LL Privacy - Add Device 7 (AL is full) Failed 0.198 seconds --- Regards, Linux Bluetooth
Hi, ti, 2024-03-05 kello 16:09 +0200, Iulia Tanasescu kirjoitti: > 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. I think the commit message and the comment should explain why there is no deadlock, and that the lockdep warning is a false positive. E.g. fuzzer calling connect() and listen() at the same time seems it would be dangerous here? > > ====================================================== > 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 | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > index 8af75d37b14c..5ca7bb0806b0 100644 > --- a/net/bluetooth/iso.c > +++ b/net/bluetooth/iso.c > @@ -1095,7 +1095,10 @@ static int iso_listen_bis(struct sock *sk) > if (!hdev) > return -EHOSTUNREACH; > > - hci_dev_lock(hdev); > + /* To avoid circular locking dependency warnings, > + * use nested lock for hdev. > + */ > + mutex_lock_nested(&hdev->lock, SINGLE_DEPTH_NESTING); > > /* Fail if user set invalid QoS */ > if (iso_pi(sk)->qos_user_set && !check_bcast_qos(&iso_pi(sk)->qos)) {
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index 8af75d37b14c..5ca7bb0806b0 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -1095,7 +1095,10 @@ static int iso_listen_bis(struct sock *sk) if (!hdev) return -EHOSTUNREACH; - hci_dev_lock(hdev); + /* To avoid circular locking dependency warnings, + * use nested lock for hdev. + */ + mutex_lock_nested(&hdev->lock, SINGLE_DEPTH_NESTING); /* Fail if user set invalid QoS */ if (iso_pi(sk)->qos_user_set && !check_bcast_qos(&iso_pi(sk)->qos)) {
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 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)