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@xxxxxxxx/ Reported-by: syzbot+479aff51bb361ef5aa18@xxxxxxxxxxxxxxxxxxxxxxxxx Closes: https://syzkaller.appspot.com/bug?extid=479aff51bb361ef5aa18 Signed-off-by: Mazin Al Haddad <mazin@xxxxxxxxxxxx> --- net/bluetooth/hci_sync.c | 5 +++-- net/bluetooth/mgmt.c | 18 ++++++++++++++++-- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index c86f4e42e..aa5aa3fed 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 b31192d47..6d2e5253b 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -5519,9 +5519,16 @@ 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 (status == -EINVAL || 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 +5547,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 -EINVAL; + struct mgmt_cp_remove_adv_monitor *cp = cmd->param; u16 handle = __le16_to_cpu(cp->monitor_handle); @@ -9544,8 +9555,11 @@ 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_BUSY; + 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); -- 2.46.0
