Bluetooth: Fix hci_link_tx_to RCU lock usage

Commit Message

Ying Hsu Sept. 4, 2023, 2:11 p.m. UTC

Syzbot found a bug "BUG: sleeping function called from invalid context
at kernel/locking/mutex.c:580". It is because hci_link_tx_to holds an
RCU read lock and calls hci_disconnect which would hold a mutex lock
since the commit a13f316e90fd ("Bluetooth: hci_conn: Consolidate code
for aborting connections"). Here's an example call trace:

   __dump_stack lib/dump_stack.c:88 [inline]
   dump_stack_lvl+0xfc/0x174 lib/dump_stack.c:106
   ___might_sleep+0x4a9/0x4d3 kernel/sched/core.c:9663
   __mutex_lock_common kernel/locking/mutex.c:576 [inline]
   __mutex_lock+0xc7/0x6e7 kernel/locking/mutex.c:732
   hci_cmd_sync_queue+0x3a/0x287 net/bluetooth/hci_sync.c:388
   hci_abort_conn+0x2cd/0x2e4 net/bluetooth/hci_conn.c:1812
   hci_disconnect+0x207/0x237 net/bluetooth/hci_conn.c:244
   hci_link_tx_to net/bluetooth/hci_core.c:3254 [inline]
   __check_timeout net/bluetooth/hci_core.c:3419 [inline]
   __check_timeout+0x310/0x361 net/bluetooth/hci_core.c:3399
   hci_sched_le net/bluetooth/hci_core.c:3602 [inline]
   hci_tx_work+0xe8f/0x12d0 net/bluetooth/hci_core.c:3652
   process_one_work+0x75c/0xba1 kernel/workqueue.c:2310
   worker_thread+0x5b2/0x73a kernel/workqueue.c:2457
   kthread+0x2f7/0x30b kernel/kthread.c:319
   ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298

This patch releases RCU read lock before calling hci_disconnect and
reacquires it afterward to fix the bug.

Fixes: a13f316e90fd ("Bluetooth: hci_conn: Consolidate code for aborting connections")
Signed-off-by: Ying Hsu <[e-mail]>
---
Tested this commit using a C reproducer on qemu-x86_64 for 10 minutes.

 net/bluetooth/hci_core.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

[e-mail] Sept. 4, 2023, 3:07 p.m. UTC | #1

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=781528

---Test result---

Test Summary:
CheckPatch                    PASS      0.63 seconds
GitLint                       PASS      0.25 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      43.81 seconds
CheckAllWarning               PASS      47.96 seconds
CheckSparse                   PASS      53.57 seconds
CheckSmatch                   PASS      144.09 seconds
BuildKernel32                 PASS      42.36 seconds
TestRunnerSetup               PASS      638.67 seconds
TestRunner_l2cap-tester       PASS      36.81 seconds
TestRunner_iso-tester         PASS      72.23 seconds
TestRunner_bnep-tester        PASS      13.51 seconds
TestRunner_mgmt-tester        PASS      263.46 seconds
TestRunner_rfcomm-tester      PASS      23.73 seconds
TestRunner_sco-tester         PASS      29.06 seconds
TestRunner_ioctl-tester       PASS      25.21 seconds
TestRunner_mesh-tester        PASS      19.68 seconds
TestRunner_smp-tester         PASS      19.87 seconds
TestRunner_userchan-tester    PASS      15.92 seconds
IncrementalBuild              PASS      39.59 seconds



---
Regards,
Linux Bluetooth

[e-mail] Sept. 5, 2023, 6:01 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Mon,  4 Sep 2023 14:11:51 +0000 you wrote:
> Syzbot found a bug "BUG: sleeping function called from invalid context
> at kernel/locking/mutex.c:580". It is because hci_link_tx_to holds an
> RCU read lock and calls hci_disconnect which would hold a mutex lock
> since the commit a13f316e90fd ("Bluetooth: hci_conn: Consolidate code
> for aborting connections"). Here's an example call trace:
> 
>    __dump_stack lib/dump_stack.c:88 [inline]
>    dump_stack_lvl+0xfc/0x174 lib/dump_stack.c:106
>    ___might_sleep+0x4a9/0x4d3 kernel/sched/core.c:9663
>    __mutex_lock_common kernel/locking/mutex.c:576 [inline]
>    __mutex_lock+0xc7/0x6e7 kernel/locking/mutex.c:732
>    hci_cmd_sync_queue+0x3a/0x287 net/bluetooth/hci_sync.c:388
>    hci_abort_conn+0x2cd/0x2e4 net/bluetooth/hci_conn.c:1812
>    hci_disconnect+0x207/0x237 net/bluetooth/hci_conn.c:244
>    hci_link_tx_to net/bluetooth/hci_core.c:3254 [inline]
>    __check_timeout net/bluetooth/hci_core.c:3419 [inline]
>    __check_timeout+0x310/0x361 net/bluetooth/hci_core.c:3399
>    hci_sched_le net/bluetooth/hci_core.c:3602 [inline]
>    hci_tx_work+0xe8f/0x12d0 net/bluetooth/hci_core.c:3652
>    process_one_work+0x75c/0xba1 kernel/workqueue.c:2310
>    worker_thread+0x5b2/0x73a kernel/workqueue.c:2457
>    kthread+0x2f7/0x30b kernel/kthread.c:319
>    ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:298
> 
> [...]

Here is the summary with links:
  - Bluetooth: Fix hci_link_tx_to RCU lock usage
    https://git.kernel.org/bluetooth/bluetooth-next/c/d606d5f4024d

You are awesome, thank you!

Patch

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a5992f1b3c9b..db4f28d68d71 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3418,7 +3418,12 @@  static void hci_link_tx_to(struct hci_dev *hdev, __u8 type)
 		if (c->type == type && c->sent) {
 			bt_dev_err(hdev, "killing stalled connection %pMR",
 				   &c->dst);
+			/* hci_disconnect might sleep, so, we have to release
+			 * the RCU read lock before calling it.
+			 */
+			rcu_read_unlock();
 			hci_disconnect(c, HCI_ERROR_REMOTE_USER_TERM);
+			rcu_read_lock();
 		}
 	}