| Message ID | 20241204122849.9000-2-iulia.tanasescu@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | Bluetooth: iso: Fix warnings | 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=914536 ---Test result--- Test Summary: CheckPatch PENDING 0.31 seconds GitLint PENDING 0.22 seconds SubjectPrefix PASS 0.30 seconds BuildKernel PASS 25.32 seconds CheckAllWarning PASS 27.56 seconds CheckSparse PASS 30.86 seconds BuildKernel32 PASS 24.37 seconds TestRunnerSetup PASS 439.51 seconds TestRunner_l2cap-tester PASS 20.67 seconds TestRunner_iso-tester FAIL 35.49 seconds TestRunner_bnep-tester PASS 6.83 seconds TestRunner_mgmt-tester FAIL 119.58 seconds TestRunner_rfcomm-tester PASS 7.58 seconds TestRunner_sco-tester PASS 9.45 seconds TestRunner_ioctl-tester PASS 8.13 seconds TestRunner_mesh-tester PASS 6.09 seconds TestRunner_smp-tester PASS 7.06 seconds TestRunner_userchan-tester PASS 5.03 seconds IncrementalBuild PENDING 0.47 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: Total: 125, Passed: 120 (96.0%), Failed: 1, Not Run: 4 Failed Test Cases ISO Connect2 Suspend - Success Failed 4.242 seconds ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 486 (98.8%), Failed: 2, Not Run: 4 Failed Test Cases LL Privacy - Start Discovery 2 (Disable RL) Failed 0.186 seconds LL Privacy - Set Device Flag 1 (Device Privacy) Failed 0.147 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
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,
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@nxp.com> --- net/bluetooth/iso.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)