All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads
@ 2022-11-11  7:53 Minsuk Kang
  2022-11-16  9:23 ` Kalle Valo
  2022-11-16 11:58 ` Arend van Spriel
  0 siblings, 2 replies; 4+ messages in thread
From: Minsuk Kang @ 2022-11-11  7:53 UTC (permalink / raw)
  To: linux-wireless, aspriel; +Cc: dokyungs, jisoo.jang, Minsuk Kang

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(+)

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads
  2022-11-11  7:53 [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads Minsuk Kang
@ 2022-11-16  9:23 ` Kalle Valo
  2022-11-16 11:27   ` Arend van Spriel
  2022-11-16 11:58 ` Arend van Spriel
  1 sibling, 1 reply; 4+ messages in thread
From: Kalle Valo @ 2022-11-16  9:23 UTC (permalink / raw)
  To: Minsuk Kang; +Cc: linux-wireless, aspriel, dokyungs, jisoo.jang, Minsuk Kang

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?

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20221111075346.136376-1-linuxlovemin@yonsei.ac.kr/

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads
  2022-11-16  9:23 ` Kalle Valo
@ 2022-11-16 11:27   ` Arend van Spriel
  0 siblings, 0 replies; 4+ messages in thread
From: Arend van Spriel @ 2022-11-16 11:27 UTC (permalink / raw)
  To: Kalle Valo, Minsuk Kang; +Cc: linux-wireless, aspriel, dokyungs, jisoo.jang

[-- Attachment #1: Type: text/plain, Size: 8039 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads
  2022-11-11  7:53 [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads Minsuk Kang
  2022-11-16  9:23 ` Kalle Valo
@ 2022-11-16 11:58 ` Arend van Spriel
  1 sibling, 0 replies; 4+ messages in thread
From: Arend van Spriel @ 2022-11-16 11:58 UTC (permalink / raw)
  To: Minsuk Kang, linux-wireless, aspriel; +Cc: dokyungs, jisoo.jang

[-- Attachment #1: Type: text/plain, Size: 2480 bytes --]

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

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-11-16 12:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-11  7:53 [PATCH] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads Minsuk Kang
2022-11-16  9:23 ` Kalle Valo
2022-11-16 11:27   ` Arend van Spriel
2022-11-16 11:58 ` Arend van Spriel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.