* [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt
@ 2024-04-07 11:33 syzbot
2024-04-07 13:59 ` syzbot
0 siblings, 1 reply; 10+ messages in thread
From: syzbot @ 2024-04-07 11:33 UTC (permalink / raw)
To: johan.hedberg, linux-bluetooth, linux-kernel, luiz.dentz, marcel,
syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 480e035fc4c7 Merge tag 'drm-next-2024-03-13' of https://gi..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=12154da9180000
kernel config: https://syzkaller.appspot.com/x/.config?x=1e5b814e91787669
dashboard link: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14136223180000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=11ee2e75180000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/5f73b6ef963d/disk-480e035f.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/46c949396aad/vmlinux-480e035f.xz
kernel image: https://storage.googleapis.com/syzbot-assets/e3b4d0f5a5f8/bzImage-480e035f.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
4 locks held by kworker/u9:2/5070:
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
#2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
__might_resched+0x5d4/0x780 kernel/sched/core.c:10187
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
</TASK>
=============================
[ BUG: Invalid wait context ]
6.8.0-syzkaller-08073-g480e035fc4c7 #0 Tainted: G W
-----------------------------
kworker/u9:2/5070 is trying to lock:
ffffffff8f4f5aa8 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
ffffffff8f4f5aa8 (hci_cb_list_lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
other info that might help us debug this:
context-{4:4}
4 locks held by kworker/u9:2/5070:
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
#2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
stack backtrace:
CPU: 0 PID: 5070 Comm: kworker/u9:2 Tainted: G W 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_lock_invalid_wait_context kernel/locking/lockdep.c:4751 [inline]
check_wait_context kernel/locking/lockdep.c:4821 [inline]
__lock_acquire+0x1507/0x1fd0 kernel/locking/lockdep.c:5087
lock_acquire+0x1e4/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:608 [inline]
__mutex_lock+0x136/0xd70 kernel/locking/mutex.c:752
hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
</TASK>
==================================================================
BUG: KASAN: slab-use-after-free in hci_le_create_big_complete_evt+0x383/0xae0
Read of size 8 at addr ffff88807cb1c000 by task kworker/u9:2/5070
CPU: 0 PID: 5070 Comm: kworker/u9:2 Tainted: G W 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
print_address_description mm/kasan/report.c:377 [inline]
print_report+0x169/0x550 mm/kasan/report.c:488
kasan_report+0x143/0x180 mm/kasan/report.c:601
hci_le_create_big_complete_evt+0x383/0xae0
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
</TASK>
Allocated by task 5070:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
__kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:387
kasan_kmalloc include/linux/kasan.h:211 [inline]
kmalloc_trace+0x1db/0x360 mm/slub.c:3997
kmalloc include/linux/slab.h:628 [inline]
kzalloc include/linux/slab.h:749 [inline]
hci_conn_add+0xc7/0x13a0 net/bluetooth/hci_conn.c:914
hci_le_big_sync_established_evt+0x1cf/0xb90 net/bluetooth/hci_event.c:6980
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
Freed by task 5070:
kasan_save_stack mm/kasan/common.c:47 [inline]
kasan_save_track+0x3f/0x80 mm/kasan/common.c:68
kasan_save_free_info+0x40/0x50 mm/kasan/generic.c:579
poison_slab_object+0xa6/0xe0 mm/kasan/common.c:240
__kasan_slab_free+0x37/0x60 mm/kasan/common.c:256
kasan_slab_free include/linux/kasan.h:184 [inline]
slab_free_hook mm/slub.c:2106 [inline]
slab_free mm/slub.c:4280 [inline]
kfree+0x14a/0x380 mm/slub.c:4390
device_release+0x99/0x1c0
kobject_cleanup lib/kobject.c:689 [inline]
kobject_release lib/kobject.c:720 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x22f/0x480 lib/kobject.c:737
hci_conn_cleanup net/bluetooth/hci_conn.c:176 [inline]
hci_conn_del+0x900/0xc80 net/bluetooth/hci_conn.c:1126
hci_le_create_big_complete_evt+0x619/0xae0 net/bluetooth/hci_event.c:6941
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
The buggy address belongs to the object at ffff88807cb1c000
which belongs to the cache kmalloc-8k of size 8192
The buggy address is located 0 bytes inside of
freed 8192-byte region [ffff88807cb1c000, ffff88807cb1e000)
The buggy address belongs to the physical page:
page:ffffea0001f2c600 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x7cb18
head:ffffea0001f2c600 order:3 entire_mapcount:0 nr_pages_mapped:0 pincount:0
ksm flags: 0xfff00000000840(slab|head|node=0|zone=1|lastcpupid=0x7ff)
page_type: 0xffffffff()
raw: 00fff00000000840 ffff888014c42280 ffffea0001f2cc00 dead000000000003
raw: 0000000000000000 0000000080020002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 3, migratetype Unmovable, gfp_mask 0xd2040(__GFP_IO|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 4524, tgid 4524 (S10udev), ts 19996984719, free_ts 17455903844
set_page_owner include/linux/page_owner.h:31 [inline]
post_alloc_hook+0x1ea/0x210 mm/page_alloc.c:1533
prep_new_page mm/page_alloc.c:1540 [inline]
get_page_from_freelist+0x33ea/0x3580 mm/page_alloc.c:3311
__alloc_pages+0x256/0x680 mm/page_alloc.c:4569
__alloc_pages_node include/linux/gfp.h:238 [inline]
alloc_pages_node include/linux/gfp.h:261 [inline]
alloc_slab_page+0x5f/0x160 mm/slub.c:2175
allocate_slab mm/slub.c:2338 [inline]
new_slab+0x84/0x2f0 mm/slub.c:2391
___slab_alloc+0xc73/0x1260 mm/slub.c:3525
__slab_alloc mm/slub.c:3610 [inline]
__slab_alloc_node mm/slub.c:3663 [inline]
slab_alloc_node mm/slub.c:3835 [inline]
kmalloc_trace+0x269/0x360 mm/slub.c:3992
kmalloc include/linux/slab.h:628 [inline]
kzalloc include/linux/slab.h:749 [inline]
tomoyo_print_bprm security/tomoyo/audit.c:26 [inline]
tomoyo_init_log+0x11ce/0x2050 security/tomoyo/audit.c:264
tomoyo_supervisor+0x38a/0x11f0 security/tomoyo/common.c:2089
tomoyo_audit_env_log security/tomoyo/environ.c:36 [inline]
tomoyo_env_perm+0x178/0x210 security/tomoyo/environ.c:63
tomoyo_environ security/tomoyo/domain.c:672 [inline]
tomoyo_find_next_domain+0x1384/0x1cf0 security/tomoyo/domain.c:878
tomoyo_bprm_check_security+0x115/0x180 security/tomoyo/tomoyo.c:102
security_bprm_check+0x65/0x90 security/security.c:1185
search_binary_handler fs/exec.c:1765 [inline]
exec_binprm fs/exec.c:1819 [inline]
bprm_execve+0xa56/0x1790 fs/exec.c:1871
do_execveat_common+0x553/0x700 fs/exec.c:1978
do_execve fs/exec.c:2052 [inline]
__do_sys_execve fs/exec.c:2128 [inline]
__se_sys_execve fs/exec.c:2123 [inline]
__x64_sys_execve+0x92/0xb0 fs/exec.c:2123
page last free pid 1 tgid 1 stack trace:
reset_page_owner include/linux/page_owner.h:24 [inline]
free_pages_prepare mm/page_alloc.c:1140 [inline]
free_unref_page_prepare+0x968/0xa90 mm/page_alloc.c:2346
free_unref_page+0x37/0x3f0 mm/page_alloc.c:2486
free_contig_range+0x9e/0x160 mm/page_alloc.c:6547
destroy_args+0x8a/0x890 mm/debug_vm_pgtable.c:1036
debug_vm_pgtable+0x4be/0x550 mm/debug_vm_pgtable.c:1416
do_one_initcall+0x238/0x830 init/main.c:1241
do_initcall_level+0x157/0x210 init/main.c:1303
do_initcalls+0x3f/0x80 init/main.c:1319
kernel_init_freeable+0x435/0x5d0 init/main.c:1550
kernel_init+0x1d/0x2a0 init/main.c:1439
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
Memory state around the buggy address:
ffff88807cb1bf00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88807cb1bf80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88807cb1c000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88807cb1c080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88807cb1c100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt
2024-04-07 11:33 syzbot
@ 2024-04-07 13:59 ` syzbot
0 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2024-04-07 13:59 UTC (permalink / raw)
To: davem, edumazet, hdanton, iulia.tanasescu, johan.hedberg, kuba,
linux-bluetooth, linux-kernel, luiz.dentz, luiz.von.dentz, marcel,
netdev, pabeni, syzkaller-bugs
syzbot has bisected this issue to:
commit a0bfde167b506423111ddb8cd71930497a40fc54
Author: Iulia Tanasescu <iulia.tanasescu@nxp.com>
Date: Tue May 30 14:21:59 2023 +0000
Bluetooth: ISO: Add support for connecting multiple BISes
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=146c679d180000
start commit: 8568bb2ccc27 Add linux-next specific files for 20240405
git tree: linux-next
final oops: https://syzkaller.appspot.com/x/report.txt?x=166c679d180000
console output: https://syzkaller.appspot.com/x/log.txt?x=126c679d180000
kernel config: https://syzkaller.appspot.com/x/.config?x=48ca5acf8d2eb3bc
dashboard link: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1338efc5180000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15428f4b180000
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Fixes: a0bfde167b50 ("Bluetooth: ISO: Add support for connecting multiple BISes")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
@ 2024-12-03 21:14 Luiz Augusto von Dentz
2024-12-03 21:56 ` [v1] " bluez.test.bot
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2024-12-03 21:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: syzbot+2fb0835e0c9cefc34614, Luiz Augusto von Dentz
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
like the bellow:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
4 locks held by kworker/u9:2/5070:
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
#2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
__might_resched+0x5d4/0x780 kernel/sched/core.c:10187
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
</TASK>
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
net/bluetooth/hci_core.c | 9 ++--
2 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ea798f07c5a2..95f11f04e24a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -804,7 +804,6 @@ struct hci_conn_params {
extern struct list_head hci_dev_list;
extern struct list_head hci_cb_list;
extern rwlock_t hci_dev_list_lock;
-extern struct mutex hci_cb_list_lock;
#define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
#define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
@@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
- list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->connect_cfm)
- cb->connect_cfm(conn, status);
+ rcu_read_lock();
+ list_for_each_entry_rcu(cb, &hci_cb_list, list) {
+ if (cb->connect_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.connect_cfm(conn, status);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->connect_cfm_cb)
conn->connect_cfm_cb(conn, status);
@@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->disconn_cfm)
- cb->disconn_cfm(conn, reason);
+ if (cb->disconn_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.disconn_cfm(conn, reason);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->disconn_cfm_cb)
conn->disconn_cfm_cb(conn, reason);
@@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->security_cfm)
- cb->security_cfm(conn, status, encrypt);
+ if (cb->security_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.security_cfm(conn, status, encrypt);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->security_cfm_cb)
conn->security_cfm_cb(conn, status);
@@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
conn->sec_level = conn->pending_sec_level;
}
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->security_cfm)
- cb->security_cfm(conn, status, encrypt);
+ if (cb->security_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.security_cfm(conn, status, encrypt);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->security_cfm_cb)
conn->security_cfm_cb(conn, status);
@@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->key_change_cfm)
- cb->key_change_cfm(conn, status);
+ if (cb->key_change_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.key_change_cfm(conn, status);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
}
static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
@@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
- list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->role_switch_cfm)
- cb->role_switch_cfm(conn, status, role);
+ rcu_read_lock();
+ list_for_each_entry_rcu(cb, &hci_cb_list, list) {
+ if (cb->role_switch_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.role_switch_cfm(conn, status, role);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
}
static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f9e19f9cb5a3..25d180d225c1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);
- mutex_lock(&hci_cb_list_lock);
- list_add_tail(&cb->list, &hci_cb_list);
- mutex_unlock(&hci_cb_list_lock);
+ list_add_tail_rcu(&cb->list, &hci_cb_list);
return 0;
}
@@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);
- mutex_lock(&hci_cb_list_lock);
- list_del(&cb->list);
- mutex_unlock(&hci_cb_list_lock);
+ list_del_rcu(&cb->list);
+ synchronize_rcu();
return 0;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* RE: [v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
2024-12-03 21:14 [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Luiz Augusto von Dentz
@ 2024-12-03 21:56 ` bluez.test.bot
2024-12-03 22:29 ` [PATCH v1] " Luiz Augusto von Dentz
2024-12-03 22:48 ` [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Pauli Virtanen
2 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2024-12-03 21:56 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
[-- Attachment #1: Type: text/plain, Size: 2114 bytes --]
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=914334
---Test result---
Test Summary:
CheckPatch PENDING 0.35 seconds
GitLint PENDING 0.35 seconds
SubjectPrefix PASS 0.12 seconds
BuildKernel PASS 26.01 seconds
CheckAllWarning PASS 30.01 seconds
CheckSparse WARNING 32.40 seconds
BuildKernel32 PASS 26.08 seconds
TestRunnerSetup PASS 449.76 seconds
TestRunner_l2cap-tester PASS 22.51 seconds
TestRunner_iso-tester FAIL 31.42 seconds
TestRunner_bnep-tester PASS 4.85 seconds
TestRunner_mgmt-tester PASS 123.67 seconds
TestRunner_rfcomm-tester PASS 7.77 seconds
TestRunner_sco-tester PASS 9.63 seconds
TestRunner_ioctl-tester PASS 8.36 seconds
TestRunner_mesh-tester PASS 6.26 seconds
TestRunner_smp-tester PASS 7.24 seconds
TestRunner_userchan-tester PASS 5.16 seconds
IncrementalBuild PENDING 0.45 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_core.c:60:1: warning: symbol 'hci_cb_list_lock' was not declared. Should it be static?
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
Total: 125, Passed: 121 (96.8%), Failed: 0, Not Run: 4
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
2024-12-03 21:14 [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Luiz Augusto von Dentz
2024-12-03 21:56 ` [v1] " bluez.test.bot
@ 2024-12-03 22:29 ` Luiz Augusto von Dentz
2024-12-04 2:04 ` [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt syzbot
2024-12-03 22:48 ` [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Pauli Virtanen
2 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2024-12-03 22:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: syzbot+2fb0835e0c9cefc34614, Luiz Augusto von Dentz
[-- Attachment #1: Type: text/plain, Size: 10242 bytes --]
#syz test
On Tue, Dec 3, 2024 at 4:15 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
> like the bellow:
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
> preempt_count: 0, expected: 0
> RCU nest depth: 1, expected: 0
> 4 locks held by kworker/u9:2/5070:
> #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
> #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
> #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
> #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
> #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
> CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> Workqueue: hci0 hci_rx_work
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> __might_resched+0x5d4/0x780 kernel/sched/core.c:10187
> __mutex_lock_common kernel/locking/mutex.c:585 [inline]
> __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
> hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
> hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
> hci_event_func net/bluetooth/hci_event.c:7514 [inline]
> hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
> hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
> process_one_work kernel/workqueue.c:3254 [inline]
> process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
> worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
> kthread+0x2f0/0x390 kernel/kthread.c:388
> ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
> </TASK>
>
> Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
> net/bluetooth/hci_core.c | 9 ++--
> 2 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ea798f07c5a2..95f11f04e24a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -804,7 +804,6 @@ struct hci_conn_params {
> extern struct list_head hci_dev_list;
> extern struct list_head hci_cb_list;
> extern rwlock_t hci_dev_list_lock;
> -extern struct mutex hci_cb_list_lock;
>
> #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
> #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
> @@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->connect_cfm)
> - cb->connect_cfm(conn, status);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->connect_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.connect_cfm(conn, status);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->connect_cfm_cb)
> conn->connect_cfm_cb(conn, status);
> @@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->disconn_cfm)
> - cb->disconn_cfm(conn, reason);
> + if (cb->disconn_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.disconn_cfm(conn, reason);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->disconn_cfm_cb)
> conn->disconn_cfm_cb(conn, reason);
> @@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
>
> encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
> conn->sec_level = conn->pending_sec_level;
> }
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->key_change_cfm)
> - cb->key_change_cfm(conn, status);
> + if (cb->key_change_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.key_change_cfm(conn, status);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> @@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->role_switch_cfm)
> - cb->role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->role_switch_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index f9e19f9cb5a3..25d180d225c1 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
> {
> BT_DBG("%p name %s", cb, cb->name);
>
> - mutex_lock(&hci_cb_list_lock);
> - list_add_tail(&cb->list, &hci_cb_list);
> - mutex_unlock(&hci_cb_list_lock);
> + list_add_tail_rcu(&cb->list, &hci_cb_list);
>
> return 0;
> }
> @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
> {
> BT_DBG("%p name %s", cb, cb->name);
>
> - mutex_lock(&hci_cb_list_lock);
> - list_del(&cb->list);
> - mutex_unlock(&hci_cb_list_lock);
> + list_del_rcu(&cb->list);
> + synchronize_rcu();
>
> return 0;
> }
> --
> 2.47.1
>
--
Luiz Augusto von Dentz
[-- Attachment #2: v1-0001-Bluetooth-hci_core-Fix-sleeping-function-called-f.patch --]
[-- Type: text/x-patch, Size: 8262 bytes --]
From b5b5693a20c51d28753d1fb2e696c018ade4e61a Mon Sep 17 00:00:00 2001
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Date: Tue, 3 Dec 2024 16:07:32 -0500
Subject: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from
invalid context
This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
like the bellow:
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
preempt_count: 0, expected: 0
RCU nest depth: 1, expected: 0
4 locks held by kworker/u9:2/5070:
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
#0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
#1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
#2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
#3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
Workqueue: hci0 hci_rx_work
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
__might_resched+0x5d4/0x780 kernel/sched/core.c:10187
__mutex_lock_common kernel/locking/mutex.c:585 [inline]
__mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
hci_event_func net/bluetooth/hci_event.c:7514 [inline]
hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
process_one_work kernel/workqueue.c:3254 [inline]
process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
kthread+0x2f0/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
</TASK>
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
net/bluetooth/hci_core.c | 9 ++--
2 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ea798f07c5a2..95f11f04e24a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -804,7 +804,6 @@ struct hci_conn_params {
extern struct list_head hci_dev_list;
extern struct list_head hci_cb_list;
extern rwlock_t hci_dev_list_lock;
-extern struct mutex hci_cb_list_lock;
#define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
#define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
@@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
- list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->connect_cfm)
- cb->connect_cfm(conn, status);
+ rcu_read_lock();
+ list_for_each_entry_rcu(cb, &hci_cb_list, list) {
+ if (cb->connect_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.connect_cfm(conn, status);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->connect_cfm_cb)
conn->connect_cfm_cb(conn, status);
@@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->disconn_cfm)
- cb->disconn_cfm(conn, reason);
+ if (cb->disconn_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.disconn_cfm(conn, reason);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->disconn_cfm_cb)
conn->disconn_cfm_cb(conn, reason);
@@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->security_cfm)
- cb->security_cfm(conn, status, encrypt);
+ if (cb->security_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.security_cfm(conn, status, encrypt);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->security_cfm_cb)
conn->security_cfm_cb(conn, status);
@@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
conn->sec_level = conn->pending_sec_level;
}
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->security_cfm)
- cb->security_cfm(conn, status, encrypt);
+ if (cb->security_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.security_cfm(conn, status, encrypt);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
if (conn->security_cfm_cb)
conn->security_cfm_cb(conn, status);
@@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
+ rcu_read_lock();
list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->key_change_cfm)
- cb->key_change_cfm(conn, status);
+ if (cb->key_change_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.key_change_cfm(conn, status);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
}
static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
@@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
{
struct hci_cb *cb;
- mutex_lock(&hci_cb_list_lock);
- list_for_each_entry(cb, &hci_cb_list, list) {
- if (cb->role_switch_cfm)
- cb->role_switch_cfm(conn, status, role);
+ rcu_read_lock();
+ list_for_each_entry_rcu(cb, &hci_cb_list, list) {
+ if (cb->role_switch_cfm) {
+ struct hci_cb cpy = *cb;
+
+ /* Callback may block so release RCU read lock */
+ rcu_read_unlock();
+ cpy.role_switch_cfm(conn, status, role);
+ rcu_read_lock();
+ }
}
- mutex_unlock(&hci_cb_list_lock);
+ rcu_read_unlock();
}
static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f9e19f9cb5a3..25d180d225c1 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);
- mutex_lock(&hci_cb_list_lock);
- list_add_tail(&cb->list, &hci_cb_list);
- mutex_unlock(&hci_cb_list_lock);
+ list_add_tail_rcu(&cb->list, &hci_cb_list);
return 0;
}
@@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);
- mutex_lock(&hci_cb_list_lock);
- list_del(&cb->list);
- mutex_unlock(&hci_cb_list_lock);
+ list_del_rcu(&cb->list);
+ synchronize_rcu();
return 0;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
2024-12-03 21:14 [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Luiz Augusto von Dentz
2024-12-03 21:56 ` [v1] " bluez.test.bot
2024-12-03 22:29 ` [PATCH v1] " Luiz Augusto von Dentz
@ 2024-12-03 22:48 ` Pauli Virtanen
2024-12-04 4:31 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 10+ messages in thread
From: Pauli Virtanen @ 2024-12-03 22:48 UTC (permalink / raw)
To: Luiz Augusto von Dentz, linux-bluetooth
Hi,
ti, 2024-12-03 kello 16:14 -0500, Luiz Augusto von Dentz kirjoitti:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
> like the bellow:
>
> BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
> in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
> preempt_count: 0, expected: 0
> RCU nest depth: 1, expected: 0
> 4 locks held by kworker/u9:2/5070:
> #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
> #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
> #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
> #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
> #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
> #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
> CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> Workqueue: hci0 hci_rx_work
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> __might_resched+0x5d4/0x780 kernel/sched/core.c:10187
> __mutex_lock_common kernel/locking/mutex.c:585 [inline]
> __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
> hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
> hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
> hci_event_func net/bluetooth/hci_event.c:7514 [inline]
> hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
> hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
> process_one_work kernel/workqueue.c:3254 [inline]
> process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
> worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
> kthread+0x2f0/0x390 kernel/kthread.c:388
> ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
> </TASK>
>
> Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
> net/bluetooth/hci_core.c | 9 ++--
> 2 files changed, 65 insertions(+), 33 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index ea798f07c5a2..95f11f04e24a 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -804,7 +804,6 @@ struct hci_conn_params {
> extern struct list_head hci_dev_list;
> extern struct list_head hci_cb_list;
> extern rwlock_t hci_dev_list_lock;
> -extern struct mutex hci_cb_list_lock;
>
> #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
> #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
> @@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->connect_cfm)
> - cb->connect_cfm(conn, status);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->connect_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.connect_cfm(conn, status);
> + rcu_read_lock();
This looks like incorrect RCU usage
[CPU 1] rcu_read_unlock()
[CPU 2] hci_unregister_cb(cb)
[CPU 2] hci_unregister_cb(next cb)
[CPU 1] rcu_read_lock()
[CPU 1] list_for_each_entry_rcu -> iterates to "next cb" not in list
If all hci_cb weren't static, it'd also UAF (maybe it is for rfcomm?).
hci_le_create_big_complete_evt() also does this (and maybe crashes if
ev->status != 0 ?), so maybe it is simples to fix it.
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->connect_cfm_cb)
> conn->connect_cfm_cb(conn, status);
> @@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->disconn_cfm)
> - cb->disconn_cfm(conn, reason);
> + if (cb->disconn_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.disconn_cfm(conn, reason);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->disconn_cfm_cb)
> conn->disconn_cfm_cb(conn, reason);
> @@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
>
> encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
> conn->sec_level = conn->pending_sec_level;
> }
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->security_cfm)
> - cb->security_cfm(conn, status, encrypt);
> + if (cb->security_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.security_cfm(conn, status, encrypt);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
>
> if (conn->security_cfm_cb)
> conn->security_cfm_cb(conn, status);
> @@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> + rcu_read_lock();
> list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->key_change_cfm)
> - cb->key_change_cfm(conn, status);
> + if (cb->key_change_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.key_change_cfm(conn, status);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> @@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> {
> struct hci_cb *cb;
>
> - mutex_lock(&hci_cb_list_lock);
> - list_for_each_entry(cb, &hci_cb_list, list) {
> - if (cb->role_switch_cfm)
> - cb->role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> + if (cb->role_switch_cfm) {
> + struct hci_cb cpy = *cb;
> +
> + /* Callback may block so release RCU read lock */
> + rcu_read_unlock();
> + cpy.role_switch_cfm(conn, status, role);
> + rcu_read_lock();
> + }
> }
> - mutex_unlock(&hci_cb_list_lock);
> + rcu_read_unlock();
> }
>
> static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index f9e19f9cb5a3..25d180d225c1 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
> {
> BT_DBG("%p name %s", cb, cb->name);
>
> - mutex_lock(&hci_cb_list_lock);
> - list_add_tail(&cb->list, &hci_cb_list);
> - mutex_unlock(&hci_cb_list_lock);
> + list_add_tail_rcu(&cb->list, &hci_cb_list);
>
> return 0;
> }
> @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
> {
> BT_DBG("%p name %s", cb, cb->name);
>
> - mutex_lock(&hci_cb_list_lock);
> - list_del(&cb->list);
> - mutex_unlock(&hci_cb_list_lock);
> + list_del_rcu(&cb->list);
> + synchronize_rcu();
>
> return 0;
> }
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt
2024-12-03 22:29 ` [PATCH v1] " Luiz Augusto von Dentz
@ 2024-12-04 2:04 ` syzbot
0 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2024-12-04 2:04 UTC (permalink / raw)
To: linux-bluetooth, linux-kernel, luiz.dentz, luiz.von.dentz,
syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested on:
commit: feffde68 Merge tag 'for-6.13-rc1-tag' of git://git.ker..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10e708df980000
kernel config: https://syzkaller.appspot.com/x/.config?x=773e6b07384b3087
dashboard link: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=100c0de8580000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
2024-12-03 22:48 ` [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Pauli Virtanen
@ 2024-12-04 4:31 ` Luiz Augusto von Dentz
2024-12-04 17:01 ` Iulia Tanasescu
0 siblings, 1 reply; 10+ messages in thread
From: Luiz Augusto von Dentz @ 2024-12-04 4:31 UTC (permalink / raw)
To: Pauli Virtanen; +Cc: linux-bluetooth
Hi Pauli,
On Tue, Dec 3, 2024 at 5:48 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> ti, 2024-12-03 kello 16:14 -0500, Luiz Augusto von Dentz kirjoitti:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
> > like the bellow:
> >
> > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
> > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
> > preempt_count: 0, expected: 0
> > RCU nest depth: 1, expected: 0
> > 4 locks held by kworker/u9:2/5070:
> > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
> > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
> > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
> > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
> > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
> > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
> > Workqueue: hci0 hci_rx_work
> > Call Trace:
> > <TASK>
> > __dump_stack lib/dump_stack.c:88 [inline]
> > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
> > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187
> > __mutex_lock_common kernel/locking/mutex.c:585 [inline]
> > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
> > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
> > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
> > hci_event_func net/bluetooth/hci_event.c:7514 [inline]
> > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
> > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
> > process_one_work kernel/workqueue.c:3254 [inline]
> > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
> > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
> > kthread+0x2f0/0x390 kernel/kthread.c:388
> > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
> > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
> > </TASK>
> >
> > Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> > ---
> > include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
> > net/bluetooth/hci_core.c | 9 ++--
> > 2 files changed, 65 insertions(+), 33 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index ea798f07c5a2..95f11f04e24a 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -804,7 +804,6 @@ struct hci_conn_params {
> > extern struct list_head hci_dev_list;
> > extern struct list_head hci_cb_list;
> > extern rwlock_t hci_dev_list_lock;
> > -extern struct mutex hci_cb_list_lock;
> >
> > #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
> > #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
> > @@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
> > {
> > struct hci_cb *cb;
> >
> > - mutex_lock(&hci_cb_list_lock);
> > - list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->connect_cfm)
> > - cb->connect_cfm(conn, status);
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> > + if (cb->connect_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.connect_cfm(conn, status);
> > + rcu_read_lock();
>
> This looks like incorrect RCU usage
>
> [CPU 1] rcu_read_unlock()
> [CPU 2] hci_unregister_cb(cb)
> [CPU 2] hci_unregister_cb(next cb)
> [CPU 1] rcu_read_lock()
> [CPU 1] list_for_each_entry_rcu -> iterates to "next cb" not in list
>
> If all hci_cb weren't static, it'd also UAF (maybe it is for rfcomm?).
>
>
> hci_le_create_big_complete_evt() also does this (and maybe crashes if
> ev->status != 0 ?), so maybe it is simples to fix it.
I guess you are right, it seems incorrect to have
rcu_read_unlock/relock inside list_for_each_entry_rcu, that said I
wonder why the entry is not accessed via rcu_dereference, anyway the
only alternative I can think of is to copy to a local stack list which
we can then run with list_for_each_entry_safe, or we create some
mechanism to differ actions that otherwise could block/sleep while
holding rcu_read_lock.
>
>
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> >
> > if (conn->connect_cfm_cb)
> > conn->connect_cfm_cb(conn, status);
> > @@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
> > {
> > struct hci_cb *cb;
> >
> > - mutex_lock(&hci_cb_list_lock);
> > + rcu_read_lock();
> > list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->disconn_cfm)
> > - cb->disconn_cfm(conn, reason);
> > + if (cb->disconn_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.disconn_cfm(conn, reason);
> > + rcu_read_lock();
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> >
> > if (conn->disconn_cfm_cb)
> > conn->disconn_cfm_cb(conn, reason);
> > @@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
> >
> > encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
> >
> > - mutex_lock(&hci_cb_list_lock);
> > + rcu_read_lock();
> > list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->security_cfm)
> > - cb->security_cfm(conn, status, encrypt);
> > + if (cb->security_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.security_cfm(conn, status, encrypt);
> > + rcu_read_lock();
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> >
> > if (conn->security_cfm_cb)
> > conn->security_cfm_cb(conn, status);
> > @@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
> > conn->sec_level = conn->pending_sec_level;
> > }
> >
> > - mutex_lock(&hci_cb_list_lock);
> > + rcu_read_lock();
> > list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->security_cfm)
> > - cb->security_cfm(conn, status, encrypt);
> > + if (cb->security_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.security_cfm(conn, status, encrypt);
> > + rcu_read_lock();
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> >
> > if (conn->security_cfm_cb)
> > conn->security_cfm_cb(conn, status);
> > @@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
> > {
> > struct hci_cb *cb;
> >
> > - mutex_lock(&hci_cb_list_lock);
> > + rcu_read_lock();
> > list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->key_change_cfm)
> > - cb->key_change_cfm(conn, status);
> > + if (cb->key_change_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.key_change_cfm(conn, status);
> > + rcu_read_lock();
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> > }
> >
> > static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> > @@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
> > {
> > struct hci_cb *cb;
> >
> > - mutex_lock(&hci_cb_list_lock);
> > - list_for_each_entry(cb, &hci_cb_list, list) {
> > - if (cb->role_switch_cfm)
> > - cb->role_switch_cfm(conn, status, role);
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
> > + if (cb->role_switch_cfm) {
> > + struct hci_cb cpy = *cb;
> > +
> > + /* Callback may block so release RCU read lock */
> > + rcu_read_unlock();
> > + cpy.role_switch_cfm(conn, status, role);
> > + rcu_read_lock();
> > + }
> > }
> > - mutex_unlock(&hci_cb_list_lock);
> > + rcu_read_unlock();
> > }
> >
> > static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index f9e19f9cb5a3..25d180d225c1 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
> > {
> > BT_DBG("%p name %s", cb, cb->name);
> >
> > - mutex_lock(&hci_cb_list_lock);
> > - list_add_tail(&cb->list, &hci_cb_list);
> > - mutex_unlock(&hci_cb_list_lock);
> > + list_add_tail_rcu(&cb->list, &hci_cb_list);
> >
> > return 0;
> > }
> > @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
> > {
> > BT_DBG("%p name %s", cb, cb->name);
> >
> > - mutex_lock(&hci_cb_list_lock);
> > - list_del(&cb->list);
> > - mutex_unlock(&hci_cb_list_lock);
> > + list_del_rcu(&cb->list);
> > + synchronize_rcu();
> >
> > return 0;
> > }
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
2024-12-04 4:31 ` Luiz Augusto von Dentz
@ 2024-12-04 17:01 ` Iulia Tanasescu
0 siblings, 0 replies; 10+ messages in thread
From: Iulia Tanasescu @ 2024-12-04 17:01 UTC (permalink / raw)
To: luiz.dentz; +Cc: linux-bluetooth, pav
Hi,
>From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
>To: Pauli Virtanen <pav@iki.fi>
>Cc: linux-bluetooth@vger.kernel.org
>Subject: Re: [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context
>Date: Tue, 3 Dec 2024 23:31:37 -0500 [thread overview]
>Message-ID: <CABBYNZ+KcZ13SF2yksP3su2kY4sSCJgiF_o0OfrXTomFigmKbQ@mail.gmail.com> (raw)
>In-Reply-To: <0b897a445022f99bb812c811135fdbc8bf73bbba.camel@iki.fi>
>
>Hi Pauli,
>
>On Tue, Dec 3, 2024 at 5:48 PM Pauli Virtanen <pav@iki.fi> wrote:
>>
>> Hi,
>>
>> ti, 2024-12-03 kello 16:14 -0500, Luiz Augusto von Dentz kirjoitti:
>> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> >
>> > This reworks hci_cb_list to not use mutex hci_cb_list_lock to avoid bugs
>> > like the bellow:
>> >
>> > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:585
>> > in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 5070, name: kworker/u9:2
>> > preempt_count: 0, expected: 0
>> > RCU nest depth: 1, expected: 0
>> > 4 locks held by kworker/u9:2/5070:
>> > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3229 [inline]
>> > #0: ffff888015be3948 ((wq_completion)hci0#2){+.+.}-{0:0}, at: process_scheduled_works+0x8e0/0x1770 kernel/workqueue.c:3335
>> > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_one_work kernel/workqueue.c:3230 [inline]
>> > #1: ffffc90003b6fd00 ((work_completion)(&hdev->rx_work)){+.+.}-{0:0}, at: process_scheduled_works+0x91b/0x1770 kernel/workqueue.c:3335
>> > #2: ffff8880665d0078 (&hdev->lock){+.+.}-{3:3}, at: hci_le_create_big_complete_evt+0xcf/0xae0 net/bluetooth/hci_event.c:6914
>> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_lock_acquire include/linux/rcupdate.h:298 [inline]
>> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: rcu_read_lock include/linux/rcupdate.h:750 [inline]
>> > #3: ffffffff8e132020 (rcu_read_lock){....}-{1:2}, at: hci_le_create_big_complete_evt+0xdb/0xae0 net/bluetooth/hci_event.c:6915
>> > CPU: 0 PID: 5070 Comm: kworker/u9:2 Not tainted 6.8.0-syzkaller-08073-g480e035fc4c7 #0
>> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/27/2024
>> > Workqueue: hci0 hci_rx_work
>> > Call Trace:
>> > <TASK>
>> > __dump_stack lib/dump_stack.c:88 [inline]
>> > dump_stack_lvl+0x241/0x360 lib/dump_stack.c:114
>> > __might_resched+0x5d4/0x780 kernel/sched/core.c:10187
>> > __mutex_lock_common kernel/locking/mutex.c:585 [inline]
>> > __mutex_lock+0xc1/0xd70 kernel/locking/mutex.c:752
>> > hci_connect_cfm include/net/bluetooth/hci_core.h:2004 [inline]
>> > hci_le_create_big_complete_evt+0x3d9/0xae0 net/bluetooth/hci_event.c:6939
>> > hci_event_func net/bluetooth/hci_event.c:7514 [inline]
>> > hci_event_packet+0xa53/0x1540 net/bluetooth/hci_event.c:7569
>> > hci_rx_work+0x3e8/0xca0 net/bluetooth/hci_core.c:4171
>> > process_one_work kernel/workqueue.c:3254 [inline]
>> > process_scheduled_works+0xa00/0x1770 kernel/workqueue.c:3335
>> > worker_thread+0x86d/0xd70 kernel/workqueue.c:3416
>> > kthread+0x2f0/0x390 kernel/kthread.c:388
>> > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>> > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
>> > </TASK>
>> >
>> > Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
>> > Closes: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
>> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>> > ---
>> > include/net/bluetooth/hci_core.h | 89 ++++++++++++++++++++++----------
>> > net/bluetooth/hci_core.c | 9 ++--
>> > 2 files changed, 65 insertions(+), 33 deletions(-)
>> >
>> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> > index ea798f07c5a2..95f11f04e24a 100644
>> > --- a/include/net/bluetooth/hci_core.h
>> > +++ b/include/net/bluetooth/hci_core.h
>> > @@ -804,7 +804,6 @@ struct hci_conn_params {
>> > extern struct list_head hci_dev_list;
>> > extern struct list_head hci_cb_list;
>> > extern rwlock_t hci_dev_list_lock;
>> > -extern struct mutex hci_cb_list_lock;
>> >
>> > #define hci_dev_set_flag(hdev, nr) set_bit((nr), (hdev)->dev_flags)
>> > #define hci_dev_clear_flag(hdev, nr) clear_bit((nr), (hdev)->dev_flags)
>> > @@ -2029,12 +2028,18 @@ static inline void hci_connect_cfm(struct hci_conn *conn, __u8 status)
>> > {
>> > struct hci_cb *cb;
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > - list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->connect_cfm)
>> > - cb->connect_cfm(conn, status);
>> > + rcu_read_lock();
>> > + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
>> > + if (cb->connect_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.connect_cfm(conn, status);
>> > + rcu_read_lock();
>>
>> This looks like incorrect RCU usage
>>
>> [CPU 1] rcu_read_unlock()
>> [CPU 2] hci_unregister_cb(cb)
>> [CPU 2] hci_unregister_cb(next cb)
>> [CPU 1] rcu_read_lock()
>> [CPU 1] list_for_each_entry_rcu -> iterates to "next cb" not in list
>>
>> If all hci_cb weren't static, it'd also UAF (maybe it is for rfcomm?).
>>
>>
>> hci_le_create_big_complete_evt() also does this (and maybe crashes if
>> ev->status != 0 ?), so maybe it is simples to fix it.
>
>I guess you are right, it seems incorrect to have
>rcu_read_unlock/relock inside list_for_each_entry_rcu, that said I
>wonder why the entry is not accessed via rcu_dereference, anyway the
>only alternative I can think of is to copy to a local stack list which
>we can then run with list_for_each_entry_safe, or we create some
>mechanism to differ actions that otherwise could block/sleep while
>holding rcu_read_lock.
>
I submitted a patch to fix this for hci_le_create_big_complete_evt:
https://patchwork.kernel.org/project/bluetooth/cover/20241204164840.14037-1-iulia.tanasescu@nxp.com/
>>
>>
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> >
>> > if (conn->connect_cfm_cb)
>> > conn->connect_cfm_cb(conn, status);
>> > @@ -2044,12 +2049,18 @@ static inline void hci_disconn_cfm(struct hci_conn *conn, __u8 reason)
>> > {
>> > struct hci_cb *cb;
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > + rcu_read_lock();
>> > list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->disconn_cfm)
>> > - cb->disconn_cfm(conn, reason);
>> > + if (cb->disconn_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.disconn_cfm(conn, reason);
>> > + rcu_read_lock();
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> >
>> > if (conn->disconn_cfm_cb)
>> > conn->disconn_cfm_cb(conn, reason);
>> > @@ -2065,12 +2076,18 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)
>> >
>> > encrypt = test_bit(HCI_CONN_ENCRYPT, &conn->flags) ? 0x01 : 0x00;
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > + rcu_read_lock();
>> > list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->security_cfm)
>> > - cb->security_cfm(conn, status, encrypt);
>> > + if (cb->security_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.security_cfm(conn, status, encrypt);
>> > + rcu_read_lock();
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> >
>> > if (conn->security_cfm_cb)
>> > conn->security_cfm_cb(conn, status);
>> > @@ -2105,12 +2122,18 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status)
>> > conn->sec_level = conn->pending_sec_level;
>> > }
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > + rcu_read_lock();
>> > list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->security_cfm)
>> > - cb->security_cfm(conn, status, encrypt);
>> > + if (cb->security_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.security_cfm(conn, status, encrypt);
>> > + rcu_read_lock();
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> >
>> > if (conn->security_cfm_cb)
>> > conn->security_cfm_cb(conn, status);
>> > @@ -2120,12 +2143,18 @@ static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
>> > {
>> > struct hci_cb *cb;
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > + rcu_read_lock();
>> > list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->key_change_cfm)
>> > - cb->key_change_cfm(conn, status);
>> > + if (cb->key_change_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.key_change_cfm(conn, status);
>> > + rcu_read_lock();
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> > }
>> >
>> > static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
>> > @@ -2133,12 +2162,18 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
>> > {
>> > struct hci_cb *cb;
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > - list_for_each_entry(cb, &hci_cb_list, list) {
>> > - if (cb->role_switch_cfm)
>> > - cb->role_switch_cfm(conn, status, role);
>> > + rcu_read_lock();
>> > + list_for_each_entry_rcu(cb, &hci_cb_list, list) {
>> > + if (cb->role_switch_cfm) {
>> > + struct hci_cb cpy = *cb;
>> > +
>> > + /* Callback may block so release RCU read lock */
>> > + rcu_read_unlock();
>> > + cpy.role_switch_cfm(conn, status, role);
>> > + rcu_read_lock();
>> > + }
>> > }
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + rcu_read_unlock();
>> > }
>> >
>> > static inline bool hci_bdaddr_is_rpa(bdaddr_t *bdaddr, u8 addr_type)
>> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> > index f9e19f9cb5a3..25d180d225c1 100644
>> > --- a/net/bluetooth/hci_core.c
>> > +++ b/net/bluetooth/hci_core.c
>> > @@ -2993,9 +2993,7 @@ int hci_register_cb(struct hci_cb *cb)
>> > {
>> > BT_DBG("%p name %s", cb, cb->name);
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > - list_add_tail(&cb->list, &hci_cb_list);
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + list_add_tail_rcu(&cb->list, &hci_cb_list);
>> >
>> > return 0;
>> > }
>> > @@ -3005,9 +3003,8 @@ int hci_unregister_cb(struct hci_cb *cb)
>> > {
>> > BT_DBG("%p name %s", cb, cb->name);
>> >
>> > - mutex_lock(&hci_cb_list_lock);
>> > - list_del(&cb->list);
>> > - mutex_unlock(&hci_cb_list_lock);
>> > + list_del_rcu(&cb->list);
>> > + synchronize_rcu();
>> >
>> > return 0;
>> > }
>>
>
>
>--
>Luiz Augusto von Dentz
Regards,
Iulia
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt
2024-12-05 15:12 [PATCH v2] " Luiz Augusto von Dentz
@ 2024-12-05 15:35 ` syzbot
0 siblings, 0 replies; 10+ messages in thread
From: syzbot @ 2024-12-05 15:35 UTC (permalink / raw)
To: linux-bluetooth, linux-kernel, luiz.dentz, syzkaller-bugs
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested-by: syzbot+2fb0835e0c9cefc34614@syzkaller.appspotmail.com
Tested on:
commit: feffde68 Merge tag 'for-6.13-rc1-tag' of git://git.ker..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=144e1330580000
kernel config: https://syzkaller.appspot.com/x/.config?x=773e6b07384b3087
dashboard link: https://syzkaller.appspot.com/bug?extid=2fb0835e0c9cefc34614
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=178490f8580000
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-12-05 15:35 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-03 21:14 [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Luiz Augusto von Dentz
2024-12-03 21:56 ` [v1] " bluez.test.bot
2024-12-03 22:29 ` [PATCH v1] " Luiz Augusto von Dentz
2024-12-04 2:04 ` [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt syzbot
2024-12-03 22:48 ` [PATCH v1] Bluetooth: hci_core: Fix sleeping function called from invalid context Pauli Virtanen
2024-12-04 4:31 ` Luiz Augusto von Dentz
2024-12-04 17:01 ` Iulia Tanasescu
-- strict thread matches above, loose matches on Subject: below --
2024-12-05 15:12 [PATCH v2] " Luiz Augusto von Dentz
2024-12-05 15:35 ` [syzbot] [bluetooth?] BUG: sleeping function called from invalid context in hci_le_create_big_complete_evt syzbot
2024-04-07 11:33 syzbot
2024-04-07 13:59 ` syzbot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).