| Message ID | 20241221170459.23095-1-mazin@getstate.dev |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] bluetooth: hci: Fix UAF from MGMT_OP_REMOVE_ADV_MONITOR during closure | expand |
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=920165 ---Test result--- Test Summary: CheckPatch PENDING 0.27 seconds GitLint PENDING 0.21 seconds SubjectPrefix FAIL 0.30 seconds BuildKernel PASS 23.83 seconds CheckAllWarning PASS 26.31 seconds CheckSparse PASS 29.67 seconds BuildKernel32 PASS 23.85 seconds TestRunnerSetup PASS 422.40 seconds TestRunner_l2cap-tester PASS 19.94 seconds TestRunner_iso-tester PASS 29.05 seconds TestRunner_bnep-tester PASS 4.66 seconds TestRunner_mgmt-tester FAIL 115.93 seconds TestRunner_rfcomm-tester PASS 7.45 seconds TestRunner_sco-tester PASS 9.16 seconds TestRunner_ioctl-tester PASS 7.87 seconds TestRunner_mesh-tester PASS 5.91 seconds TestRunner_smp-tester PASS 6.77 seconds TestRunner_userchan-tester PASS 6.13 seconds IncrementalBuild PENDING 0.62 seconds Details ############################## Test: CheckPatch - PENDING Desc: Run checkpatch.pl script Output: ############################## Test: GitLint - PENDING Desc: Run gitlint Output: ############################## Test: SubjectPrefix - FAIL Desc: Check subject contains "Bluetooth" prefix Output: "Bluetooth: " prefix is not specified in the subject ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 490, Passed: 484 (98.8%), Failed: 2, Not Run: 4 Failed Test Cases LL Privacy - Add Device 3 (AL is full) Failed 0.214 seconds LL Privacy - Set Flags 1 (Add to RL) Failed 0.158 seconds ############################## Test: IncrementalBuild - PENDING Desc: Incremental build with the patches in the series Output: --- Regards, Linux Bluetooth
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c86f4e42e69c..aa5aa3fed32d 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -5197,6 +5197,9 @@ int hci_dev_close_sync(struct hci_dev *hdev) */ drain_workqueue(hdev->workqueue); + /* flush cmd work */ + flush_work(&hdev->cmd_work); + hci_dev_lock(hdev); hci_discovery_set_state(hdev, DISCOVERY_STOPPED); @@ -5234,8 +5237,6 @@ int hci_dev_close_sync(struct hci_dev *hdev) clear_bit(HCI_INIT, &hdev->flags); } - /* flush cmd work */ - flush_work(&hdev->cmd_work); /* Drop queues */ skb_queue_purge(&hdev->rx_q); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index b31192d473d0..ec86ae851e56 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -5519,9 +5519,17 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, { struct mgmt_rp_remove_adv_monitor rp; struct mgmt_pending_cmd *cmd = data; - struct mgmt_cp_remove_adv_monitor *cp = cmd->param; + struct mgmt_cp_remove_adv_monitor *cp; + + // if executing while device is closing down, status could + // be invalid as pending cmd could be removed by __mgmt_power_off + // so exit early if the device was busy. + if (status == -ECANCELED || + cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) + return; hci_dev_lock(hdev); + cp = cmd->param; rp.monitor_handle = cp->monitor_handle; @@ -5540,6 +5548,10 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev, static int mgmt_remove_adv_monitor_sync(struct hci_dev *hdev, void *data) { struct mgmt_pending_cmd *cmd = data; + + if (cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) + return -ECANCELED; + struct mgmt_cp_remove_adv_monitor *cp = cmd->param; u16 handle = __le16_to_cpu(cp->monitor_handle); @@ -9544,8 +9556,12 @@ void __mgmt_power_off(struct hci_dev *hdev) */ if (hci_dev_test_flag(hdev, HCI_UNREGISTER)) match.mgmt_status = MGMT_STATUS_INVALID_INDEX; - else + else { + match.mgmt_status = MGMT_STATUS_CANCELLED; + mgmt_pending_foreach(MGMT_OP_REMOVE_ADV_MONITOR, hdev, + cmd_status_rsp, &match); match.mgmt_status = MGMT_STATUS_NOT_POWERED; + } mgmt_pending_foreach(0, hdev, cmd_complete_rsp, &match);
When hci_dev is closing down, mgmt_power_off will free parts of the device. The freed memory can then be accessed when processing pending MGMT_OP_REMOVE_ADV_MONITOR cmds. Since submitting the command is allowed when it is powered off (as in previous discussions linked below), fix this by returning MGMT_STATUS_BUSY to pending MGMT_OP_REMOVE_ADV_MONITOR operations submitted as hci_dev_close_sync is running. Avoid processing pending cmds since doing so will lead to reacquiring the same lock. Add a sanity check within mgmt_remove_adv_monitor to ensure the cmd is still valid and exit early if not. BUG: KASAN: slab-use-after-free in mgmt_remove_adv_monitor_sync+0x3a/0xd0 net/bluetooth/mgmt.c:5543 Read of size 8 at addr ffff88814128f898 by task kworker/u9:4/5961 <TASK> __dump_stack lib/dump_stack.c:94 [inline] dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120 print_address_description mm/kasan/report.c:378 [inline] print_report+0x169/0x550 mm/kasan/report.c:489 kasan_report+0x143/0x180 mm/kasan/report.c:602 mgmt_remove_adv_monitor_sync+0x3a/0xd0 net/bluetooth/mgmt.c:5543 hci_cmd_sync_work+0x22b/0x400 net/bluetooth/hci_sync.c:332 process_one_work kernel/workqueue.c:3229 [inline] process_scheduled_works+0xa63/0x1850 kernel/workqueue.c:3310 worker_thread+0x870/0xd30 kernel/workqueue.c:3391 kthread+0x2f0/0x390 kernel/kthread.c:389 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244 </TASK> Freed by task 16022: 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:582 poison_slab_object mm/kasan/common.c:247 [inline] __kasan_slab_free+0x59/0x70 mm/kasan/common.c:264 kasan_slab_free include/linux/kasan.h:233 [inline] slab_free_hook mm/slub.c:2338 [inline] slab_free mm/slub.c:4598 [inline] kfree+0x196/0x420 mm/slub.c:4746 mgmt_pending_foreach+0xd1/0x130 net/bluetooth/mgmt_util.c:259 __mgmt_power_off+0x183/0x430 net/bluetooth/mgmt.c:9550 hci_dev_close_sync+0x6c4/0x11c0 net/bluetooth/hci_sync.c:5208 hci_dev_do_close net/bluetooth/hci_core.c:483 [inline] hci_dev_close+0x112/0x210 net/bluetooth/hci_core.c:508 sock_do_ioctl+0x158/0x460 net/socket.c:1209 sock_ioctl+0x626/0x8e0 net/socket.c:1328 vfs_ioctl fs/ioctl.c:51 [inline] __do_sys_ioctl fs/ioctl.c:906 [inline] __se_sys_ioctl+0xf5/0x170 fs/ioctl.c:892 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f Link: https://lore.kernel.org/lkml/20240424135903.24169-1-jlee@suse.com/ Reported-by: syzbot+479aff51bb361ef5aa18@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=479aff51bb361ef5aa18 Signed-off-by: Mazin Al Haddad <mazin@getstate.dev> --- Changes since v1: * Change return code to ECANCELED * Send out MGMT_STATUS_CANCELLED instead of MGMT_STATUS_BUSY * Style fixes net/bluetooth/hci_sync.c | 5 +++-- net/bluetooth/mgmt.c | 20 ++++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) base-commit: 499551201b5f4fd3c0618a3e95e3d0d15ea18f31