Bluetooth: hci_core: fix suspicious RCU usage in hci_conn_drop()

Commit Message

Yunseong Kim July 23, 2024, 5:17 p.m. UTC

Protection from the queuing operation is achieved with an RCU read lock
to avoid calling 'queue_delayed_work()' after 'cancel_delayed_work()',
but this does not apply to 'hci_conn_drop()'.

commit deee93d13d38 ("Bluetooth: use hdev->workqueue when queuing
 hdev->{cmd,ncmd}_timer works")

The situation described raises concerns about suspicious RCU usage in a
corrupted context.

CPU 1                   CPU 2
 hci_dev_do_reset()
  synchronize_rcu()      hci_conn_drop()
  drain_workqueue()       <-- no RCU read protection during queuing. -->
                           queue_delayed_work()

It displays a warning message like the following

Bluetooth: hci0: unexpected cc 0x0c38 length: 249 > 2
=============================
WARNING: suspicious RCU usage
6.10.0-rc6-01340-gf14c0bb78769 #5 Not tainted
-----------------------------
net/mac80211/util.c:4000 RCU-list traversed in non-reader section!!

other info that might help us debug this:

rcu_scheduler_active = 2, debug_locks = 1
2 locks held by syz-executor/798:
 #0: ffff800089a3de50 (rtnl_mutex){+.+.}-{4:4},
    at: rtnl_lock+0x28/0x40 net/core/rtnetlink.c:79

stack backtrace:
CPU: 0 PID: 798 Comm: syz-executor Not tainted
  6.10.0-rc6-01340-gf14c0bb78769 #5
Hardware name: linux,dummy-virt (DT)
Call trace:
 dump_backtrace.part.0+0x1b8/0x1d0 arch/arm64/kernel/stacktrace.c:317
 dump_backtrace arch/arm64/kernel/stacktrace.c:323 [inline]
 show_stack+0x34/0x50 arch/arm64/kernel/stacktrace.c:324
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xf0/0x170 lib/dump_stack.c:114
 dump_stack+0x20/0x30 lib/dump_stack.c:123
 lockdep_rcu_suspicious+0x204/0x2f8 kernel/locking/lockdep.c:6712
 ieee80211_check_combinations+0x71c/0x828 [mac80211]
 ieee80211_check_concurrent_iface+0x494/0x700 [mac80211]
 ieee80211_open+0x140/0x238 [mac80211]
 __dev_open+0x270/0x498 net/core/dev.c:1474
 __dev_change_flags+0x47c/0x610 net/core/dev.c:8837
 dev_change_flags+0x98/0x170 net/core/dev.c:8909
 devinet_ioctl+0xdf0/0x18d0 net/ipv4/devinet.c:1177
 inet_ioctl+0x34c/0x388 net/ipv4/af_inet.c:1003
 sock_do_ioctl+0xe4/0x240 net/socket.c:1222
 sock_ioctl+0x4cc/0x740 net/socket.c:1341
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:907 [inline]
 __se_sys_ioctl fs/ioctl.c:893 [inline]
 __arm64_sys_ioctl+0x184/0x218 fs/ioctl.c:893
 __invoke_syscall arch/arm64/kernel/syscall.c:34 [inline]
 invoke_syscall+0x90/0x2e8 arch/arm64/kernel/syscall.c:48
 el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:131
 el0_svc+0x48/0xc0 arch/arm64/kernel/entry-common.c:712
 el0t_64_sync_handler+0x120/0x130 arch/arm64/kernel/entry-common.c:730
 el0t_64_sync+0x190/0x198 arch/arm64/kernel/entry.S:598

This patch attempts to fix that issue with the same convention.

Cc: [e-mail] # v6.1+
Fixes: deee93d13d38 ("Bluetooth: use hdev->workqueue when queuing hdev->
{cmd,ncmd}_timer works")
Signed-off-by: Yeoreum Yun <[e-mail]>
Tested-by: Yunseong Kim <[e-mail]>
Signed-off-by: Yunseong Kim <[e-mail]>
---
 include/net/bluetooth/hci_core.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

[e-mail] July 23, 2024, 5:59 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=873322

---Test result---

Test Summary:
CheckPatch                    FAIL      0.91 seconds
GitLint                       PASS      0.38 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      33.00 seconds
CheckAllWarning               PASS      35.66 seconds
CheckSparse                   PASS      40.98 seconds
CheckSmatch                   PASS      103.93 seconds
BuildKernel32                 PASS      29.85 seconds
TestRunnerSetup               PASS      540.47 seconds
TestRunner_l2cap-tester       PASS      22.85 seconds
TestRunner_iso-tester         PASS      32.82 seconds
TestRunner_bnep-tester        PASS      5.43 seconds
TestRunner_mgmt-tester        FAIL      116.18 seconds
TestRunner_rfcomm-tester      PASS      7.48 seconds
TestRunner_sco-tester         PASS      15.21 seconds
TestRunner_ioctl-tester       PASS      7.88 seconds
TestRunner_mesh-tester        PASS      5.93 seconds
TestRunner_smp-tester         PASS      6.97 seconds
TestRunner_userchan-tester    PASS      5.01 seconds
IncrementalBuild              PASS      27.84 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: hci_core: fix suspicious RCU usage in hci_conn_drop()
WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 1ae534019415 ("Merge 3586166ab17668992c0cb052b6658d4eba37e05b into e0fec3b758385c70fedc3b4b1b3e34f51a1855ac")'
#169: 
Fixes: deee93d13d38 ("Bluetooth: use hdev->workqueue when queuing hdev->

total: 0 errors, 1 warnings, 15 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13740235.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
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 6 (RL is full)               Failed       0.202 seconds


---
Regards,
Linux Bluetooth

Simon Horman July 24, 2024, 5:13 p.m. UTC | #2

+ Handa-san

On Wed, Jul 24, 2024 at 02:17:57AM +0900, Yunseong Kim wrote:
> Protection from the queuing operation is achieved with an RCU read lock
> to avoid calling 'queue_delayed_work()' after 'cancel_delayed_work()',
> but this does not apply to 'hci_conn_drop()'.
> 
> commit deee93d13d38 ("Bluetooth: use hdev->workqueue when queuing
>  hdev->{cmd,ncmd}_timer works")
> 
> The situation described raises concerns about suspicious RCU usage in a
> corrupted context.
> 
> CPU 1                   CPU 2
>  hci_dev_do_reset()
>   synchronize_rcu()      hci_conn_drop()
>   drain_workqueue()       <-- no RCU read protection during queuing. -->
>                            queue_delayed_work()
> 
> It displays a warning message like the following
> 
> Bluetooth: hci0: unexpected cc 0x0c38 length: 249 > 2
> =============================
> WARNING: suspicious RCU usage
> 6.10.0-rc6-01340-gf14c0bb78769 #5 Not tainted
> -----------------------------
> net/mac80211/util.c:4000 RCU-list traversed in non-reader section!!
> 
> other info that might help us debug this:
> 
> rcu_scheduler_active = 2, debug_locks = 1
> 2 locks held by syz-executor/798:
>  #0: ffff800089a3de50 (rtnl_mutex){+.+.}-{4:4},
>     at: rtnl_lock+0x28/0x40 net/core/rtnetlink.c:79
> 
> stack backtrace:
> CPU: 0 PID: 798 Comm: syz-executor Not tainted
>   6.10.0-rc6-01340-gf14c0bb78769 #5
> Hardware name: linux,dummy-virt (DT)
> Call trace:
>  dump_backtrace.part.0+0x1b8/0x1d0 arch/arm64/kernel/stacktrace.c:317
>  dump_backtrace arch/arm64/kernel/stacktrace.c:323 [inline]
>  show_stack+0x34/0x50 arch/arm64/kernel/stacktrace.c:324
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0xf0/0x170 lib/dump_stack.c:114
>  dump_stack+0x20/0x30 lib/dump_stack.c:123
>  lockdep_rcu_suspicious+0x204/0x2f8 kernel/locking/lockdep.c:6712
>  ieee80211_check_combinations+0x71c/0x828 [mac80211]
>  ieee80211_check_concurrent_iface+0x494/0x700 [mac80211]
>  ieee80211_open+0x140/0x238 [mac80211]
>  __dev_open+0x270/0x498 net/core/dev.c:1474
>  __dev_change_flags+0x47c/0x610 net/core/dev.c:8837
>  dev_change_flags+0x98/0x170 net/core/dev.c:8909
>  devinet_ioctl+0xdf0/0x18d0 net/ipv4/devinet.c:1177
>  inet_ioctl+0x34c/0x388 net/ipv4/af_inet.c:1003
>  sock_do_ioctl+0xe4/0x240 net/socket.c:1222
>  sock_ioctl+0x4cc/0x740 net/socket.c:1341
>  vfs_ioctl fs/ioctl.c:51 [inline]
>  __do_sys_ioctl fs/ioctl.c:907 [inline]
>  __se_sys_ioctl fs/ioctl.c:893 [inline]
>  __arm64_sys_ioctl+0x184/0x218 fs/ioctl.c:893
>  __invoke_syscall arch/arm64/kernel/syscall.c:34 [inline]
>  invoke_syscall+0x90/0x2e8 arch/arm64/kernel/syscall.c:48
>  el0_svc_common.constprop.0+0x200/0x2a8 arch/arm64/kernel/syscall.c:131
>  el0_svc+0x48/0xc0 arch/arm64/kernel/entry-common.c:712
>  el0t_64_sync_handler+0x120/0x130 arch/arm64/kernel/entry-common.c:730
>  el0t_64_sync+0x190/0x198 arch/arm64/kernel/entry.S:598
> 
> This patch attempts to fix that issue with the same convention.
> 
> Cc: [e-mail] # v6.1+
> Fixes: deee93d13d38 ("Bluetooth: use hdev->workqueue when queuing hdev->
> {cmd,ncmd}_timer works")

nit: Fixes tags should not be line-wrapped.

> Signed-off-by: Yeoreum Yun <[e-mail]>
> Tested-by: Yunseong Kim <[e-mail]>
> Signed-off-by: Yunseong Kim <[e-mail]>

...

Patch

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 31020891fc68..111509dc1a23 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1572,8 +1572,13 @@  static inline void hci_conn_drop(struct hci_conn *conn)
 		}
 
 		cancel_delayed_work(&conn->disc_work);
-		queue_delayed_work(conn->hdev->workqueue,
-				   &conn->disc_work, timeo);
+
+		rcu_read_lock();
+		if (!hci_dev_test_flag(conn->hdev, HCI_CMD_DRAIN_WORKQUEUE)) {
+			queue_delayed_work(conn->hdev->workqueue,
+							   &conn->disc_work, timeo);
+		}
+		rcu_read_unlock();
 	}
 }