diff mbox series

Bluetooth: HCI: fix slab-use-after-free in cmd_sync_work

Message ID 20240425041128.3093970-1-iam@sung-woo.kim (mailing list archive)
State Accepted
Commit 37dd04e4d59487c928bf3561e4bd3a045762eabc
Headers show
Series Bluetooth: HCI: fix slab-use-after-free in cmd_sync_work | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #88: set_name_complete() ... callback from cmd_sync_work. cmd->param causes UAF. total: 0 errors, 1 warnings, 0 checks, 21 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/13642862.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.
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 14: B1 Line exceeds max length (104>80): "To fix this, this patch makes hci_dev_close_sync() call hci_cmd_sync_clear() to clear the cmd_sync_work." 20: B1 Line exceeds max length (89>80): "BUG: KASAN: slab-use-after-free in set_name_complete+0x4a/0x330 net/bluetooth/mgmt.c:3815" 103: B1 Line exceeds max length (106>80): "page:000000006bdb81a5 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff888107259280 pfn:0x107259"
tedd_an/SubjectPrefix success Gitlint PASS
tedd_an/BuildKernel success BuildKernel PASS
tedd_an/CheckAllWarning success CheckAllWarning PASS
tedd_an/CheckSparse success CheckSparse PASS
tedd_an/CheckSmatch fail CheckSparse: FAIL: Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139 make[4]: *** Deleting file 'net/bluetooth/hci_core.o' make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: net] Error 2 make[2]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o' make[4]: *** Waiting for unfinished jobs.... make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: drivers] Error 2 make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2 make: *** [Makefile:240: __sub-make] Error 2
tedd_an/BuildKernel32 success BuildKernel32 PASS
tedd_an/TestRunnerSetup success TestRunnerSetup PASS
tedd_an/TestRunner_l2cap-tester success TestRunner PASS
tedd_an/TestRunner_iso-tester fail TestRunner_iso-tester: Total: 122, Passed: 121 (99.2%), Failed: 1, Not Run: 0
tedd_an/TestRunner_bnep-tester success TestRunner PASS

Commit Message

Sungwoo Kim April 25, 2024, 4:11 a.m. UTC
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.

==================================================================
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@sung-woo.kim>
---
 net/bluetooth/hci_core.c | 2 --
 net/bluetooth/hci_sync.c | 5 ++++-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

patchwork-bot+bluetooth@kernel.org April 25, 2024, 4:30 p.m. UTC | #1
Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu, 25 Apr 2024 00:11:28 -0400 you 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.
> 
> [...]

Here is the summary with links:
  - Bluetooth: HCI: fix slab-use-after-free in cmd_sync_work
    https://git.kernel.org/bluetooth/bluetooth-next/c/37dd04e4d594

You are awesome, thank you!
Marek Szyprowski April 26, 2024, 11:10 a.m. UTC | #2
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@sung-woo.kim>
> ---
>   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
Luiz Augusto von Dentz April 30, 2024, 5 p.m. UTC | #3
Hi,

On Fri, Apr 26, 2024 at 7:10 AM Marek Szyprowski
<m.szyprowski@samsung.com> 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@sung-woo.kim>
> > ---
> >   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.
diff mbox series

Patch

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);