diff mbox series

wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads

Message ID 20221111075346.136376-1-linuxlovemin@yonsei.ac.kr
State New
Headers show
Series wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads | expand

Commit Message

Minsuk Kang Nov. 11, 2022, 7:53 a.m. UTC
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 <dokyungs@yonsei.ac.kr>
Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Kalle Valo Nov. 16, 2022, 9:23 a.m. UTC | #1
Minsuk Kang <linuxlovemin@yonsei.ac.kr> 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 <dokyungs@yonsei.ac.kr>
> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>

Can someone review this, please?
Arend van Spriel Nov. 16, 2022, 11:27 a.m. UTC | #2
On 11/16/2022 10:23 AM, Kalle Valo wrote:
> Minsuk Kang <linuxlovemin@yonsei.ac.kr> 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 <dokyungs@yonsei.ac.kr>
>> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
>> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
>> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> 
> Can someone review this, please?

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

Regards,
Arend
Arend van Spriel Nov. 16, 2022, 11:58 a.m. UTC | #3
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 <dokyungs@yonsei.ac.kr>
> Reported-by: Jisoo Jang <jisoo.jang@yonsei.ac.kr>
> Reported-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> Signed-off-by: Minsuk Kang <linuxlovemin@yonsei.ac.kr>
> ---
>   .../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);
diff mbox series

Patch

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);