Hi,
On Fri, Apr 26, 2024 at 7:10 AM Marek Szyprowski
<m.szyprowski@xxxxxxxxxxx> wrote:
>
> Dear All,
>
> On 25.04.2024 06:11, Sungwoo Kim wrote:
> > Hello, could you review the UAF bug and its fix?
> > The stack trace is at the bottom.
> >
> > mgmt sync cmd could be used after freed in this scenario:
> >
> > set_local_name() ... cmd is allocated, set_name_complete() is
> > queued in cmd_sync_work.
> > hci_error_reset() ... hci device reset.
> > hci_dev_close_sync() ... close hdev, at this point, cmd is freed.
> > set_name_complete() ... callback from cmd_sync_work. cmd->param causes UAF.
> >
> > To fix this, this patch makes hci_dev_close_sync() call hci_cmd_sync_clear() to clear the cmd_sync_work.
> >
> > Thanks,
> > Sungwoo Kim.
>
> This patch landed in today's linux-next as commit 37dd04e4d594
> ("Bluetooth: HCI: fix slab-use-after-free in cmd_sync_work"). I believe
> it correctly fixes the mentioned problem, but on the other hand it
> introduces the following kernel's lock dependency checker warning:
>
> ============================================
> WARNING: possible recursive locking detected
> 6.9.0-rc4-01047-g37dd04e4d594 #14949 Not tainted
> --------------------------------------------
> kworker/u9:0/56 is trying to acquire lock:
> c3aba78c ((work_completion)(&hdev->cmd_sync_work)){+.+.}-{0:0}, at:
> __flush_work+0x1e0/0x538
>
> but task is already holding lock:
> f0eb5f18 ((work_completion)(&hdev->cmd_sync_work)){+.+.}-{0:0}, at:
> process_scheduled_works+0x328/0x7a8
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock((work_completion)(&hdev->cmd_sync_work));
> lock((work_completion)(&hdev->cmd_sync_work));
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation
>
> 4 locks held by kworker/u9:0/56:
> #0: c3a4e8b4 ((wq_completion)hci0){+.+.}-{0:0}, at:
> process_scheduled_works+0x2fc/0x7a8
> #1: f0eb5f18 ((work_completion)(&hdev->cmd_sync_work)){+.+.}-{0:0},
> at: process_scheduled_works+0x328/0x7a8
> #2: c3abab6c (&hdev->req_lock){+.+.}-{3:3}, at:
> hci_cmd_sync_work+0xa0/0x154 [bluetooth]
> #3: c137bdfc (rcu_read_lock){....}-{1:2}, at: __flush_work+0x40/0x538
>
> stack backtrace:
> CPU: 0 PID: 56 Comm: kworker/u9:0 Not tainted
> 6.9.0-rc4-01047-g37dd04e4d594 #14949
> Hardware name: Samsung Exynos (Flattened Device Tree)
> Workqueue: hci0 hci_cmd_sync_work [bluetooth]
> Call trace:.
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x68/0x88
> dump_stack_lvl from __lock_acquire+0x152c/0x1744
> __lock_acquire from lock_acquire+0x21c/0x394
> lock_acquire from __flush_work+0x20c/0x538
> __flush_work from __cancel_work_sync+0x12c/0x20c
> __cancel_work_sync from hci_cmd_sync_clear+0x1c/0x6c [bluetooth]
> hci_cmd_sync_clear [bluetooth] from hci_dev_close_sync+0x31c/0x5f4
> [bluetooth]
> hci_dev_close_sync [bluetooth] from hci_set_powered_sync+0x27c/0x288
> [bluetooth]
> hci_set_powered_sync [bluetooth] from hci_cmd_sync_work+0xb0/0x154
> [bluetooth]
> hci_cmd_sync_work [bluetooth] from process_scheduled_works+0x390/0x7a8
> process_scheduled_works from worker_thread+0x150/0x3bc
> worker_thread from kthread+0x108/0x140
> kthread from ret_from_fork+0x14/0x28
> Exception stack(0xf0eb5fb0 to 0xf0eb5ff8)
> ...
>
>
> Please check if the locking is really correct and if so, add the needed
> lockdep annotation to the bluetooth code to silence the above warning.
>
>
> > ==================================================================
> > BUG: KASAN: slab-use-after-free in set_name_complete+0x4a/0x330 net/bluetooth/mgmt.c:3815
> > Read of size 8 at addr ffff888107259098 by task kworker/u3:0/66
> >
> > CPU: 0 PID: 66 Comm: kworker/u3:0 Not tainted 6.8.0+ #61
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > Workqueue: hci0 hci_cmd_sync_work
> > Call Trace:
> > <TASK>
> > __dump_stack lib/dump_stack.c:88 [inline]
> > dump_stack_lvl+0x85/0xb0 lib/dump_stack.c:106
> > print_address_description mm/kasan/report.c:377 [inline]
> > print_report+0x18f/0x560 mm/kasan/report.c:488
> > kasan_report+0xd7/0x110 mm/kasan/report.c:601
> > __asan_report_load8_noabort+0x18/0x20 mm/kasan/report_generic.c:381
> > set_name_complete+0x4a/0x330 net/bluetooth/mgmt.c:3815
> > hci_cmd_sync_work+0x269/0x3e0 net/bluetooth/hci_sync.c:308
> > process_one_work kernel/workqueue.c:2633 [inline]
> > process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
> > worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
> > kthread+0x2a9/0x340 kernel/kthread.c:388
> > ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
> > ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> > </TASK>
> >
> > Allocated by task 308:
> > kasan_save_stack mm/kasan/common.c:47 [inline]
> > kasan_save_track+0x30/0x70 mm/kasan/common.c:68
> > kasan_save_alloc_info+0x3c/0x50 mm/kasan/generic.c:575
> > poison_kmalloc_redzone mm/kasan/common.c:370 [inline]
> > __kasan_kmalloc+0xa2/0xc0 mm/kasan/common.c:387
> > kasan_kmalloc include/linux/kasan.h:211 [inline]
> > kmalloc_trace+0x1c9/0x390 mm/slub.c:4012
> > kmalloc include/linux/slab.h:590 [inline]
> > kzalloc include/linux/slab.h:711 [inline]
> > mgmt_pending_new+0x6f/0x230 net/bluetooth/mgmt_util.c:269
> > mgmt_pending_add+0x3f/0x120 net/bluetooth/mgmt_util.c:296
> > set_local_name+0x15a/0x4c0 net/bluetooth/mgmt.c:3892
> > hci_mgmt_cmd+0xb79/0x1190 net/bluetooth/hci_sock.c:1715
> > hci_sock_sendmsg+0x63a/0xf00 net/bluetooth/hci_sock.c:1835
> > sock_sendmsg_nosec net/socket.c:730 [inline]
> > __sock_sendmsg+0x227/0x270 net/socket.c:745
> > sock_write_iter+0x28d/0x3d0 net/socket.c:1160
> > do_iter_readv_writev+0x331/0x4c0
> > vfs_writev+0x2e6/0xa40 fs/read_write.c:971
> > do_writev+0xfd/0x250 fs/read_write.c:1018
> > __do_sys_writev fs/read_write.c:1091 [inline]
> > __se_sys_writev fs/read_write.c:1088 [inline]
> > __x64_sys_writev+0x86/0xa0 fs/read_write.c:1088
> > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > do_syscall_64+0x84/0x120 arch/x86/entry/common.c:83
> > entry_SYSCALL_64_after_hwframe+0x6e/0x76
> >
> > Freed by task 66:
> > kasan_save_stack mm/kasan/common.c:47 [inline]
> > kasan_save_track+0x30/0x70 mm/kasan/common.c:68
> > kasan_save_free_info+0x44/0x50 mm/kasan/generic.c:589
> > poison_slab_object+0x11a/0x190 mm/kasan/common.c:240
> > __kasan_slab_free+0x3b/0x60 mm/kasan/common.c:256
> > kasan_slab_free include/linux/kasan.h:184 [inline]
> > slab_free_hook mm/slub.c:2121 [inline]
> > slab_free mm/slub.c:4299 [inline]
> > kfree+0x106/0x2e0 mm/slub.c:4409
> > mgmt_pending_free net/bluetooth/mgmt_util.c:309 [inline]
> > mgmt_pending_remove+0x19e/0x1d0 net/bluetooth/mgmt_util.c:315
> > cmd_complete_rsp+0x104/0x1a0
> > mgmt_pending_foreach+0xc7/0x120 net/bluetooth/mgmt_util.c:259
> > __mgmt_power_off+0x137/0x370 net/bluetooth/mgmt.c:9496
> > hci_dev_close_sync+0x4ab/0xe80 net/bluetooth/hci_sync.c:4953
> > hci_dev_do_close net/bluetooth/hci_core.c:554 [inline]
> > hci_error_reset+0x150/0x410 net/bluetooth/hci_core.c:1060
> > process_one_work kernel/workqueue.c:2633 [inline]
> > process_scheduled_works+0x6b9/0xdc0 kernel/workqueue.c:2706
> > worker_thread+0xb2b/0x13d0 kernel/workqueue.c:2787
> > kthread+0x2a9/0x340 kernel/kthread.c:388
> > ret_from_fork+0x5c/0x90 arch/x86/kernel/process.c:147
> > ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
> >
> > The buggy address belongs to the object at ffff888107259080
> > which belongs to the cache kmalloc-96 of size 96
> > The buggy address is located 24 bytes inside of
> > freed 96-byte region [ffff888107259080, ffff8881072590e0)
> >
> > The buggy address belongs to the physical page:
> > page:000000006bdb81a5 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888107259280 pfn:0x107259
> > flags: 0x17ffffc0000a00(workingset|slab|node=0|zone=2|lastcpupid=0x1fffff)
> > page_type: 0xffffffff()
> > raw: 0017ffffc0000a00 ffff888100041780 ffffea0004145510 ffffea0004240190
> > raw: ffff888107259280 000000000020000f 00000001ffffffff 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff888107258f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff888107259000: 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc fc
> >> ffff888107259080: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ^
> > ffff888107259100: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> > ffff888107259180: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
> > ==================================================================
> >
> > Signed-off-by: Sungwoo Kim <iam@xxxxxxxxxxxx>
> > ---
> > net/bluetooth/hci_core.c | 2 --
> > net/bluetooth/hci_sync.c | 5 ++++-
> > 2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index a7028d38c..c347efc4f 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -2764,8 +2764,6 @@ void hci_unregister_dev(struct hci_dev *hdev)
> >
> > cancel_work_sync(&hdev->power_on);
> >
> > - hci_cmd_sync_clear(hdev);
> > -
> > hci_unregister_suspend_notifier(hdev);
> >
> > msft_unregister(hdev);
> > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > index c5d879904..aa8e0c33c 100644
> > --- a/net/bluetooth/hci_sync.c
> > +++ b/net/bluetooth/hci_sync.c
> > @@ -5181,9 +5181,12 @@ int hci_dev_close_sync(struct hci_dev *hdev)
> > clear_bit(HCI_INIT, &hdev->flags);
> > }
> >
> > - /* flush cmd work */
> > + /* flush cmd work */
> > flush_work(&hdev->cmd_work);
> >
> > + /* flush cmd sync work */
> > + hci_cmd_sync_clear(hdev);
> > +
> > /* Drop queues */
> > skb_queue_purge(&hdev->rx_q);
> > skb_queue_purge(&hdev->cmd_q);
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Yeah, I will be reverting it from bluetooth-next until we find a
solution that doesn't cause side effects.
--
Luiz Augusto von Dentz
