Skip to content

Commit e6ed54e

Browse files
VudentzVudentz
committed
Bluetooth: MGMT: Fix UAF on mgmt_remove_adv_monitor_complete
This reworks MGMT_OP_REMOVE_ADV_MONITOR to not use mgmt_pending_add to avoid crashes like bellow: ================================================================== BUG: KASAN: slab-use-after-free in mgmt_remove_adv_monitor_complete+0xe5/0x540 net/bluetooth/mgmt.c:5406 Read of size 8 at addr ffff88801c53f318 by task kworker/u5:5/5341 CPU: 0 UID: 0 PID: 5341 Comm: kworker/u5:5 Not tainted 6.15.0-syzkaller-10402-g4cb6c8af8591 #0 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014 Workqueue: hci0 hci_cmd_sync_work Call Trace: <TASK> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120 print_address_description mm/kasan/report.c:408 [inline] print_report+0xd2/0x2b0 mm/kasan/report.c:521 kasan_report+0x118/0x150 mm/kasan/report.c:634 mgmt_remove_adv_monitor_complete+0xe5/0x540 net/bluetooth/mgmt.c:5406 hci_cmd_sync_work+0x261/0x3a0 net/bluetooth/hci_sync.c:334 process_one_work kernel/workqueue.c:3238 [inline] process_scheduled_works+0xade/0x17b0 kernel/workqueue.c:3321 worker_thread+0x8a0/0xda0 kernel/workqueue.c:3402 kthread+0x711/0x8a0 kernel/kthread.c:464 ret_from_fork+0x3fc/0x770 arch/x86/kernel/process.c:148 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 </TASK> Allocated by task 5987: kasan_save_stack mm/kasan/common.c:47 [inline] kasan_save_track+0x3e/0x80 mm/kasan/common.c:68 poison_kmalloc_redzone mm/kasan/common.c:377 [inline] __kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:394 kasan_kmalloc include/linux/kasan.h:260 [inline] __kmalloc_cache_noprof+0x230/0x3d0 mm/slub.c:4358 kmalloc_noprof include/linux/slab.h:905 [inline] kzalloc_noprof include/linux/slab.h:1039 [inline] mgmt_pending_new+0x65/0x240 net/bluetooth/mgmt_util.c:252 mgmt_pending_add+0x34/0x120 net/bluetooth/mgmt_util.c:279 remove_adv_monitor+0x103/0x1b0 net/bluetooth/mgmt.c:5454 hci_mgmt_cmd+0x9c9/0xef0 net/bluetooth/hci_sock.c:1719 hci_sock_sendmsg+0x6ca/0xef0 net/bluetooth/hci_sock.c:1839 sock_sendmsg_nosec net/socket.c:712 [inline] __sock_sendmsg+0x219/0x270 net/socket.c:727 sock_write_iter+0x258/0x330 net/socket.c:1131 new_sync_write fs/read_write.c:593 [inline] vfs_write+0x548/0xa90 fs/read_write.c:686 ksys_write+0x145/0x250 fs/read_write.c:738 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94 entry_SYSCALL_64_after_hwframe+0x77/0x7f Freed by task 5989: kasan_save_stack mm/kasan/common.c:47 [inline] kasan_save_track+0x3e/0x80 mm/kasan/common.c:68 kasan_save_free_info+0x46/0x50 mm/kasan/generic.c:576 poison_slab_object mm/kasan/common.c:247 [inline] __kasan_slab_free+0x62/0x70 mm/kasan/common.c:264 kasan_slab_free include/linux/kasan.h:233 [inline] slab_free_hook mm/slub.c:2380 [inline] slab_free mm/slub.c:4642 [inline] kfree+0x18e/0x440 mm/slub.c:4841 mgmt_pending_foreach+0xc9/0x120 net/bluetooth/mgmt_util.c:242 mgmt_index_removed+0x10d/0x2f0 net/bluetooth/mgmt.c:9366 hci_sock_bind+0xbe9/0x1000 net/bluetooth/hci_sock.c:1314 __sys_bind_socket net/socket.c:1810 [inline] __sys_bind+0x2c3/0x3e0 net/socket.c:1841 __do_sys_bind net/socket.c:1846 [inline] __se_sys_bind net/socket.c:1844 [inline] __x64_sys_bind+0x7a/0x90 net/socket.c:1844 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline] do_syscall_64+0xfa/0x3b0 arch/x86/entry/syscall_64.c:94 entry_SYSCALL_64_after_hwframe+0x77/0x7f Fixes: 66bd095 ("Bluetooth: advmon offload MSFT remove monitor") Closes: https://syzkaller.appspot.com/bug?extid=feb0dc579bbe30a13190 Reported-by: syzbot+feb0dc579bbe30a13190@syzkaller.appspotmail.com Tested-by: syzbot+feb0dc579bbe30a13190@syzkaller.appspotmail.com Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
1 parent bf2ffc4 commit e6ed54e

File tree

3 files changed

+12
-30
lines changed

3 files changed

+12
-30
lines changed

‎include/net/bluetooth/hci_core.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2400,7 +2400,6 @@ void mgmt_advertising_added(struct sock *sk, struct hci_dev *hdev,
24002400
u8 instance);
24012401
void mgmt_advertising_removed(struct sock *sk, struct hci_dev *hdev,
24022402
u8 instance);
2403-
void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle);
24042403
int mgmt_phy_configuration_changed(struct hci_dev *hdev, struct sock *skip);
24052404
void mgmt_adv_monitor_device_lost(struct hci_dev *hdev, u16 handle,
24062405
bdaddr_t *bdaddr, u8 addr_type);

‎net/bluetooth/hci_core.c‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,10 +1877,8 @@ void hci_free_adv_monitor(struct hci_dev *hdev, struct adv_monitor *monitor)
18771877
if (monitor->handle)
18781878
idr_remove(&hdev->adv_monitors_idr, monitor->handle);
18791879

1880-
if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED) {
1880+
if (monitor->state != ADV_MONITOR_STATE_NOT_REGISTERED)
18811881
hdev->adv_monitors_cnt--;
1882-
mgmt_adv_monitor_removed(hdev, monitor->handle);
1883-
}
18841882

18851883
kfree(monitor);
18861884
}

‎net/bluetooth/mgmt.c‎

Lines changed: 11 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5108,24 +5108,14 @@ static void mgmt_adv_monitor_added(struct sock *sk, struct hci_dev *hdev,
51085108
mgmt_event(MGMT_EV_ADV_MONITOR_ADDED, hdev, &ev, sizeof(ev), sk);
51095109
}
51105110

5111-
void mgmt_adv_monitor_removed(struct hci_dev *hdev, u16 handle)
5111+
static void mgmt_adv_monitor_removed(struct sock *sk, struct hci_dev *hdev,
5112+
u16 handle)
51125113
{
51135114
struct mgmt_ev_adv_monitor_removed ev;
5114-
struct mgmt_pending_cmd *cmd;
5115-
struct sock *sk_skip = NULL;
5116-
struct mgmt_cp_remove_adv_monitor *cp;
5117-
5118-
cmd = pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev);
5119-
if (cmd) {
5120-
cp = cmd->param;
5121-
5122-
if (cp->monitor_handle)
5123-
sk_skip = cmd->sk;
5124-
}
51255115

51265116
ev.monitor_handle = cpu_to_le16(handle);
51275117

5128-
mgmt_event(MGMT_EV_ADV_MONITOR_REMOVED, hdev, &ev, sizeof(ev), sk_skip);
5118+
mgmt_event(MGMT_EV_ADV_MONITOR_REMOVED, hdev, &ev, sizeof(ev), sk);
51295119
}
51305120

51315121
static int read_adv_mon_features(struct sock *sk, struct hci_dev *hdev,
@@ -5227,8 +5217,7 @@ static int __add_adv_patterns_monitor(struct sock *sk, struct hci_dev *hdev,
52275217

52285218
if (pending_find(MGMT_OP_SET_LE, hdev) ||
52295219
pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) ||
5230-
pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev) ||
5231-
pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev)) {
5220+
pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev)) {
52325221
status = MGMT_STATUS_BUSY;
52335222
goto unlock;
52345223
}
@@ -5398,8 +5387,7 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
53985387
struct mgmt_pending_cmd *cmd = data;
53995388
struct mgmt_cp_remove_adv_monitor *cp;
54005389

5401-
if (status == -ECANCELED ||
5402-
cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev))
5390+
if (status == -ECANCELED)
54035391
return;
54045392

54055393
hci_dev_lock(hdev);
@@ -5408,12 +5396,14 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
54085396

54095397
rp.monitor_handle = cp->monitor_handle;
54105398

5411-
if (!status)
5399+
if (!status) {
5400+
mgmt_adv_monitor_removed(cmd->sk, hdev, cp->monitor_handle);
54125401
hci_update_passive_scan(hdev);
5402+
}
54135403

54145404
mgmt_cmd_complete(cmd->sk, cmd->index, cmd->opcode,
54155405
mgmt_status(status), &rp, sizeof(rp));
5416-
mgmt_pending_remove(cmd);
5406+
mgmt_pending_free(cmd);
54175407

54185408
hci_dev_unlock(hdev);
54195409
bt_dev_dbg(hdev, "remove monitor %d complete, status %d",
@@ -5423,10 +5413,6 @@ static void mgmt_remove_adv_monitor_complete(struct hci_dev *hdev,
54235413
static int mgmt_remove_adv_monitor_sync(struct hci_dev *hdev, void *data)
54245414
{
54255415
struct mgmt_pending_cmd *cmd = data;
5426-
5427-
if (cmd != pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev))
5428-
return -ECANCELED;
5429-
54305416
struct mgmt_cp_remove_adv_monitor *cp = cmd->param;
54315417
u16 handle = __le16_to_cpu(cp->monitor_handle);
54325418

@@ -5445,14 +5431,13 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev,
54455431
hci_dev_lock(hdev);
54465432

54475433
if (pending_find(MGMT_OP_SET_LE, hdev) ||
5448-
pending_find(MGMT_OP_REMOVE_ADV_MONITOR, hdev) ||
54495434
pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR, hdev) ||
54505435
pending_find(MGMT_OP_ADD_ADV_PATTERNS_MONITOR_RSSI, hdev)) {
54515436
status = MGMT_STATUS_BUSY;
54525437
goto unlock;
54535438
}
54545439

5455-
cmd = mgmt_pending_add(sk, MGMT_OP_REMOVE_ADV_MONITOR, hdev, data, len);
5440+
cmd = mgmt_pending_new(sk, MGMT_OP_REMOVE_ADV_MONITOR, hdev, data, len);
54565441
if (!cmd) {
54575442
status = MGMT_STATUS_NO_RESOURCES;
54585443
goto unlock;
@@ -5462,7 +5447,7 @@ static int remove_adv_monitor(struct sock *sk, struct hci_dev *hdev,
54625447
mgmt_remove_adv_monitor_complete);
54635448

54645449
if (err) {
5465-
mgmt_pending_remove(cmd);
5450+
mgmt_pending_free(cmd);
54665451

54675452
if (err == -ENOMEM)
54685453
status = MGMT_STATUS_NO_RESOURCES;

0 commit comments

Comments
 (0)