2022-11-16 15:00:59

by Minsuk Kang

[permalink] [raw]
Subject: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

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 <[email protected]>
Reported-by: Jisoo Jang <[email protected]>
Reported-by: Minsuk Kang <[email protected]>
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Minsuk Kang <[email protected]>
---
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;
+ }
+
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);
--
2.25.1



2022-12-22 15:56:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

Minsuk Kang <[email protected]> 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 <[email protected]>
> Reported-by: Jisoo Jang <[email protected]>
> Reported-by: Minsuk Kang <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Minsuk Kang <[email protected]>

Can someone review this?

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2022-12-22 16:20:14

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

On December 22, 2022 4:55:31 PM Kalle Valo <[email protected]> wrote:

> Minsuk Kang <[email protected]> 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 <[email protected]>
>> Reported-by: Jisoo Jang <[email protected]>
>> Reported-by: Minsuk Kang <[email protected]>
>> Reported-by: kernel test robot <[email protected]>
>> Signed-off-by: Minsuk Kang <[email protected]>
>
> 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/[email protected]/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches




Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2022-12-22 16:28:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

Arend Van Spriel <[email protected]> writes:

> On December 22, 2022 4:55:31 PM Kalle Valo <[email protected]> wrote:
>
>> Minsuk Kang <[email protected]> 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 <[email protected]>
>>> Reported-by: Jisoo Jang <[email protected]>
>>> Reported-by: Minsuk Kang <[email protected]>
>>> Reported-by: kernel test robot <[email protected]>
>>> Signed-off-by: Minsuk Kang <[email protected]>
>>
>> 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/

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2023-02-27 15:18:35

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

Minsuk Kang <[email protected]> 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 <[email protected]>
> Reported-by: Jisoo Jang <[email protected]>
> Reported-by: Minsuk Kang <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Minsuk Kang <[email protected]>

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/[email protected]/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


2023-02-27 18:59:23

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

Ok. Will check.

Regards,
Arend

On February 27, 2023 4:18:32 PM Kalle Valo <[email protected]> wrote:

> Minsuk Kang <[email protected]> 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 <[email protected]>
>> Reported-by: Jisoo Jang <[email protected]>
>> Reported-by: Minsuk Kang <[email protected]>
>> Reported-by: kernel test robot <[email protected]>
>> Signed-off-by: Minsuk Kang <[email protected]>
>
> 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/[email protected]/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches




Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature

2023-02-27 19:59:40

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

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 <[email protected]>
> Reported-by: Dokyung Song <[email protected]>
> Reported-by: Jisoo Jang <[email protected]>
> Reported-by: Minsuk Kang <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Minsuk Kang <[email protected]>
> ---
> 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

2023-02-27 20:02:24

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH v2] wifi: brcmfmac: Fix potential slab-out-of-bounds read in brcmf_inform_single_bss()

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 <[email protected]> wrote:
>
>> Minsuk Kang <[email protected]> wrote:
>>


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature