* [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
@ 2024-07-31 19:03 Thadeu Lima de Souza Cascardo
2024-07-31 19:33 ` bluez.test.bot
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2024-07-31 19:03 UTC (permalink / raw)
To: linux-bluetooth
Cc: linux-mediatek, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Jing Cai, Chris Lu,
Sean Wang, kernel-dev, Thadeu Lima de Souza Cascardo
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>
---
drivers/bluetooth/btmtk.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-07-31 19:03 [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump Thadeu Lima de Souza Cascardo
@ 2024-07-31 19:33 ` bluez.test.bot
2024-08-22 9:57 ` [PATCH] " Thadeu Lima de Souza Cascardo
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
2 siblings, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2024-07-31 19:33 UTC (permalink / raw)
To: linux-bluetooth, cascardo
[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]
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=875598
---Test result---
Test Summary:
CheckPatch FAIL 0.94 seconds
GitLint FAIL 0.50 seconds
SubjectPrefix PASS 0.83 seconds
BuildKernel PASS 30.40 seconds
CheckAllWarning PASS 45.30 seconds
CheckSparse PASS 38.73 seconds
CheckSmatch PASS 104.59 seconds
BuildKernel32 PASS 29.56 seconds
TestRunnerSetup PASS 539.78 seconds
TestRunner_l2cap-tester PASS 20.46 seconds
TestRunner_iso-tester PASS 41.69 seconds
TestRunner_bnep-tester PASS 5.08 seconds
TestRunner_mgmt-tester PASS 119.77 seconds
TestRunner_rfcomm-tester PASS 7.79 seconds
TestRunner_sco-tester PASS 15.28 seconds
TestRunner_ioctl-tester PASS 8.33 seconds
TestRunner_mesh-tester PASS 6.25 seconds
TestRunner_smp-tester PASS 7.19 seconds
TestRunner_userchan-tester PASS 5.36 seconds
IncrementalBuild PASS 28.58 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#81:
CPU: 0 PID: 82 Comm: kworker/0:3 Tainted: G U 6.6.40-lockdep-03464-g1d8b4eb3060e #1 b0b3c1cc0c842735643fb411799d97921d1f688c
total: 0 errors, 1 warnings, 37 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13749065.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
10: B1 Line exceeds max length (140>80): "CPU: 0 PID: 82 Comm: kworker/0:3 Tainted: G U 6.6.40-lockdep-03464-g1d8b4eb3060e #1 b0b3c1cc0c842735643fb411799d97921d1f688c"
11: B1 Line exceeds max length (90>80): "Hardware name: Google Yaviks_Ufs/Yaviks_Ufs, BIOS Google_Yaviks_Ufs.15217.552.0 05/07/2024"
18: B1 Line exceeds max length (84>80): " btmtk_process_coredump+0x2a7/0x2d0 [btmtk 03edd567dd71a65958807c95a65db31d433e1d01]"
58: B1 Line exceeds max length (90>80): "page:00000000a174ba93 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x33cfa"
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-07-31 19:03 [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump Thadeu Lima de Souza Cascardo
2024-07-31 19:33 ` bluez.test.bot
@ 2024-08-22 9:57 ` Thadeu Lima de Souza Cascardo
2024-09-02 3:39 ` Chris Lu (陸稚泓)
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
2 siblings, 1 reply; 8+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2024-08-22 9:57 UTC (permalink / raw)
To: linux-bluetooth
Cc: linux-mediatek, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Jing Cai, Chris Lu,
Sean Wang, kernel-dev
On Wed, Jul 31, 2024 at 04:03:30PM -0300, Thadeu Lima de Souza Cascardo 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
>
> 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>
> ---
> drivers/bluetooth/btmtk.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> 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;
> }
> --
> 2.34.1
>
>
Hi, anyone can help review this?
Thanks.
Cascardo.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-08-22 9:57 ` [PATCH] " Thadeu Lima de Souza Cascardo
@ 2024-09-02 3:39 ` Chris Lu (陸稚泓)
0 siblings, 0 replies; 8+ messages in thread
From: Chris Lu (陸稚泓) @ 2024-09-02 3:39 UTC (permalink / raw)
To: cascardo@igalia.com, linux-bluetooth@vger.kernel.org
Cc: marcel@holtmann.org, linux-mediatek@lists.infradead.org,
kernel-dev@igalia.com, Jing Cai (蔡晶), Sean Wang,
matthias.bgg@gmail.com, luiz.dentz@gmail.com,
angelogioacchino.delregno@collabora.com
On Thu, 2024-08-22 at 06:57 -0300, Thadeu Lima de Souza Cascardo wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Wed, Jul 31, 2024 at 04:03:30PM -0300, Thadeu Lima de Souza
> Cascardo 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
> >
> > 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>
> > ---
> > drivers/bluetooth/btmtk.c | 20 ++++++++++++--------
> > 1 file changed, 12 insertions(+), 8 deletions(-)
> >
> > 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;
> > }
> > --
> > 2.34.1
> >
> >
>
> Hi, anyone can help review this?
>
> Thanks.
Hi Thadeu,
Sorry for late reply.
I've also verified on the platform which supports devcoredump feature
that Bluetooth can recover after triggerring devcoredump.
LGTM. It may need maintainer's approval for the next step.
Thanks a lot!
Chris
> Cascardo.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-07-31 19:03 [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump Thadeu Lima de Souza Cascardo
2024-07-31 19:33 ` bluez.test.bot
2024-08-22 9:57 ` [PATCH] " Thadeu Lima de Souza Cascardo
@ 2024-12-10 19:36 ` Thadeu Lima de Souza Cascardo
2024-12-10 20:34 ` [Resend] " bluez.test.bot
` (2 more replies)
2 siblings, 3 replies; 8+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2024-12-10 19:36 UTC (permalink / raw)
To: linux-bluetooth
Cc: linux-mediatek, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, AngeloGioacchino Del Regno, Jing Cai, Chris Lu,
Sean Wang, kernel-dev, Thadeu Lima de Souza Cascardo
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>
---
Resending as this has not been applied yet.
This has been tested by Chris Lu.
Also, the tests GitLint and CheckPatch are complaining about long lines,
but those are from dmesg logs, so they are kept as is with no added
wrapping.
Thanks.
Cascardo.
---
drivers/bluetooth/btmtk.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
@ 2024-12-10 20:34 ` bluez.test.bot
2024-12-11 16:50 ` [PATCH Resend] " patchwork-bot+bluetooth
2024-12-12 10:03 ` AngeloGioacchino Del Regno
2 siblings, 0 replies; 8+ messages in thread
From: bluez.test.bot @ 2024-12-10 20:34 UTC (permalink / raw)
To: linux-bluetooth, cascardo
[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]
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=916487
---Test result---
Test Summary:
CheckPatch PENDING 0.24 seconds
GitLint PENDING 0.21 seconds
SubjectPrefix PASS 0.11 seconds
BuildKernel PASS 25.57 seconds
CheckAllWarning PASS 27.55 seconds
CheckSparse PASS 31.19 seconds
BuildKernel32 PASS 24.98 seconds
TestRunnerSetup PASS 440.43 seconds
TestRunner_l2cap-tester PASS 20.71 seconds
TestRunner_iso-tester FAIL 31.68 seconds
TestRunner_bnep-tester PASS 4.86 seconds
TestRunner_mgmt-tester FAIL 120.89 seconds
TestRunner_rfcomm-tester PASS 7.65 seconds
TestRunner_sco-tester PASS 9.48 seconds
TestRunner_ioctl-tester PASS 8.28 seconds
TestRunner_mesh-tester PASS 6.03 seconds
TestRunner_smp-tester PASS 7.08 seconds
TestRunner_userchan-tester PASS 5.13 seconds
IncrementalBuild PENDING 0.91 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
WARNING: possible circular locking dependency detected
Total: 125, Passed: 121 (96.8%), Failed: 0, Not Run: 4
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 487 (99.0%), Failed: 1, Not Run: 4
Failed Test Cases
LL Privacy - Start Discovery 2 (Disable RL) Failed 0.194 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
2024-12-10 20:34 ` [Resend] " bluez.test.bot
@ 2024-12-11 16:50 ` patchwork-bot+bluetooth
2024-12-12 10:03 ` AngeloGioacchino Del Regno
2 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+bluetooth @ 2024-12-11 16:50 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo
Cc: linux-bluetooth, linux-mediatek, marcel, luiz.dentz, matthias.bgg,
angelogioacchino.delregno, jing.cai, chris.lu, sean.wang,
kernel-dev
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH Resend] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
2024-12-10 20:34 ` [Resend] " bluez.test.bot
2024-12-11 16:50 ` [PATCH Resend] " patchwork-bot+bluetooth
@ 2024-12-12 10:03 ` AngeloGioacchino Del Regno
2 siblings, 0 replies; 8+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-12-12 10:03 UTC (permalink / raw)
To: Thadeu Lima de Souza Cascardo, linux-bluetooth
Cc: linux-mediatek, Marcel Holtmann, Luiz Augusto von Dentz,
Matthias Brugger, Jing Cai, Chris Lu, Sean Wang, kernel-dev
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>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-12-12 10:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-31 19:03 [PATCH] Bluetooth: btmtk: avoid UAF in btmtk_process_coredump Thadeu Lima de Souza Cascardo
2024-07-31 19:33 ` bluez.test.bot
2024-08-22 9:57 ` [PATCH] " Thadeu Lima de Souza Cascardo
2024-09-02 3:39 ` Chris Lu (陸稚泓)
2024-12-10 19:36 ` [PATCH Resend] " Thadeu Lima de Souza Cascardo
2024-12-10 20:34 ` [Resend] " bluez.test.bot
2024-12-11 16:50 ` [PATCH Resend] " patchwork-bot+bluetooth
2024-12-12 10:03 ` AngeloGioacchino Del Regno
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).