2022-11-11 07:59:45

by Minsuk Kang

[permalink] [raw]
Subject: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads

This patch fixes slab-out-of-bounds reads in brcmfmac that occur in
brcmf_construct_chaninfo() and brcmf_enable_bw40_2g() when the count
value of channel specifications provided by the device is greater than
the length of 'list->element[]', decided by the size of the 'list'
allocated with kzalloc(). The patch adds checks that make the functions
free the buffer and return -EINVAL if that is the case. Note that the
negative return is handled by the caller, brcmf_setup_wiphybands() or
brcmf_cfg80211_attach().

Found by a modified version of syzkaller.

Crash Report from brcmf_construct_chaninfo():
==================================================================
BUG: KASAN: slab-out-of-bounds in brcmf_setup_wiphybands+0x1238/0x1430
Read of size 4 at addr ffff888115f24600 by task kworker/0:2/1896

CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Workqueue: usb_hub_wq hub_event
Call Trace:
dump_stack_lvl+0x57/0x7d
print_address_description.constprop.0.cold+0x93/0x334
kasan_report.cold+0x83/0xdf
brcmf_setup_wiphybands+0x1238/0x1430
brcmf_cfg80211_attach+0x2118/0x3fd0
brcmf_attach+0x389/0xd40
brcmf_usb_probe+0x12de/0x1690
usb_probe_interface+0x25f/0x710
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_set_configuration+0x984/0x1770
usb_generic_driver_probe+0x69/0x90
usb_probe_device+0x9c/0x220
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_new_device.cold+0x463/0xf66
hub_event+0x10d5/0x3330
process_one_work+0x873/0x13e0
worker_thread+0x8b/0xd10
kthread+0x379/0x450
ret_from_fork+0x1f/0x30

Allocated by task 1896:
kasan_save_stack+0x1b/0x40
__kasan_kmalloc+0x7c/0x90
kmem_cache_alloc_trace+0x19e/0x330
brcmf_setup_wiphybands+0x290/0x1430
brcmf_cfg80211_attach+0x2118/0x3fd0
brcmf_attach+0x389/0xd40
brcmf_usb_probe+0x12de/0x1690
usb_probe_interface+0x25f/0x710
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_set_configuration+0x984/0x1770
usb_generic_driver_probe+0x69/0x90
usb_probe_device+0x9c/0x220
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_new_device.cold+0x463/0xf66
hub_event+0x10d5/0x3330
process_one_work+0x873/0x13e0
worker_thread+0x8b/0xd10
kthread+0x379/0x450
ret_from_fork+0x1f/0x30

The buggy address belongs to the object at ffff888115f24000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 1536 bytes inside of
2048-byte region [ffff888115f24000, ffff888115f24800)

Memory state around the buggy address:
ffff888115f24500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888115f24580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888115f24600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff888115f24680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888115f24700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

Crash Report from brcmf_enable_bw40_2g():
==================================================================
BUG: KASAN: slab-out-of-bounds in brcmf_cfg80211_attach+0x3d11/0x3fd0
Read of size 4 at addr ffff888103787600 by task kworker/0:2/1896

CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
Workqueue: usb_hub_wq hub_event
Call Trace:
dump_stack_lvl+0x57/0x7d
print_address_description.constprop.0.cold+0x93/0x334
kasan_report.cold+0x83/0xdf
brcmf_cfg80211_attach+0x3d11/0x3fd0
brcmf_attach+0x389/0xd40
brcmf_usb_probe+0x12de/0x1690
usb_probe_interface+0x25f/0x710
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_set_configuration+0x984/0x1770
usb_generic_driver_probe+0x69/0x90
usb_probe_device+0x9c/0x220
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_new_device.cold+0x463/0xf66
hub_event+0x10d5/0x3330
process_one_work+0x873/0x13e0
worker_thread+0x8b/0xd10
kthread+0x379/0x450
ret_from_fork+0x1f/0x30

Allocated by task 1896:
kasan_save_stack+0x1b/0x40
__kasan_kmalloc+0x7c/0x90
kmem_cache_alloc_trace+0x19e/0x330
brcmf_cfg80211_attach+0x3302/0x3fd0
brcmf_attach+0x389/0xd40
brcmf_usb_probe+0x12de/0x1690
usb_probe_interface+0x25f/0x710
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_set_configuration+0x984/0x1770
usb_generic_driver_probe+0x69/0x90
usb_probe_device+0x9c/0x220
really_probe+0x1be/0xa90
__driver_probe_device+0x2ab/0x460
driver_probe_device+0x49/0x120
__device_attach_driver+0x18a/0x250
bus_for_each_drv+0x123/0x1a0
__device_attach+0x207/0x330
bus_probe_device+0x1a2/0x260
device_add+0xa61/0x1ce0
usb_new_device.cold+0x463/0xf66
hub_event+0x10d5/0x3330
process_one_work+0x873/0x13e0
worker_thread+0x8b/0xd10
kthread+0x379/0x450
ret_from_fork+0x1f/0x30

The buggy address belongs to the object at ffff888103787000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 1536 bytes inside of
2048-byte region [ffff888103787000, ffff888103787800)

Memory state around the buggy address:
ffff888103787500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff888103787580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff888103787600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff888103787680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff888103787700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

Reported-by: Dokyung Song <[email protected]>
Reported-by: Jisoo Jang <[email protected]>
Reported-by: Minsuk Kang <[email protected]>
Signed-off-by: Minsuk Kang <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index ae9507dec74a..3a1c0743e19c 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6840,6 +6840,13 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
band->channels[i].flags = IEEE80211_CHAN_DISABLED;

total = le32_to_cpu(list->count);
+ if (total > BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1) {
+ bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
+ total);
+ err = -EINVAL;
+ goto fail_pbuf;
+ }
+
for (i = 0; i < total; i++) {
ch.chspec = (u16)le32_to_cpu(list->element[i]);
cfg->d11inf.decchspec(&ch);
@@ -6985,6 +6992,13 @@ static int brcmf_enable_bw40_2g(struct brcmf_cfg80211_info *cfg)
band = cfg_to_wiphy(cfg)->bands[NL80211_BAND_2GHZ];
list = (struct brcmf_chanspec_list *)pbuf;
num_chan = le32_to_cpu(list->count);
+ if (num_chan > BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1) {
+ bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
+ num_chan);
+ kfree(pbuf);
+ return -EINVAL;
+ }
+
for (i = 0; i < num_chan; i++) {
ch.chspec = (u16)le32_to_cpu(list->element[i]);
cfg->d11inf.decchspec(&ch);
--
2.25.1



2022-11-16 09:32:13

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads

Minsuk Kang <[email protected]> wrote:

> This patch fixes slab-out-of-bounds reads in brcmfmac that occur in
> brcmf_construct_chaninfo() and brcmf_enable_bw40_2g() when the count
> value of channel specifications provided by the device is greater than
> the length of 'list->element[]', decided by the size of the 'list'
> allocated with kzalloc(). The patch adds checks that make the functions
> free the buffer and return -EINVAL if that is the case. Note that the
> negative return is handled by the caller, brcmf_setup_wiphybands() or
> brcmf_cfg80211_attach().
>
> Found by a modified version of syzkaller.
>
> Crash Report from brcmf_construct_chaninfo():
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in brcmf_setup_wiphybands+0x1238/0x1430
> Read of size 4 at addr ffff888115f24600 by task kworker/0:2/1896
>
> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> dump_stack_lvl+0x57/0x7d
> print_address_description.constprop.0.cold+0x93/0x334
> kasan_report.cold+0x83/0xdf
> brcmf_setup_wiphybands+0x1238/0x1430
> brcmf_cfg80211_attach+0x2118/0x3fd0
> brcmf_attach+0x389/0xd40
> brcmf_usb_probe+0x12de/0x1690
> usb_probe_interface+0x25f/0x710
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_set_configuration+0x984/0x1770
> usb_generic_driver_probe+0x69/0x90
> usb_probe_device+0x9c/0x220
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_new_device.cold+0x463/0xf66
> hub_event+0x10d5/0x3330
> process_one_work+0x873/0x13e0
> worker_thread+0x8b/0xd10
> kthread+0x379/0x450
> ret_from_fork+0x1f/0x30
>
> Allocated by task 1896:
> kasan_save_stack+0x1b/0x40
> __kasan_kmalloc+0x7c/0x90
> kmem_cache_alloc_trace+0x19e/0x330
> brcmf_setup_wiphybands+0x290/0x1430
> brcmf_cfg80211_attach+0x2118/0x3fd0
> brcmf_attach+0x389/0xd40
> brcmf_usb_probe+0x12de/0x1690
> usb_probe_interface+0x25f/0x710
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_set_configuration+0x984/0x1770
> usb_generic_driver_probe+0x69/0x90
> usb_probe_device+0x9c/0x220
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_new_device.cold+0x463/0xf66
> hub_event+0x10d5/0x3330
> process_one_work+0x873/0x13e0
> worker_thread+0x8b/0xd10
> kthread+0x379/0x450
> ret_from_fork+0x1f/0x30
>
> The buggy address belongs to the object at ffff888115f24000
> which belongs to the cache kmalloc-2k of size 2048
> The buggy address is located 1536 bytes inside of
> 2048-byte region [ffff888115f24000, ffff888115f24800)
>
> Memory state around the buggy address:
> ffff888115f24500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff888115f24580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff888115f24600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff888115f24680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff888115f24700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==================================================================
>
> Crash Report from brcmf_enable_bw40_2g():
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in brcmf_cfg80211_attach+0x3d11/0x3fd0
> Read of size 4 at addr ffff888103787600 by task kworker/0:2/1896
>
> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
> Workqueue: usb_hub_wq hub_event
> Call Trace:
> dump_stack_lvl+0x57/0x7d
> print_address_description.constprop.0.cold+0x93/0x334
> kasan_report.cold+0x83/0xdf
> brcmf_cfg80211_attach+0x3d11/0x3fd0
> brcmf_attach+0x389/0xd40
> brcmf_usb_probe+0x12de/0x1690
> usb_probe_interface+0x25f/0x710
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_set_configuration+0x984/0x1770
> usb_generic_driver_probe+0x69/0x90
> usb_probe_device+0x9c/0x220
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_new_device.cold+0x463/0xf66
> hub_event+0x10d5/0x3330
> process_one_work+0x873/0x13e0
> worker_thread+0x8b/0xd10
> kthread+0x379/0x450
> ret_from_fork+0x1f/0x30
>
> Allocated by task 1896:
> kasan_save_stack+0x1b/0x40
> __kasan_kmalloc+0x7c/0x90
> kmem_cache_alloc_trace+0x19e/0x330
> brcmf_cfg80211_attach+0x3302/0x3fd0
> brcmf_attach+0x389/0xd40
> brcmf_usb_probe+0x12de/0x1690
> usb_probe_interface+0x25f/0x710
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_set_configuration+0x984/0x1770
> usb_generic_driver_probe+0x69/0x90
> usb_probe_device+0x9c/0x220
> really_probe+0x1be/0xa90
> __driver_probe_device+0x2ab/0x460
> driver_probe_device+0x49/0x120
> __device_attach_driver+0x18a/0x250
> bus_for_each_drv+0x123/0x1a0
> __device_attach+0x207/0x330
> bus_probe_device+0x1a2/0x260
> device_add+0xa61/0x1ce0
> usb_new_device.cold+0x463/0xf66
> hub_event+0x10d5/0x3330
> process_one_work+0x873/0x13e0
> worker_thread+0x8b/0xd10
> kthread+0x379/0x450
> ret_from_fork+0x1f/0x30
>
> The buggy address belongs to the object at ffff888103787000
> which belongs to the cache kmalloc-2k of size 2048
> The buggy address is located 1536 bytes inside of
> 2048-byte region [ffff888103787000, ffff888103787800)
>
> Memory state around the buggy address:
> ffff888103787500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff888103787580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffff888103787600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff888103787680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff888103787700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==================================================================
>
> Reported-by: Dokyung Song <[email protected]>
> Reported-by: Jisoo Jang <[email protected]>
> Reported-by: Minsuk Kang <[email protected]>
> Signed-off-by: Minsuk Kang <[email protected]>

Can someone review this, please?

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

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


2022-11-16 11:49:26

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads

On 11/16/2022 10:23 AM, Kalle Valo wrote:
> Minsuk Kang <[email protected]> wrote:
>
>> This patch fixes slab-out-of-bounds reads in brcmfmac that occur in
>> brcmf_construct_chaninfo() and brcmf_enable_bw40_2g() when the count
>> value of channel specifications provided by the device is greater than
>> the length of 'list->element[]', decided by the size of the 'list'
>> allocated with kzalloc(). The patch adds checks that make the functions
>> free the buffer and return -EINVAL if that is the case. Note that the
>> negative return is handled by the caller, brcmf_setup_wiphybands() or
>> brcmf_cfg80211_attach().
>>
>> Found by a modified version of syzkaller.
>>
>> Crash Report from brcmf_construct_chaninfo():
>> ==================================================================
>> BUG: KASAN: slab-out-of-bounds in brcmf_setup_wiphybands+0x1238/0x1430
>> Read of size 4 at addr ffff888115f24600 by task kworker/0:2/1896
>>
>> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
>> Workqueue: usb_hub_wq hub_event
>> Call Trace:
>> dump_stack_lvl+0x57/0x7d
>> print_address_description.constprop.0.cold+0x93/0x334
>> kasan_report.cold+0x83/0xdf
>> brcmf_setup_wiphybands+0x1238/0x1430
>> brcmf_cfg80211_attach+0x2118/0x3fd0
>> brcmf_attach+0x389/0xd40
>> brcmf_usb_probe+0x12de/0x1690
>> usb_probe_interface+0x25f/0x710
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_set_configuration+0x984/0x1770
>> usb_generic_driver_probe+0x69/0x90
>> usb_probe_device+0x9c/0x220
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_new_device.cold+0x463/0xf66
>> hub_event+0x10d5/0x3330
>> process_one_work+0x873/0x13e0
>> worker_thread+0x8b/0xd10
>> kthread+0x379/0x450
>> ret_from_fork+0x1f/0x30
>>
>> Allocated by task 1896:
>> kasan_save_stack+0x1b/0x40
>> __kasan_kmalloc+0x7c/0x90
>> kmem_cache_alloc_trace+0x19e/0x330
>> brcmf_setup_wiphybands+0x290/0x1430
>> brcmf_cfg80211_attach+0x2118/0x3fd0
>> brcmf_attach+0x389/0xd40
>> brcmf_usb_probe+0x12de/0x1690
>> usb_probe_interface+0x25f/0x710
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_set_configuration+0x984/0x1770
>> usb_generic_driver_probe+0x69/0x90
>> usb_probe_device+0x9c/0x220
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_new_device.cold+0x463/0xf66
>> hub_event+0x10d5/0x3330
>> process_one_work+0x873/0x13e0
>> worker_thread+0x8b/0xd10
>> kthread+0x379/0x450
>> ret_from_fork+0x1f/0x30
>>
>> The buggy address belongs to the object at ffff888115f24000
>> which belongs to the cache kmalloc-2k of size 2048
>> The buggy address is located 1536 bytes inside of
>> 2048-byte region [ffff888115f24000, ffff888115f24800)
>>
>> Memory state around the buggy address:
>> ffff888115f24500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff888115f24580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ffff888115f24600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ^
>> ffff888115f24680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff888115f24700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ==================================================================
>>
>> Crash Report from brcmf_enable_bw40_2g():
>> ==================================================================
>> BUG: KASAN: slab-out-of-bounds in brcmf_cfg80211_attach+0x3d11/0x3fd0
>> Read of size 4 at addr ffff888103787600 by task kworker/0:2/1896
>>
>> CPU: 0 PID: 1896 Comm: kworker/0:2 Tainted: G W O 5.14.0+ #132
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
>> Workqueue: usb_hub_wq hub_event
>> Call Trace:
>> dump_stack_lvl+0x57/0x7d
>> print_address_description.constprop.0.cold+0x93/0x334
>> kasan_report.cold+0x83/0xdf
>> brcmf_cfg80211_attach+0x3d11/0x3fd0
>> brcmf_attach+0x389/0xd40
>> brcmf_usb_probe+0x12de/0x1690
>> usb_probe_interface+0x25f/0x710
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_set_configuration+0x984/0x1770
>> usb_generic_driver_probe+0x69/0x90
>> usb_probe_device+0x9c/0x220
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_new_device.cold+0x463/0xf66
>> hub_event+0x10d5/0x3330
>> process_one_work+0x873/0x13e0
>> worker_thread+0x8b/0xd10
>> kthread+0x379/0x450
>> ret_from_fork+0x1f/0x30
>>
>> Allocated by task 1896:
>> kasan_save_stack+0x1b/0x40
>> __kasan_kmalloc+0x7c/0x90
>> kmem_cache_alloc_trace+0x19e/0x330
>> brcmf_cfg80211_attach+0x3302/0x3fd0
>> brcmf_attach+0x389/0xd40
>> brcmf_usb_probe+0x12de/0x1690
>> usb_probe_interface+0x25f/0x710
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_set_configuration+0x984/0x1770
>> usb_generic_driver_probe+0x69/0x90
>> usb_probe_device+0x9c/0x220
>> really_probe+0x1be/0xa90
>> __driver_probe_device+0x2ab/0x460
>> driver_probe_device+0x49/0x120
>> __device_attach_driver+0x18a/0x250
>> bus_for_each_drv+0x123/0x1a0
>> __device_attach+0x207/0x330
>> bus_probe_device+0x1a2/0x260
>> device_add+0xa61/0x1ce0
>> usb_new_device.cold+0x463/0xf66
>> hub_event+0x10d5/0x3330
>> process_one_work+0x873/0x13e0
>> worker_thread+0x8b/0xd10
>> kthread+0x379/0x450
>> ret_from_fork+0x1f/0x30
>>
>> The buggy address belongs to the object at ffff888103787000
>> which belongs to the cache kmalloc-2k of size 2048
>> The buggy address is located 1536 bytes inside of
>> 2048-byte region [ffff888103787000, ffff888103787800)
>>
>> Memory state around the buggy address:
>> ffff888103787500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ffff888103787580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ffff888103787600: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ^
>> ffff888103787680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ffff888103787700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>> ==================================================================
>>
>> Reported-by: Dokyung Song <[email protected]>
>> Reported-by: Jisoo Jang <[email protected]>
>> Reported-by: Minsuk Kang <[email protected]>
>> Signed-off-by: Minsuk Kang <[email protected]>
>
> Can someone review this, please?

Missed this one, but I will have a look now.

Regards,
Arend


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

2022-11-16 12:08:18

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads

On 11/11/2022 8:53 AM, Minsuk Kang wrote:
> This patch fixes slab-out-of-bounds reads in brcmfmac that occur in
> brcmf_construct_chaninfo() and brcmf_enable_bw40_2g() when the count
> value of channel specifications provided by the device is greater than
> the length of 'list->element[]', decided by the size of the 'list'
> allocated with kzalloc(). The patch adds checks that make the functions
> free the buffer and return -EINVAL if that is the case. Note that the
> negative return is handled by the caller, brcmf_setup_wiphybands() or
> brcmf_cfg80211_attach().
>
> Found by a modified version of syzkaller.

[snip]

> Reported-by: Dokyung Song <[email protected]>
> Reported-by: Jisoo Jang <[email protected]>
> Reported-by: Minsuk Kang <[email protected]>
> Signed-off-by: Minsuk Kang <[email protected]>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index ae9507dec74a..3a1c0743e19c 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -6840,6 +6840,13 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
> band->channels[i].flags = IEEE80211_CHAN_DISABLED;
>
> total = le32_to_cpu(list->count);
> + if (total > BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1) {

Please add and use macro definition here:

#define BRCMF_MAX_CHANSPEC_LIST (BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1)

> + bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
> + total);
> + err = -EINVAL;
> + goto fail_pbuf;
> + }
> +
> for (i = 0; i < total; i++) {
> ch.chspec = (u16)le32_to_cpu(list->element[i]);
> cfg->d11inf.decchspec(&ch);
> @@ -6985,6 +6992,13 @@ static int brcmf_enable_bw40_2g(struct brcmf_cfg80211_info *cfg)
> band = cfg_to_wiphy(cfg)->bands[NL80211_BAND_2GHZ];
> list = (struct brcmf_chanspec_list *)pbuf;
> num_chan = le32_to_cpu(list->count);
> + if (num_chan > BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1) {

...and here.

> + bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
> + num_chan);
> + kfree(pbuf);
> + return -EINVAL;
> + }
> +
> for (i = 0; i < num_chan; i++) {
> ch.chspec = (u16)le32_to_cpu(list->element[i]);
> cfg->d11inf.decchspec(&ch);


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