| Message ID | 20221116145821.544266-1-linuxlovemin@yonsei.ac.kr |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss() | expand |
Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: > This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in > cfg80211_find_elem_match() called from brcmf_inform_single_bss() when > the offset and length values of information elements provided by the > device exceed the boundary of the escan buffer that contains information > elements. The patch adds a check that makes the function return -EINVAL > if that is the case. Note that the negative return is handled by the > caller, brcmf_inform_bss(). > > Found by a modified version of syzkaller. > > ================================================================== > BUG: KASAN: slab-out-of-bounds in cfg80211_find_elem_match+0x164/0x180 > Read of size 1 at addr ffff888018f0fde9 by task kworker/0:2/1896 > > CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G O 5.14.0+ #139 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > Workqueue: events brcmf_fweh_event_worker > Call Trace: > dump_stack_lvl+0x8e/0xd1 > print_address_description.constprop.0.cold+0x93/0x334 > ? cfg80211_find_elem_match+0x164/0x180 > kasan_report.cold+0x79/0xd5 > ? cfg80211_find_elem_match+0x164/0x180 > cfg80211_find_elem_match+0x164/0x180 > cfg80211_get_bss_channel+0x69/0x320 > cfg80211_inform_single_bss_data+0x1a6/0x1060 > ? cfg80211_bss_update+0x1e20/0x1e20 > ? rcu_read_lock_sched_held+0xa1/0xd0 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? find_held_lock+0x2d/0x110 > ? cfg80211_inform_bss_data+0xcb/0x160 > cfg80211_inform_bss_data+0xcb/0x160 > ? cfg80211_parse_mbssid_data+0x1540/0x1540 > ? kvm_clock_get_cycles+0x14/0x20 > ? ktime_get_with_offset+0x2b9/0x450 > brcmf_inform_single_bss+0x36d/0x4d0 > ? brcmf_notify_mic_status+0xb0/0xb0 > ? __lock_acquire+0x181f/0x5790 > ? brcmf_p2p_cancel_remain_on_channel+0x30/0x30 > brcmf_inform_bss+0x131/0x210 > brcmf_cfg80211_escan_handler+0x779/0xd20 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? lock_acquire+0x19d/0x4e0 > ? find_held_lock+0x2d/0x110 > ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 > ? brcmf_fweh_event_worker+0x249/0xc00 > ? mark_held_locks+0x9f/0xe0 > ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0 > ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 > brcmf_fweh_call_event_handler.isra.0+0x90/0x100 > brcmf_fweh_event_worker+0x117/0xc00 > ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 > ? rcu_read_lock_sched_held+0xa1/0xd0 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? lockdep_hardirqs_on_prepare+0x273/0x3e0 > process_one_work+0x92b/0x1460 > ? pwq_dec_nr_in_flight+0x330/0x330 > ? rwlock_bug.part.0+0x90/0x90 > worker_thread+0x95/0xe00 > ? __kthread_parkme+0x115/0x1e0 > ? process_one_work+0x1460/0x1460 > kthread+0x3a1/0x480 > ? set_kthread_struct+0x120/0x120 > ret_from_fork+0x1f/0x30 > > The buggy address belongs to the page: > page:ffffea000063c000 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x18f00 > head:ffffea000063c000 order:4 compound_mapcount:0 compound_pincount:0 > flags: 0x100000000010000(head|node=0|zone=1) > raw: 0100000000010000 0000000000000000 dead000000000122 0000000000000000 > raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 > page dumped because: kasan: bad access detected > page_owner tracks the page as allocated > page last allocated via order 4, migratetype Unmovable, gfp_mask 0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), pid 1896, ts 44510886600, free_ts 0 > prep_new_page+0x1aa/0x240 > get_page_from_freelist+0x159a/0x27c0 > __alloc_pages+0x2da/0x6a0 > alloc_pages+0xec/0x1e0 > kmalloc_order+0x39/0xf0 > kmalloc_order_trace+0x19/0x120 > brcmf_cfg80211_attach+0x5c9/0x3fd0 > brcmf_attach+0x389/0xd40 > brcmf_usb_probe+0x12de/0x1690 > usb_probe_interface+0x2aa/0x760 > really_probe+0x205/0xb70 > __driver_probe_device+0x311/0x4b0 > driver_probe_device+0x4e/0x150 > __device_attach_driver+0x1cc/0x2a0 > bus_for_each_drv+0x156/0x1d0 > __device_attach+0x23f/0x3a0 > page_owner free stack trace missing > > Memory state around the buggy address: > ffff888018f0fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ffff888018f0fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > >ffff888018f0fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fe fe fe > ^ > ffff888018f0fe00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe > ffff888018f0fe80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe > ================================================================== > > Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> > Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> > Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> Can someone review this?
On December 22, 2022 4:55:31 PM Kalle Valo <kvalo@kernel.org> wrote: > Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: > >> This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in >> cfg80211_find_elem_match() called from brcmf_inform_single_bss() when >> the offset and length values of information elements provided by the >> device exceed the boundary of the escan buffer that contains information >> elements. The patch adds a check that makes the function return -EINVAL >> if that is the case. Note that the negative return is handled by the >> caller, brcmf_inform_bss(). >> >> Found by a modified version of syzkaller. >> >> ================================================================== >> BUG: KASAN: slab-out-of-bounds in cfg80211_find_elem_match+0x164/0x180 >> Read of size 1 at addr ffff888018f0fde9 by task kworker/0:2/1896 >> >> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G O 5.14.0+ #139 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >> rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 >> Workqueue: events brcmf_fweh_event_worker >> Call Trace: >> dump_stack_lvl+0x8e/0xd1 >> print_address_description.constprop.0.cold+0x93/0x334 >> ? cfg80211_find_elem_match+0x164/0x180 >> kasan_report.cold+0x79/0xd5 >> ? cfg80211_find_elem_match+0x164/0x180 >> cfg80211_find_elem_match+0x164/0x180 >> cfg80211_get_bss_channel+0x69/0x320 >> cfg80211_inform_single_bss_data+0x1a6/0x1060 >> ? cfg80211_bss_update+0x1e20/0x1e20 >> ? rcu_read_lock_sched_held+0xa1/0xd0 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? find_held_lock+0x2d/0x110 >> ? cfg80211_inform_bss_data+0xcb/0x160 >> cfg80211_inform_bss_data+0xcb/0x160 >> ? cfg80211_parse_mbssid_data+0x1540/0x1540 >> ? kvm_clock_get_cycles+0x14/0x20 >> ? ktime_get_with_offset+0x2b9/0x450 >> brcmf_inform_single_bss+0x36d/0x4d0 >> ? brcmf_notify_mic_status+0xb0/0xb0 >> ? __lock_acquire+0x181f/0x5790 >> ? brcmf_p2p_cancel_remain_on_channel+0x30/0x30 >> brcmf_inform_bss+0x131/0x210 >> brcmf_cfg80211_escan_handler+0x779/0xd20 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? lock_acquire+0x19d/0x4e0 >> ? find_held_lock+0x2d/0x110 >> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >> ? brcmf_fweh_event_worker+0x249/0xc00 >> ? mark_held_locks+0x9f/0xe0 >> ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0 >> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >> brcmf_fweh_call_event_handler.isra.0+0x90/0x100 >> brcmf_fweh_event_worker+0x117/0xc00 >> ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 >> ? rcu_read_lock_sched_held+0xa1/0xd0 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? lockdep_hardirqs_on_prepare+0x273/0x3e0 >> process_one_work+0x92b/0x1460 >> ? pwq_dec_nr_in_flight+0x330/0x330 >> ? rwlock_bug.part.0+0x90/0x90 >> worker_thread+0x95/0xe00 >> ? __kthread_parkme+0x115/0x1e0 >> ? process_one_work+0x1460/0x1460 >> kthread+0x3a1/0x480 >> ? set_kthread_struct+0x120/0x120 >> ret_from_fork+0x1f/0x30 >> >> The buggy address belongs to the page: >> page:ffffea000063c000 refcount:1 mapcount:0 mapping:0000000000000000 >> index:0x0 pfn:0x18f00 >> head:ffffea000063c000 order:4 compound_mapcount:0 compound_pincount:0 >> flags: 0x100000000010000(head|node=0|zone=1) >> raw: 0100000000010000 0000000000000000 dead000000000122 0000000000000000 >> raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 >> page dumped because: kasan: bad access detected >> page_owner tracks the page as allocated >> page last allocated via order 4, migratetype Unmovable, gfp_mask >> 0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), pid 1896, ts 44510886600, free_ts 0 >> prep_new_page+0x1aa/0x240 >> get_page_from_freelist+0x159a/0x27c0 >> __alloc_pages+0x2da/0x6a0 >> alloc_pages+0xec/0x1e0 >> kmalloc_order+0x39/0xf0 >> kmalloc_order_trace+0x19/0x120 >> brcmf_cfg80211_attach+0x5c9/0x3fd0 >> brcmf_attach+0x389/0xd40 >> brcmf_usb_probe+0x12de/0x1690 >> usb_probe_interface+0x2aa/0x760 >> really_probe+0x205/0xb70 >> __driver_probe_device+0x311/0x4b0 >> driver_probe_device+0x4e/0x150 >> __device_attach_driver+0x1cc/0x2a0 >> bus_for_each_drv+0x156/0x1d0 >> __device_attach+0x23f/0x3a0 >> page_owner free stack trace missing >> >> Memory state around the buggy address: >> ffff888018f0fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> ffff888018f0fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>> ffff888018f0fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fe fe fe >> ^ >> ffff888018f0fe00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >> ffff888018f0fe80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >> ================================================================== >> >> Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> >> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> >> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > > Can someone review this? Will have to see the bigger picture. Probably have time to do that later today. What's the deadline? ;-) Regards, Arend > -- > https://patchwork.kernel.org/project/linux-wireless/patch/20221116145821.544266-1-linuxlovemin@yonsei.ac.kr/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Arend Van Spriel <arend.vanspriel@broadcom.com> writes: > On December 22, 2022 4:55:31 PM Kalle Valo <kvalo@kernel.org> wrote: > >> Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: >> >>> This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in >>> cfg80211_find_elem_match() called from brcmf_inform_single_bss() when >>> the offset and length values of information elements provided by the >>> device exceed the boundary of the escan buffer that contains information >>> elements. The patch adds a check that makes the function return -EINVAL >>> if that is the case. Note that the negative return is handled by the >>> caller, brcmf_inform_bss(). >>> >>> Found by a modified version of syzkaller. >>> >>> ================================================================== >>> BUG: KASAN: slab-out-of-bounds in cfg80211_find_elem_match+0x164/0x180 >>> Read of size 1 at addr ffff888018f0fde9 by task kworker/0:2/1896 >>> >>> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G O 5.14.0+ #139 >>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >>> rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 >>> Workqueue: events brcmf_fweh_event_worker >>> Call Trace: >>> dump_stack_lvl+0x8e/0xd1 >>> print_address_description.constprop.0.cold+0x93/0x334 >>> ? cfg80211_find_elem_match+0x164/0x180 >>> kasan_report.cold+0x79/0xd5 >>> ? cfg80211_find_elem_match+0x164/0x180 >>> cfg80211_find_elem_match+0x164/0x180 >>> cfg80211_get_bss_channel+0x69/0x320 >>> cfg80211_inform_single_bss_data+0x1a6/0x1060 >>> ? cfg80211_bss_update+0x1e20/0x1e20 >>> ? rcu_read_lock_sched_held+0xa1/0xd0 >>> ? rcu_read_lock_bh_held+0xb0/0xb0 >>> ? find_held_lock+0x2d/0x110 >>> ? cfg80211_inform_bss_data+0xcb/0x160 >>> cfg80211_inform_bss_data+0xcb/0x160 >>> ? cfg80211_parse_mbssid_data+0x1540/0x1540 >>> ? kvm_clock_get_cycles+0x14/0x20 >>> ? ktime_get_with_offset+0x2b9/0x450 >>> brcmf_inform_single_bss+0x36d/0x4d0 >>> ? brcmf_notify_mic_status+0xb0/0xb0 >>> ? __lock_acquire+0x181f/0x5790 >>> ? brcmf_p2p_cancel_remain_on_channel+0x30/0x30 >>> brcmf_inform_bss+0x131/0x210 >>> brcmf_cfg80211_escan_handler+0x779/0xd20 >>> ? rcu_read_lock_bh_held+0xb0/0xb0 >>> ? lock_acquire+0x19d/0x4e0 >>> ? find_held_lock+0x2d/0x110 >>> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >>> ? brcmf_fweh_event_worker+0x249/0xc00 >>> ? mark_held_locks+0x9f/0xe0 >>> ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0 >>> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >>> brcmf_fweh_call_event_handler.isra.0+0x90/0x100 >>> brcmf_fweh_event_worker+0x117/0xc00 >>> ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 >>> ? rcu_read_lock_sched_held+0xa1/0xd0 >>> ? rcu_read_lock_bh_held+0xb0/0xb0 >>> ? lockdep_hardirqs_on_prepare+0x273/0x3e0 >>> process_one_work+0x92b/0x1460 >>> ? pwq_dec_nr_in_flight+0x330/0x330 >>> ? rwlock_bug.part.0+0x90/0x90 >>> worker_thread+0x95/0xe00 >>> ? __kthread_parkme+0x115/0x1e0 >>> ? process_one_work+0x1460/0x1460 >>> kthread+0x3a1/0x480 >>> ? set_kthread_struct+0x120/0x120 >>> ret_from_fork+0x1f/0x30 >>> >>> The buggy address belongs to the page: >>> page:ffffea000063c000 refcount:1 mapcount:0 >>> mapping:0000000000000000 index:0x0 pfn:0x18f00 >>> head:ffffea000063c000 order:4 compound_mapcount:0 compound_pincount:0 >>> flags: 0x100000000010000(head|node=0|zone=1) >>> raw: 0100000000010000 0000000000000000 dead000000000122 0000000000000000 >>> raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 >>> page dumped because: kasan: bad access detected >>> page_owner tracks the page as allocated >>> page last allocated via order 4, migratetype Unmovable, gfp_mask >>> 0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), pid 1896, ts >>> 44510886600, free_ts 0 >>> prep_new_page+0x1aa/0x240 >>> get_page_from_freelist+0x159a/0x27c0 >>> __alloc_pages+0x2da/0x6a0 >>> alloc_pages+0xec/0x1e0 >>> kmalloc_order+0x39/0xf0 >>> kmalloc_order_trace+0x19/0x120 >>> brcmf_cfg80211_attach+0x5c9/0x3fd0 >>> brcmf_attach+0x389/0xd40 >>> brcmf_usb_probe+0x12de/0x1690 >>> usb_probe_interface+0x2aa/0x760 >>> really_probe+0x205/0xb70 >>> __driver_probe_device+0x311/0x4b0 >>> driver_probe_device+0x4e/0x150 >>> __device_attach_driver+0x1cc/0x2a0 >>> bus_for_each_drv+0x156/0x1d0 >>> __device_attach+0x23f/0x3a0 >>> page_owner free stack trace missing >>> >>> Memory state around the buggy address: >>> ffff888018f0fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>> ffff888018f0fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>>> ffff888018f0fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fe fe fe >>> ^ >>> ffff888018f0fe00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >>> ffff888018f0fe80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >>> ================================================================== >>> >>> Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> >>> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> >>> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> >>> Reported-by: kernel test robot <lkp@intel.com> >>> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> >> >> Can someone review this? > > Will have to see the bigger picture. Probably have time to do that > later today. Thanks, and no rush. > What's the deadline? ;-) After looking at the crystall ball[1] I would say around February 5th to get this to v6.3 via -next ;) [1] https://phb-crystal-ball.sipsolutions.net/
Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: > This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in > cfg80211_find_elem_match() called from brcmf_inform_single_bss() when > the offset and length values of information elements provided by the > device exceed the boundary of the escan buffer that contains information > elements. The patch adds a check that makes the function return -EINVAL > if that is the case. Note that the negative return is handled by the > caller, brcmf_inform_bss(). > > Found by a modified version of syzkaller. > > ================================================================== > BUG: KASAN: slab-out-of-bounds in cfg80211_find_elem_match+0x164/0x180 > Read of size 1 at addr ffff888018f0fde9 by task kworker/0:2/1896 > > CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G O 5.14.0+ #139 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 > Workqueue: events brcmf_fweh_event_worker > Call Trace: > dump_stack_lvl+0x8e/0xd1 > print_address_description.constprop.0.cold+0x93/0x334 > ? cfg80211_find_elem_match+0x164/0x180 > kasan_report.cold+0x79/0xd5 > ? cfg80211_find_elem_match+0x164/0x180 > cfg80211_find_elem_match+0x164/0x180 > cfg80211_get_bss_channel+0x69/0x320 > cfg80211_inform_single_bss_data+0x1a6/0x1060 > ? cfg80211_bss_update+0x1e20/0x1e20 > ? rcu_read_lock_sched_held+0xa1/0xd0 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? find_held_lock+0x2d/0x110 > ? cfg80211_inform_bss_data+0xcb/0x160 > cfg80211_inform_bss_data+0xcb/0x160 > ? cfg80211_parse_mbssid_data+0x1540/0x1540 > ? kvm_clock_get_cycles+0x14/0x20 > ? ktime_get_with_offset+0x2b9/0x450 > brcmf_inform_single_bss+0x36d/0x4d0 > ? brcmf_notify_mic_status+0xb0/0xb0 > ? __lock_acquire+0x181f/0x5790 > ? brcmf_p2p_cancel_remain_on_channel+0x30/0x30 > brcmf_inform_bss+0x131/0x210 > brcmf_cfg80211_escan_handler+0x779/0xd20 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? lock_acquire+0x19d/0x4e0 > ? find_held_lock+0x2d/0x110 > ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 > ? brcmf_fweh_event_worker+0x249/0xc00 > ? mark_held_locks+0x9f/0xe0 > ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0 > ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 > brcmf_fweh_call_event_handler.isra.0+0x90/0x100 > brcmf_fweh_event_worker+0x117/0xc00 > ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 > ? rcu_read_lock_sched_held+0xa1/0xd0 > ? rcu_read_lock_bh_held+0xb0/0xb0 > ? lockdep_hardirqs_on_prepare+0x273/0x3e0 > process_one_work+0x92b/0x1460 > ? pwq_dec_nr_in_flight+0x330/0x330 > ? rwlock_bug.part.0+0x90/0x90 > worker_thread+0x95/0xe00 > ? __kthread_parkme+0x115/0x1e0 > ? process_one_work+0x1460/0x1460 > kthread+0x3a1/0x480 > ? set_kthread_struct+0x120/0x120 > ret_from_fork+0x1f/0x30 > > The buggy address belongs to the page: > page:ffffea000063c000 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x18f00 > head:ffffea000063c000 order:4 compound_mapcount:0 compound_pincount:0 > flags: 0x100000000010000(head|node=0|zone=1) > raw: 0100000000010000 0000000000000000 dead000000000122 0000000000000000 > raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 > page dumped because: kasan: bad access detected > page_owner tracks the page as allocated > page last allocated via order 4, migratetype Unmovable, gfp_mask 0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), pid 1896, ts 44510886600, free_ts 0 > prep_new_page+0x1aa/0x240 > get_page_from_freelist+0x159a/0x27c0 > __alloc_pages+0x2da/0x6a0 > alloc_pages+0xec/0x1e0 > kmalloc_order+0x39/0xf0 > kmalloc_order_trace+0x19/0x120 > brcmf_cfg80211_attach+0x5c9/0x3fd0 > brcmf_attach+0x389/0xd40 > brcmf_usb_probe+0x12de/0x1690 > usb_probe_interface+0x2aa/0x760 > really_probe+0x205/0xb70 > __driver_probe_device+0x311/0x4b0 > driver_probe_device+0x4e/0x150 > __device_attach_driver+0x1cc/0x2a0 > bus_for_each_drv+0x156/0x1d0 > __device_attach+0x23f/0x3a0 > page_owner free stack trace missing > > Memory state around the buggy address: > ffff888018f0fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > ffff888018f0fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > >ffff888018f0fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fe fe fe > ^ > ffff888018f0fe00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe > ffff888018f0fe80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe > ================================================================== > > Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> > Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> > Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> Arend, can you take a look at this? I don't dare to take it unless you have checked it.
Ok. Will check. Regards, Arend On February 27, 2023 4:18:32 PM Kalle Valo <kvalo@kernel.org> wrote: > Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: > >> This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in >> cfg80211_find_elem_match() called from brcmf_inform_single_bss() when >> the offset and length values of information elements provided by the >> device exceed the boundary of the escan buffer that contains information >> elements. The patch adds a check that makes the function return -EINVAL >> if that is the case. Note that the negative return is handled by the >> caller, brcmf_inform_bss(). >> >> Found by a modified version of syzkaller. >> >> ================================================================== >> BUG: KASAN: slab-out-of-bounds in cfg80211_find_elem_match+0x164/0x180 >> Read of size 1 at addr ffff888018f0fde9 by task kworker/0:2/1896 >> >> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G O 5.14.0+ #139 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS >> rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014 >> Workqueue: events brcmf_fweh_event_worker >> Call Trace: >> dump_stack_lvl+0x8e/0xd1 >> print_address_description.constprop.0.cold+0x93/0x334 >> ? cfg80211_find_elem_match+0x164/0x180 >> kasan_report.cold+0x79/0xd5 >> ? cfg80211_find_elem_match+0x164/0x180 >> cfg80211_find_elem_match+0x164/0x180 >> cfg80211_get_bss_channel+0x69/0x320 >> cfg80211_inform_single_bss_data+0x1a6/0x1060 >> ? cfg80211_bss_update+0x1e20/0x1e20 >> ? rcu_read_lock_sched_held+0xa1/0xd0 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? find_held_lock+0x2d/0x110 >> ? cfg80211_inform_bss_data+0xcb/0x160 >> cfg80211_inform_bss_data+0xcb/0x160 >> ? cfg80211_parse_mbssid_data+0x1540/0x1540 >> ? kvm_clock_get_cycles+0x14/0x20 >> ? ktime_get_with_offset+0x2b9/0x450 >> brcmf_inform_single_bss+0x36d/0x4d0 >> ? brcmf_notify_mic_status+0xb0/0xb0 >> ? __lock_acquire+0x181f/0x5790 >> ? brcmf_p2p_cancel_remain_on_channel+0x30/0x30 >> brcmf_inform_bss+0x131/0x210 >> brcmf_cfg80211_escan_handler+0x779/0xd20 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? lock_acquire+0x19d/0x4e0 >> ? find_held_lock+0x2d/0x110 >> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >> ? brcmf_fweh_event_worker+0x249/0xc00 >> ? mark_held_locks+0x9f/0xe0 >> ? lockdep_hardirqs_on_prepare+0x3e0/0x3e0 >> ? brcmf_cfg80211_escan_timeout_worker+0x60/0x60 >> brcmf_fweh_call_event_handler.isra.0+0x90/0x100 >> brcmf_fweh_event_worker+0x117/0xc00 >> ? brcmf_fweh_call_event_handler.isra.0+0x100/0x100 >> ? rcu_read_lock_sched_held+0xa1/0xd0 >> ? rcu_read_lock_bh_held+0xb0/0xb0 >> ? lockdep_hardirqs_on_prepare+0x273/0x3e0 >> process_one_work+0x92b/0x1460 >> ? pwq_dec_nr_in_flight+0x330/0x330 >> ? rwlock_bug.part.0+0x90/0x90 >> worker_thread+0x95/0xe00 >> ? __kthread_parkme+0x115/0x1e0 >> ? process_one_work+0x1460/0x1460 >> kthread+0x3a1/0x480 >> ? set_kthread_struct+0x120/0x120 >> ret_from_fork+0x1f/0x30 >> >> The buggy address belongs to the page: >> page:ffffea000063c000 refcount:1 mapcount:0 mapping:0000000000000000 >> index:0x0 pfn:0x18f00 >> head:ffffea000063c000 order:4 compound_mapcount:0 compound_pincount:0 >> flags: 0x100000000010000(head|node=0|zone=1) >> raw: 0100000000010000 0000000000000000 dead000000000122 0000000000000000 >> raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000 >> page dumped because: kasan: bad access detected >> page_owner tracks the page as allocated >> page last allocated via order 4, migratetype Unmovable, gfp_mask >> 0x40dc0(GFP_KERNEL|__GFP_COMP|__GFP_ZERO), pid 1896, ts 44510886600, free_ts 0 >> prep_new_page+0x1aa/0x240 >> get_page_from_freelist+0x159a/0x27c0 >> __alloc_pages+0x2da/0x6a0 >> alloc_pages+0xec/0x1e0 >> kmalloc_order+0x39/0xf0 >> kmalloc_order_trace+0x19/0x120 >> brcmf_cfg80211_attach+0x5c9/0x3fd0 >> brcmf_attach+0x389/0xd40 >> brcmf_usb_probe+0x12de/0x1690 >> usb_probe_interface+0x2aa/0x760 >> really_probe+0x205/0xb70 >> __driver_probe_device+0x311/0x4b0 >> driver_probe_device+0x4e/0x150 >> __device_attach_driver+0x1cc/0x2a0 >> bus_for_each_drv+0x156/0x1d0 >> __device_attach+0x23f/0x3a0 >> page_owner free stack trace missing >> >> Memory state around the buggy address: >> ffff888018f0fc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> ffff888018f0fd00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >> >ffff888018f0fd80: 00 00 00 00 00 00 00 00 00 00 00 00 00 fe fe fe >> ^ >> ffff888018f0fe00: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >> ffff888018f0fe80: fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe fe >> ================================================================== >> >> Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> >> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> >> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> >> Reported-by: kernel test robot <lkp@intel.com> >> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > > Arend, can you take a look at this? I don't dare to take it unless you have > checked it. > > -- > https://patchwork.kernel.org/project/linux-wireless/patch/20221116145821.544266-1-linuxlovemin@yonsei.ac.kr/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 11/16/2022 3:58 PM, Minsuk Kang wrote: > This patch fixes a slab-out-of-bounds read in brcmfmac that occurs in > cfg80211_find_elem_match() called from brcmf_inform_single_bss() when > the offset and length values of information elements provided by the > device exceed the boundary of the escan buffer that contains information > elements. The patch adds a check that makes the function return -EINVAL > if that is the case. Note that the negative return is handled by the > caller, brcmf_inform_bss(). [...] Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com> > Reported-by: Dokyung Song <dokyungs@yonsei.ac.kr> > Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr> > Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > Reported-by: kernel test robot <lkp@intel.com> > Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr> > --- > v1->v2: Use the correct format for size_t in bphy_err() > > .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > index ae9507dec74a..2148027eb42b 100644 > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > @@ -3298,6 +3298,13 @@ static s32 brcmf_inform_single_bss(struct brcmf_cfg80211_info *cfg, > notify_ielen = le32_to_cpu(bi->ie_length); > bss_data.signal = (s16)le16_to_cpu(bi->RSSI) * 100; > > + if ((unsigned long)notify_ie + notify_ielen - > + (unsigned long)cfg->escan_info.escan_buf > BRCMF_ESCAN_BUF_SIZE) { > + bphy_err(drvr, "Invalid information element offset: %u, length: %zu\n", > + le16_to_cpu(bi->ie_offset), notify_ielen); > + return -EINVAL; > + } > + Maybe this works, but it was not immediately obvious to me. Also this seems late in processing the scan results. Better catch it early and check the ie_offset and ie_length values in brcmf_cfg80211_escan_handler() when processing the partial result event. It already checks bi->length there so add a check there: bss_ie_offset = le16_to_cpu(bi->ie_offset); bss_ie_length = le16_to_cpu(bi->ie_length); if (bi->ie_offset + bi->ie_length > bi->length) { bphy_err(drvr, "Ignoring invalid information element offset: %u, length: %zu\n" bss_ie_offset, bss_ie_length); goto exit; } Regards, Arend
On 2/27/2023 7:59 PM, Arend Van Spriel wrote: > Ok. Will check. > > Regards, > Arend oops. sorry for top posting that :-s > On February 27, 2023 4:18:32 PM Kalle Valo <kvalo@kernel.org> wrote: > >> Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote: >>
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c index ae9507dec74a..2148027eb42b 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c @@ -3298,6 +3298,13 @@ static s32 brcmf_inform_single_bss(struct brcmf_cfg80211_info *cfg, notify_ielen = le32_to_cpu(bi->ie_length); bss_data.signal = (s16)le16_to_cpu(bi->RSSI) * 100; + if ((unsigned long)notify_ie + notify_ielen - + (unsigned long)cfg->escan_info.escan_buf > BRCMF_ESCAN_BUF_SIZE) { + bphy_err(drvr, "Invalid information element offset: %u, length: %zu\n", + le16_to_cpu(bi->ie_offset), notify_ielen); + return -EINVAL; + } + brcmf_dbg(CONN, "bssid: %pM\n", bi->BSSID); brcmf_dbg(CONN, "Channel: %d(%d)\n", channel, freq); brcmf_dbg(CONN, "Capability: %X\n", notify_capability);