[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20241221170459.23095-1-mazin@getstate.dev>
Date: Sat, 21 Dec 2024 20:04:59 +0300
From: Mazin Al Haddad <mazin@...state.dev>
To: marcel@...tmann.org,
johan.hedberg@...il.com,
luiz.dentz@...il.com
Cc: linux-bluetooth@...r.kernel.org,
linux-kernel@...r.kernel.org,
Mazin Al Haddad <mazin@...state.dev>,
syzbot+479aff51bb361ef5aa18@...kaller.appspotmail.com
Subject: [PATCH v2] bluetooth: hci: Fix UAF from MGMT_OP_REMOVE_ADV_MONITOR during closure
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@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=479aff51bb361ef5aa18
Signed-off-by: Mazin Al Haddad <mazin@...state.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(-)
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);
base-commit: 499551201b5f4fd3c0618a3e95e3d0d15ea18f31
--
2.46.0
Powered by blists - more mailing lists