| Message ID | Z1iYKvmenw81i1UG@quatroqueijos.cascardo.eti.br (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump | expand |
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 10 Dec 2024 16:36:10 -0300 you wrote: > hci_devcd_append may lead to the release of the skb, so it cannot be > accessed once it is called. > > ================================================================== > BUG: KASAN: slab-use-after-free in btmtk_process_coredump+0x2a7/0x2d0 [btmtk] > Read of size 4 at addr ffff888033cfabb0 by task kworker/0:3/82 > > [...] Here is the summary with links: - [Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump https://git.kernel.org/bluetooth/bluetooth-next/c/845b4c649f56 You are awesome, thank you!
Il 10/12/24 20:36, Thadeu Lima de Souza Cascardo ha scritto: > hci_devcd_append may lead to the release of the skb, so it cannot be > accessed once it is called. > > ================================================================== > BUG: KASAN: slab-use-after-free in btmtk_process_coredump+0x2a7/0x2d0 [btmtk] > Read of size 4 at addr ffff888033cfabb0 by task kworker/0:3/82 > > CPU: 0 PID: 82 Comm: kworker/0:3 Tainted: G U 6.6.40-lockdep-03464-g1d8b4eb3060e #1 b0b3c1cc0c842735643fb411799d97921d1f688c > Hardware name: Google Yaviks_Ufs/Yaviks_Ufs, BIOS Google_Yaviks_Ufs.15217.552.0 05/07/2024 > Workqueue: events btusb_rx_work [btusb] > Call Trace: > <TASK> > dump_stack_lvl+0xfd/0x150 > print_report+0x131/0x780 > kasan_report+0x177/0x1c0 > btmtk_process_coredump+0x2a7/0x2d0 [btmtk 03edd567dd71a65958807c95a65db31d433e1d01] > btusb_recv_acl_mtk+0x11c/0x1a0 [btusb 675430d1e87c4f24d0c1f80efe600757a0f32bec] > btusb_rx_work+0x9e/0xe0 [btusb 675430d1e87c4f24d0c1f80efe600757a0f32bec] > worker_thread+0xe44/0x2cc0 > kthread+0x2ff/0x3a0 > ret_from_fork+0x51/0x80 > ret_from_fork_asm+0x1b/0x30 > </TASK> > > Allocated by task 82: > stack_trace_save+0xdc/0x190 > kasan_set_track+0x4e/0x80 > __kasan_slab_alloc+0x4e/0x60 > kmem_cache_alloc+0x19f/0x360 > skb_clone+0x132/0xf70 > btusb_recv_acl_mtk+0x104/0x1a0 [btusb] > btusb_rx_work+0x9e/0xe0 [btusb] > worker_thread+0xe44/0x2cc0 > kthread+0x2ff/0x3a0 > ret_from_fork+0x51/0x80 > ret_from_fork_asm+0x1b/0x30 > > Freed by task 1733: > stack_trace_save+0xdc/0x190 > kasan_set_track+0x4e/0x80 > kasan_save_free_info+0x28/0xb0 > ____kasan_slab_free+0xfd/0x170 > kmem_cache_free+0x183/0x3f0 > hci_devcd_rx+0x91a/0x2060 [bluetooth] > worker_thread+0xe44/0x2cc0 > kthread+0x2ff/0x3a0 > ret_from_fork+0x51/0x80 > ret_from_fork_asm+0x1b/0x30 > > The buggy address belongs to the object at ffff888033cfab40 > which belongs to the cache skbuff_head_cache of size 232 > The buggy address is located 112 bytes inside of > freed 232-byte region [ffff888033cfab40, ffff888033cfac28) > > The buggy address belongs to the physical page: > page:00000000a174ba93 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x33cfa > head:00000000a174ba93 order:1 entire_mapcount:0 nr_pages_mapped:0 pincount:0 > anon flags: 0x4000000000000840(slab|head|zone=1) > page_type: 0xffffffff() > raw: 4000000000000840 ffff888100848a00 0000000000000000 0000000000000001 > raw: 0000000000000000 0000000080190019 00000001ffffffff 0000000000000000 > page dumped because: kasan: bad access detected > > Memory state around the buggy address: > ffff888033cfaa80: fb fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc > ffff888033cfab00: fc fc fc fc fc fc fc fc fa fb fb fb fb fb fb fb >> ffff888033cfab80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ^ > ffff888033cfac00: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc > ffff888033cfac80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb > ================================================================== > > Check if we need to call hci_devcd_complete before calling > hci_devcd_append. That requires that we check data->cd_info.cnt >= > MTK_COREDUMP_NUM instead of data->cd_info.cnt > MTK_COREDUMP_NUM, as we > increment data->cd_info.cnt only once the call to hci_devcd_append > succeeds. > > Fixes: 0b7015132878 ("Bluetooth: btusb: mediatek: add MediaTek devcoredump support") > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c index b7c348687a77..46f605249df7 100644 --- a/drivers/bluetooth/btmtk.c +++ b/drivers/bluetooth/btmtk.c @@ -395,6 +395,7 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb) { struct btmtk_data *data = hci_get_priv(hdev); int err; + bool complete = false; if (!IS_ENABLED(CONFIG_DEV_COREDUMP)) { kfree_skb(skb); @@ -416,19 +417,22 @@ int btmtk_process_coredump(struct hci_dev *hdev, struct sk_buff *skb) fallthrough; case HCI_DEVCOREDUMP_ACTIVE: default: + /* Mediatek coredump data would be more than MTK_COREDUMP_NUM */ + if (data->cd_info.cnt >= MTK_COREDUMP_NUM && + skb->len > MTK_COREDUMP_END_LEN) + if (!memcmp((char *)&skb->data[skb->len - MTK_COREDUMP_END_LEN], + MTK_COREDUMP_END, MTK_COREDUMP_END_LEN - 1)) + complete = true; + err = hci_devcd_append(hdev, skb); if (err < 0) break; data->cd_info.cnt++; - /* Mediatek coredump data would be more than MTK_COREDUMP_NUM */ - if (data->cd_info.cnt > MTK_COREDUMP_NUM && - skb->len > MTK_COREDUMP_END_LEN) - if (!memcmp((char *)&skb->data[skb->len - MTK_COREDUMP_END_LEN], - MTK_COREDUMP_END, MTK_COREDUMP_END_LEN - 1)) { - bt_dev_info(hdev, "Mediatek coredump end"); - hci_devcd_complete(hdev); - } + if (complete) { + bt_dev_info(hdev, "Mediatek coredump end"); + hci_devcd_complete(hdev); + } break; }