BCM43455: brcmf_notify_rssi / cfg80211_cqm_rssi_notify : Unable to handle kernel NULL pointer dereference

BCM43455: brcmf_notify_rssi / cfg80211_cqm_rssi_notify : Unable to handle kernel NULL pointer dereference

[Max Schulze]

Hello,

I have a recurring kernel crash with the wifi device driver on kernel 6.1.42 and 6.1.44 . Hardware is a raspberry pi cm4 on the original CM4IO Board. The wifi chip is soldered on. From the datasheet, the chip is a BCM43455/CYW43455.

This crash happens roughly every 60 hours on devices in the field, and I have observed it every ~30th reboot on my homelab.

It seems that it is related to disconnecting from a station right beforehand. Same on reboot, seems to be just after disconnection.

I am using wpa_supplicant and NetworkManager on userspace. Due to roaming problems, I am running the module with parameters

options brcmfmac p2pon=0 debug=50180 roamoff=1

I am attaching a crash from the field today, because it has some output after the kernel trace.

Thanks,
Max





Aug 09 11:28:33 pch8i: brcmfmac: brcmf_cfg80211_get_station RSSI -66 dBm
Aug 09 11:28:37 pch8i: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=0000000088524b81
Aug 09 11:28:37 pch8i: brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:00:00:00:00
Aug 09 11:28:37 pch8i: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
Aug 09 11:28:37 pch8i: brcmutil: event payload, len=12
Aug 09 11:28:37 pch8i: 00000000: ff ff ff b6 00 00 00 00 ff ff ff ab          ............
Aug 09 11:28:37 pch8i: brcmfmac: brcmf_notify_rssi LOW rssi=-74
Aug 09 11:28:37 pch8i: brcmfmac: brcmf_cfg80211_get_station RSSI -74 dBm
Aug 09 11:28:37 pch8i wpa_supplicant[391]: wlan0: CTRL-EVENT-SIGNAL-CHANGE above=0 signal=-74 noise=9999 txrate=24000
[...to show normal behaviour...]

Aug 09 11:28:57 pch8i: brcmfmac: brcmf_cfg80211_get_station RSSI -80 dBm
Aug 09 11:29:03 pch8i: brcmfmac: brcmf_cfg80211_get_station RSSI -81 dBm
Aug 09 11:29:07 pch8i wpa_supplicant[391]: wlan0: CTRL-EVENT-DISCONNECTED bssid=00:01:XX:XX:a1:70 reason=0 locally_generated=1
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000203d6aae
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000203d6aae
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_fweh_event_worker event LINK (16) ifidx 0 bsscfg 0 addr 00:01:XX:XX:a1:70
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 1
Aug 09 11:29:07 pch8i: brcmutil: event payload, len=22
Aug 09 11:29:07 pch8i: 00000000: 30 14 01 00 00 0f ac 04 01 00 00 0f ac 04 01 00  0...............
Aug 09 11:29:07 pch8i: 00000010: 00 0f ac 02 00 00          ......
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_is_linkdown Processing link down
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_notify_connect_status Linkdown
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_link_down Call WLC_DISASSOC to stop excess roaming
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000203d6aae
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_btcoex_set_mode DHCP session ends
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:00:00:00:00
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
Aug 09 11:29:07 pch8i: brcmutil: event payload, len=12
Aug 09 11:29:07 pch8i: 00000000: 00 00 00 00 00 00 00 00 00 00 00 00          ............
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_notify_rssi LOW rssi=0
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (0)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (1)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (2)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (3)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (4)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key key index (5)
Aug 09 11:29:07 pch8i: brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
Aug 09 11:29:07 pch8i: ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52
Aug 09 11:29:07 pch8i: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
Aug 09 11:29:07 pch8i: Mem abort info:
Aug 09 11:29:07 pch8i:   ESR = 0x0000000096000005
Aug 09 11:29:07 pch8i:   EC = 0x25: DABT (current EL), IL = 32 bits
Aug 09 11:29:07 pch8i:   SET = 0, FnV = 0
Aug 09 11:29:07 pch8i:   EA = 0, S1PTW = 0
Aug 09 11:29:07 pch8i:   FSC = 0x05: level 1 translation fault
Aug 09 11:29:07 pch8i: Data abort info:
Aug 09 11:29:07 pch8i:   ISV = 0, ISS = 0x00000005
Aug 09 11:29:07 pch8i:   CM = 0, WnR = 0
Aug 09 11:29:07 pch8i: user pgtable: 4k pages, 39-bit VAs, pgdp=00000000470bc000
Aug 09 11:29:07 pch8i: [0000000000000004] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
Aug 09 11:29:07 pch8i: Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
Aug 09 11:29:07 pch8i: Modules linked in: ov9281 rtc_pcf85063 regmap_i2c brcmfmac brcmutil cfg80211 vc4 v3d gpu_sched gpio_keys rfkill drm_shmem_helper i2c_mux_pinctrl snd_soc_hdmi_codec i2c_mux raspberrypi_hwmon i2c_brcmstb drm_display_helper bcm2835_codec(C) bcm2835_v4l2(C) bcm2835_isp(C) snd_bcm2835(C) cec bcm2835_mmal_vchiq(C) drm_dma_helper bcm2835_unicam drm_kms_helper v4l2_dv_timings v4l2_fwnode v4l2_async rpivid_hevc(C) v4l2_mem2mem snd_soc_core videobuf2_vmalloc videobuf2_dma_contig i2c_bcm2835 videobuf2_memops videobuf2_v4l2 snd_compress videobuf2_common snd_pcm_dmaengine snd_pcm videodev snd_timer vc_sm_cma(C) snd syscopyarea mc sysfillrect sysimgblt uio_pdrv_genirq fb_sys_fops nvmem_rmem uio drm fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6
Aug 09 11:29:08 pch8i: CPU: 0 PID: 1792 Comm: kworker/0:0 Tainted: G     C         6.1.42-v8+ #1
Aug 09 11:29:08 pch8i: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
Aug 09 11:29:08 pch8i: Workqueue: events brcmf_fweh_event_worker [brcmfmac]
Aug 09 11:29:08 pch8i: pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
Aug 09 11:29:08 pch8i: pc : cfg80211_cqm_rssi_notify+0x78/0x1b0 [cfg80211]
Aug 09 11:29:08 pch8i: lr : cfg80211_cqm_rssi_notify+0x70/0x1b0 [cfg80211]
Aug 09 11:29:08 pch8i: sp : ffffffc00c37bc40
Aug 09 11:29:08 pch8i: x29: ffffffc00c37bc40 x28: ffffff8001e2ddd0 x27: ffffff8001e2dd80
Aug 09 11:29:08 pch8i: x26: ffffff80437b4a48 x25: dead000000000100 x24: ffffff80437b43a0
Aug 09 11:29:08 pch8i: x23: ffffff80434f6008 x22: 0000000000000cc0 x21: 0000000000000000
Aug 09 11:29:08 pch8i: x20: 0000000000000000 x19: ffffff8040890000 x18: ffffffffffffffff
Aug 09 11:29:08 pch8i: x17: 2c64656c69616620 x16: 4f464e4920415453 x15: 20544547203a6e6f
Aug 09 11:29:08 pch8i: x14: 69746174735f7465 x13: 32352d202c64656c x12: 696166204f464e49
Aug 09 11:29:08 pch8i: x11: 2041545320544547 x10: ffffffec638568a0 x9 : ffffffec16093000
Aug 09 11:29:08 pch8i: x8 : ffffff8042de3e00 x7 : 0000000000000000 x6 : ffffffc00c37b8c8
Aug 09 11:29:08 pch8i: x5 : ffffffc00c37b8f0 x4 : 00000000ffffffd8 x3 : 000000000000c404
Aug 09 11:29:08 pch8i: x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000
Aug 09 11:29:08 pch8i: Call trace:
Aug 09 11:29:08 pch8i:  cfg80211_cqm_rssi_notify+0x78/0x1b0 [cfg80211]
Aug 09 11:29:08 pch8i:  brcmf_notify_rssi+0x10c/0x1b0 [brcmfmac]
Aug 09 11:29:08 pch8i:  brcmf_fweh_call_event_handler+0x40/0xa0 [brcmfmac]
Aug 09 11:29:08 pch8i:  brcmf_fweh_event_worker+0x1e4/0x4f0 [brcmfmac]
Aug 09 11:29:08 pch8i:  process_one_work+0x1dc/0x450
Aug 09 11:29:08 pch8i:  worker_thread+0x154/0x450
Aug 09 11:29:08 pch8i:  kthread+0x104/0x110
Aug 09 11:29:08 pch8i:  ret_from_fork+0x10/0x20
Aug 09 11:29:08 pch8i: Code: d10e8300 97fffee9 35000074 f941bae0 (b9400414)
Aug 09 11:29:08 pch8i: ---[ end trace 0000000000000000 ]---
Aug 09 11:29:10 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:10 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:10 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:10 pch8i: ieee80211 phy0: brcmf_cfg80211_reg_notifier: Country code iovar returned err = -110
Aug 09 11:29:12 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:12 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:12 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:15 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:15 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:15 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:15 pch8i: ieee80211 phy0: brcmf_cfg80211_reg_notifier: Country code iovar returned err = -110
Aug 09 11:29:18 pch8i wpa_supplicant[391]: wlan0: CTRL-EVENT-REGDOM-CHANGE init=CORE type=WORLD
Aug 09 11:29:18 pch8i wpa_supplicant[391]: wlan0: CTRL-EVENT-REGDOM-CHANGE init=USER type=COUNTRY alpha2=DE
Aug 09 11:29:18 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:18 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:18 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:18 pch8i: ieee80211 phy0: brcmf_cfg80211_get_tx_power: error (-110)
Aug 09 11:29:20 pch8i NetworkManager[347]: <info>  [1691573360.5820] device (wlan0): supplicant interface state: completed -> disconnected
Aug 09 11:29:20 pch8i NetworkManager[347]: <info>  [1691573360.5822] device (p2p-dev-wlan0): supplicant management interface state: completed -> disconnected
Aug 09 11:29:20 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:20 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:20 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:20 pch8i: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110
Aug 09 11:29:20 pch8i: brcmfmac: brcmf_cfg80211_scan START ESCAN
Aug 09 11:29:23 pch8i wpa_supplicant[391]: wlan0: CTRL-EVENT-SCAN-FAILED ret=-110 retry=1
Aug 09 11:29:23 pch8i: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
Aug 09 11:29:23 pch8i: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70
Aug 09 11:29:23 pch8i: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert
Aug 09 11:29:23 pch8i: ieee80211 phy0: brcmf_vif_set_mgmt_ie: vndr ie set error : -110
Aug 09 11:29:23 pch8i: ieee80211 phy0: brcmf_cfg80211_scan: scan error (-110)
Aug 09 11:29:24 pch8i: brcmfmac: brcmf_cfg80211_scan START ESCAN
Aug 09 11:29:24 pch8i: brcmfmac: brcmf_do_escan Enter

Re: BCM43455: brcmf_notify_rssi / cfg80211_cqm_rssi_notify : Unable to handle kernel NULL pointer dereference (RSSI notification after station disconnect?)

[Max Schulze]

I have reproduced this on linux 6.5.0-rc5 with more debug info and source line pointers.

I get the impression that the firmware generates a rssi notification, even after it has disconnected from the AP.

Looking into the
> BUG: KASAN: null-ptr-deref in cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211

with source context:

net/wireless/nl80211.c
  19069 void cfg80211_cqm_rssi_notify(struct net_device *dev,
  ...
  19082 
  19083         if (wdev->cqm_config) {
  19084                 wdev->cqm_config->last_rssi_event_value = rssi_level;
  19085 
  19086                 cfg80211_cqm_rssi_update(rdev, dev);
  19087 
  19088                 if (rssi_level == 0)
  19089                         rssi_level = wdev->cqm_config->last_rssi_event_value;
  19090         }
  19091 

it looks like the cfg80211_cqm_rssi_update would unset the ->cqm_config, as the firmware is telling us, that there is (currently) no station

> brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_NOTFOUND (-30)
> brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=tdls_sta_info, len=296, err=-52

> brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_BADADDR (-21)
> brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=sta_info, len=296, err=-52

> ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52


And then we have the null pointer at

 
   19088                 if (rssi_level == 0)
   19089  -->                    rssi_level = wdev->cqm_config->last_rssi_event_value;
   19090         }

So my suggestion would be to do 

   19088                 if (rssi_level == 0 && wdev->cqm_config) // config has not been deleted / station deleted in the meantime
   19089                         rssi_level = wdev->cqm_config->last_rssi_event_value;
   19090         }


Any opinions?


Best,

Max


(nota: the interleaved brcmdata in between the KASAN report are original)

wpa_supplicant[332]: wlan0: CTRL-EVENT-DISCONNECTED bssid=XX:XX:XX:XX:74:1f reason=3 locally_generated=1
systemd[1]: Stopping User Manager for UID 1000...
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=0000000017163222
brcmfmac: brcmf_fweh_event_worker event LINK (16) ifidx 0 bsscfg 0 addr xx:xx:xx:xx:74:1f
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 2
brcmutil: event payload, len=0
brcmfmac: brcmf_is_linkdown Processing link down
brcmfmac: brcmf_notify_connect_status Linkdown
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000dcf7c0c0
brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:xx:xx:00:50
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
brcmutil: event payload, len=12
00000000: 00 00 00 00 00 00 00 00 00 00 00 00              ............
brcmfmac: brcmf_notify_rssi LOW rssi=0
brcmfmac: brcmf_cfg80211_del_key key index (0)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (1)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (2)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (3)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (4)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (5)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_NOTFOUND (-30)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=tdls_sta_info, len=296, err=-52
brcmutil: data
00000000: 34 2c c4 3a 74 1f 00 00 00 00 00 00 00 00 00 00  4,.:t...........
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_BADADDR (-21)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=sta_info, len=296, err=-52
brcmutil: data
00000000: 34 2c c4 3a 74 1f 00 00 00 00 00 00 00 00 00 00  4,.:t...........
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52
==================================================================
BUG: KASAN: null-ptr-deref in cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
Read of size 4 at addr 0000000000000004 by task kworker/1:2/126
 kernel:
CPU: 1 PID: 126 Comm: kworker/1:2 Tainted: G         C         6.5.0-rc5-v8-g1af52ad6a306 #2
Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
Workqueue: events brcmf_fweh_event_worker [brcmfmac]
Call trace:
dump_backtrace (/home/r/linux/arch/arm64/kernel/stacktrace.c:235) 
show_stack (/home/r/linux/arch/arm64/kernel/stacktrace.c:242) 
dump_stack_lvl (/home/r/linux/lib/dump_stack.c:107) 
kasan_report (/home/r/linux/mm/kasan/report.c:590) 
__asan_load4 (/home/r/linux/mm/kasan/generic.c:259) 
brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=rssi_event, len=16
brcmutil: data
00000000: 00 00 00 00 03 00 00 7f 00 00 00 00 00 00 00 00  ................
cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
brcmf_notify_rssi (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6651) brcmfmac
brcmf_fweh_call_event_handler (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:109) brcmfmac
brcmf_fweh_event_worker (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:268) brcmfmac
process_one_work (/home/r/linux/./arch/arm64/include/asm/jump_label.h:21 /home/r/linux/./include/linux/jump_label.h:207 /home/r/linux/./include/trace/events/workqueue.h:108 /home/r/linux/kernel/workqueue.c:2602) 
worker_thread (/home/r/linux/./include/linux/list.h:292 /home/r/linux/kernel/workqueue.c:2749) 
kthread (/home/r/linux/kernel/kthread.c:389) 
ret_from_fork (/home/r/linux/arch/arm64/kernel/entry.S:854) 
==================================================================
Disabling lock debugging due to kernel taint
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
Mem abort info:
  ESR = 0x0000000096000005
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x05: level 1 translation fault
Data abort info:
  ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
  CM = 0, WnR = 0, TnD = 0, TagAccess = 0
  GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 39-bit VAs, pgdp=000000004848b000
[0000000000000004] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
Modules linked in: brcmfmac_wcc vc4 brcmfmac snd_soc_hdmi_codec cfg80211 snd_soc_core snd_pcm_dmaengine rtc_pcf85063 snd_pcm regmap_i2c snd_timer rpivid_hevc(C) snd xt_tcpudp nft_compat bcm2835_isp(C) nf_tables drm_display_helper bcm2835_mmal_vchiq(C) videobuf2_dma_contig v3d drm_dma_helper videobuf2_memops v4l2_mem2mem videobuf2_v4l2 videodev nfnetlink drm_shmem_helper drm_kms_helper gpu_sched i2c_mux_pinctrl gpio_keys raspberrypi_hwmon i2c_mux i2c_brcmstb rfkill joydev hid_microsoft videobuf2_common ff_memless brcmutil i2c_bcm2835 mc vc_sm_cma(C) cec nvmem_rmem uio_pdrv_genirq uio drm fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6
CPU: 1 PID: 126 Comm: kworker/1:2 Tainted: G    B    C         6.5.0-rc5-v8-g1af52ad6a306 #2
Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
Workqueue: events brcmf_fweh_event_worker [brcmfmac]
pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
lr : cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
sp : ffffffc0808f7a10
x29: ffffffc0808f7a10 x28: ffffff80432d0a68 x27: ffffff804b1d4368
x26: ffffff80432d03c0 x25: 1ffffff81011ef4e x24: ffffff8049b6fbc0
x23: 0000000000000cc0 x22: 0000000000000000 x21: ffffff804b1d4008
x20: 0000000000000000 x19: ffffff8048096000 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
x14: 0000000000000000 x13: 746e696174206c65 x12: ffffffbcc23e1e29
x11: 1ffffffcc23e1e28 x10: ffffffbcc23e1e28 x9 : dfffffc000000000
x8 : 000000433dc1e1d8 x7 : ffffffe611f0f147 x6 : 0000000000000001
x5 : ffffffe611f0f140 x4 : ffffffbcc23e1e29 x3 : ffffffe60f6c0924
x2 : 0000000000000000 x1 : ffffff80469f4000 x0 : 0000000000000001
Call trace:
cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
brcmf_notify_rssi (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6651) brcmfmac
brcmf_fweh_call_event_handler (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:109) brcmfmac
brcmf_fweh_event_worker (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:268) brcmfmac
process_one_work (/home/r/linux/./arch/arm64/include/asm/jump_label.h:21 /home/r/linux/./include/linux/jump_label.h:207 /home/r/linux/./include/trace/events/workqueue.h:108 /home/r/linux/kernel/workqueue.c:2602) 
worker_thread (/home/r/linux/./include/linux/list.h:292 /home/r/linux/kernel/workqueue.c:2749) 
kthread (/home/r/linux/kernel/kthread.c:389) 
ret_from_fork (/home/r/linux/arch/arm64/kernel/entry.S:854) 
Code: 9401bd51 f941b2b4 91001280 9401bd48 (b9400694)
All code
========
   0:	9401bd51 	bl	0x6f544
   4:	f941b2b4 	ldr	x20, [x21, #864]
   8:	91001280 	add	x0, x20, #0x4
   c:	9401bd48 	bl	0x6f52c
  10:*	b9400694 	ldr	w20, [x20, #4]		<-- trapping instruction

Code starting with the faulting instruction
===========================================
   0:	b9400694 	ldr	w20, [x20, #4]
---[ end trace 0000000000000000 ]---
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=country, len=12, err=0
brcmutil: data
00000000: 44 45 00 00 00 00 00 00 44 45 00 41              DE......DE.A
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=0000000068d7cc22
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=qtxpower, len=4, err=0

[PATCH] wifi: nl80211: avoid NULL-ptr deref after cfg80211_cqm_rssi_update

[Max Schulze]

In cfg80211_cqm_rssi_notify, when calling cfg80211_cqm_rssi_update, this might free
the wdev->cqm_config . Check for this when it returns.

This has been observed on brcmfmac, when a RSSI event is generated just right
after disconnecting from AP. Then probing for STA details returns nothing, as
evidenced i.e. by
"ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52".


Signed-off-by: Max Schulze <max.schulze@online.de>
Tested-by: Max Schulze <max.schulze@online.de>
Link: https://lore.kernel.org/linux-wireless/bc3bf8f6-7ad7-bf69-9227-f972dac4e66b@online.de/
---

I have deployed this to 22 systems without issues and eliminating those null-ptr deref.

Example Trace from Problem:

wpa_supplicant[332]: wlan0: CTRL-EVENT-DISCONNECTED bssid=XX:XX:XX:XX:74:1f reason=3 locally_generated=1
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=0000000017163222
brcmfmac: brcmf_fweh_event_worker event LINK (16) ifidx 0 bsscfg 0 addr xx:xx:xx:xx:74:1f
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 2
brcmutil: event payload, len=0
brcmfmac: brcmf_is_linkdown Processing link down
brcmfmac: brcmf_notify_connect_status Linkdown
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000dcf7c0c0
brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:xx:xx:00:50
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
brcmutil: event payload, len=12
00000000: 00 00 00 00 00 00 00 00 00 00 00 00              ............
brcmfmac: brcmf_notify_rssi LOW rssi=0
brcmfmac: brcmf_cfg80211_del_key key index (0)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_NOTFOUND (-30)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=tdls_sta_info, len=296, err=-52
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_BADADDR (-21)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=sta_info, len=296, err=-52
ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52
==================================================================
BUG: KASAN: null-ptr-deref in cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211


 net/wireless/nl80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8bcf8e293..b12424382 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -19088,7 +19088,7 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 
 		cfg80211_cqm_rssi_update(rdev, dev);
 
-		if (rssi_level == 0)
+		if (rssi_level == 0 && wdev->cqm_config)
 			rssi_level = wdev->cqm_config->last_rssi_event_value;
 	}
 
-- 
2.39.1

Re: [PATCH] wifi: nl80211: avoid NULL-ptr deref after cfg80211_cqm_rssi_update

[Johannes Berg]

On Fri, 2023-08-11 at 09:30 +0200, Max Schulze wrote:
> In cfg80211_cqm_rssi_notify, when calling cfg80211_cqm_rssi_update, this might free
> the wdev->cqm_config . Check for this when it returns.

That doesn't seem right? How does cfg80211_cqm_rssi_update() free it?

> This has been observed on brcmfmac, when a RSSI event is generated just right
> after disconnecting from AP. Then probing for STA details returns nothing, as
> evidenced i.e. by
> "ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52".

I think the issue then isn't that this frees it but rather than a free
of it races with the reporting?

> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -19088,7 +19088,7 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
>  
>  		cfg80211_cqm_rssi_update(rdev, dev);
>  
> -		if (rssi_level == 0)
> +		if (rssi_level == 0 && wdev->cqm_config)
>  			rssi_level = wdev->cqm_config->last_rssi_event_value;
> 

But if it's a race, then this isn't actually going to really fix the
issue, rather it just makes it (much) less likely.

Since we can probably neither lock the wdev here nor require calls to
this function with wdev lock held, it looks like we need to protect the
pointer with RCU instead?

johannes

Re: BCM43455: brcmf_notify_rssi / cfg80211_cqm_rssi_notify : Unable to handle kernel NULL pointer dereference (RSSI notification after station disconnect?)

[Max Schulze]

Johannes pointed out that I have no clear link demonstrating that cfg80211_cqm_config_free is being called from cfg80211_cqm_rssi_notify.

Fair enough. So I compiled with

>  void cfg80211_cqm_config_free(struct wireless_dev *wdev)
>  {
> +       pr_err("someone freeing the cqm_config");
> +       dump_stack();
>         kfree(wdev->cqm_config);
>         wdev->cqm_config = NULL;
>  }


And get the attached output.

Now I am out of ideas. This to me clearly looks like some kind of race. Freeing the cqm as part of netlink_sendmsg?

Maybe cfg80211_cqm_rssi_notify could just exit() when there currently is no station connected and thus avoid the null-ptr deref?



brcmfmac: brcmf_fil_cmd_data_set ifidx=0, cmd=52, len=12
brcmutil: data
00000000: 03 00 00 00 34 2c c4 3a 74 1f 37 da              ....4,.:t.7.
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=ffffff8040227240
brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=ffffff8040226340
brcmfmac: brcmf_fweh_event_worker event LINK (16) ifidx 0 bsscfg 0 addr xx:xx:xx:xx:74:1f
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 2
brcmutil: event payload, len=0
brcmfmac: brcmf_is_linkdown Processing link down
brcmfmac: brcmf_notify_connect_status Linkdown
brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr xx:xx:xx:xx:00:50
brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
brcmutil: event payload, len=12
00000000: 00 00 00 00 00 00 00 00 00 00 00 00              ............
brcmfmac: brcmf_notify_rssi LOW rssi=0
brcmfmac: brcmf_cfg80211_del_key key index (0)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (1)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (2)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (3)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (4)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_cfg80211_del_key key index (5)
brcmfmac: brcmf_cfg80211_del_key Ignore clearing of (never configured) key
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=country, len=12, err=0
brcmutil: data
00000000: 44 45 00 00 00 00 00 00 44 45 00 41              DE......DE.A
cfg80211: someone freeing the cqm_config
CPU: 0 PID: 335 Comm: wpa_supplicant Tainted: G         C         6.5.0-rc5-v8-gd9595c9a4f6d-dirty #7
Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
Call trace:
dump_backtrace (/home/r/linux/arch/arm64/kernel/stacktrace.c:235) 
show_stack (/home/r/linux/arch/arm64/kernel/stacktrace.c:242) 
dump_stack_lvl (/home/r/linux/lib/dump_stack.c:107) 
dump_stack (/home/r/linux/lib/dump_stack.c:114) 
cfg80211_cqm_config_free (/home/r/linux/net/wireless/core.c:1188) cfg80211
nl80211_set_cqm (/home/r/linux/net/wireless/core.h:239 /home/r/linux/net/wireless/nl80211.c:12883 /home/r/linux/net/wireless/nl80211.c:12955) cfg80211
genl_family_rcv_msg_doit.isra.0 (/home/r/linux/net/netlink/genetlink.c:970) 
genl_rcv_msg (/home/r/linux/net/netlink/genetlink.c:1050 /home/r/linux/net/netlink/genetlink.c:1067) 
netlink_rcv_skb (/home/r/linux/net/netlink/af_netlink.c:2549) 
genl_rcv (/home/r/linux/net/netlink/genetlink.c:1079) 
netlink_unicast (/home/r/linux/net/netlink/af_netlink.c:1340 /home/r/linux/net/netlink/af_netlink.c:1365) 
netlink_sendmsg (/home/r/linux/net/netlink/af_netlink.c:1914) 
sock_sendmsg (/home/r/linux/net/socket.c:725 /home/r/linux/net/socket.c:748) 
____sys_sendmsg (/home/r/linux/net/socket.c:2494) 
___sys_sendmsg (/home/r/linux/net/socket.c:2548) 
__sys_sendmsg (/home/r/linux/./include/linux/file.h:32 /home/r/linux/net/socket.c:2579) 
__arm64_sys_sendmsg (/home/r/linux/net/socket.c:2584) 
invoke_syscall (/home/r/linux/arch/arm64/kernel/syscall.c:38 /home/r/linux/arch/arm64/kernel/syscall.c:52) 
el0_svc_common.constprop.0 (/home/r/linux/./include/linux/thread_info.h:127 /home/r/linux/arch/arm64/kernel/syscall.c:147) 
do_el0_svc (/home/r/linux/arch/arm64/kernel/syscall.c:189) 
el0_svc (/home/r/linux/arch/arm64/kernel/entry-common.c:133 /home/r/linux/arch/arm64/kernel/entry-common.c:144 /home/r/linux/arch/arm64/kernel/entry-common.c:648) 
el0t_64_sync_handler (/home/r/linux/arch/arm64/kernel/entry-common.c:666) 
el0t_64_sync (/home/r/linux/arch/arm64/kernel/entry.S:591) 
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_NOTFOUND (-30)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=tdls_sta_info, len=296, err=-52
brcmutil: data
00000000: 34 2c c4 3a 74 1f 00 00 00 00 00 00 00 00 00 00  4,.:t...........
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
brcmfmac: brcmf_fil_cmd_data Firmware error: BCME_BADADDR (-21)
brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=sta_info, len=296, err=-52
brcmutil: data
00000000: 34 2c c4 3a 74 1f 00 00 00 00 00 00 00 00 00 00  4,.:t...........
00000010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
00000030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52
==================================================================
BUG: KASAN: null-ptr-deref in cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
Read of size 4 at addr 0000000000000004 by task kworker/0:2/38
Aug 12 09:28:20 hlcm4 kernel:
CPU: 0 PID: 38 Comm: kworker/0:2 Tainted: G         C         6.5.0-rc5-v8-gd9595c9a4f6d-dirty #7
Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
Workqueue: events brcmf_fweh_event_worker [brcmfmac]
Call trace:
dump_backtrace (/home/r/linux/arch/arm64/kernel/stacktrace.c:235) 
show_stack (/home/r/linux/arch/arm64/kernel/stacktrace.c:242) 
dump_stack_lvl (/home/r/linux/lib/dump_stack.c:107) 
kasan_report (/home/r/linux/mm/kasan/report.c:590) 
__asan_load4 (/home/r/linux/mm/kasan/generic.c:259) 
cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
brcmf_notify_rssi (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6651) brcmfmac
brcmf_fweh_call_event_handler (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:109) brcmfmac
brcmf_fweh_event_worker (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:268) brcmfmac
process_one_work (/home/r/linux/./arch/arm64/include/asm/jump_label.h:21 /home/r/linux/./include/linux/jump_label.h:207 /home/r/linux/./include/trace/events/workqueue.h:108 /home/r/linux/kernel/workqueue.c:2602) 
worker_thread (/home/r/linux/./include/linux/list.h:292 /home/r/linux/kernel/workqueue.c:2749) 
kthread (/home/r/linux/kernel/kthread.c:389) 
ret_from_fork (/home/r/linux/arch/arm64/kernel/entry.S:854) 
==================================================================
Disabling lock debugging due to kernel taint
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000004
Mem abort info:
  ESR = 0x0000000096000005
  EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x05: level 1 translation fault
 Data abort info:
   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
 user pgtable: 4k pages, 39-bit VAs, pgdp=000000004c5c2000
 brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=rssi_event, len=16
 [0000000000000004] pgd=0000000000000000
 brcmutil: data
 , p4d=0000000000000000, pud=0000000000000000
 00000000: 00 00 00 00 03 00 00 7f 00 00 00 00 00 00 00 00  ................

 Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
 Modules linked in: brcmfmac_wcc brcmfmac cfg80211 rpivid_hevc(C) bcm2835_isp(C) rtc_pcf85063 bcm2835_mmal_vchiq(C) regmap_i2c xt_tcpudp nft_compat videobuf2_dma_contig v3d videobuf2_memops nf_tables v4l2_mem2mem videobuf2_v4l2 videodev nfnetlink drm_shmem_helper gpu_sched i2c_mux_pinctrl gpio_keys i2c_mux hid_microsoft raspberrypi_hwmon videobuf2_common joydev ff_memless rfkill brcmutil i2c_brcmstb i2c_bcm2835 vc_sm_cma(C) mc uio_pdrv_genirq nvmem_rmem uio drm fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6
 CPU: 0 PID: 38 Comm: kworker/0:2 Tainted: G    B    C         6.5.0-rc5-v8-gd9595c9a4f6d-dirty #7
 Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
 Workqueue: events brcmf_fweh_event_worker [brcmfmac]
 pstate: 40000005 (nZcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
 lr : cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
 sp : ffffffc0805c7a10
 x29: ffffffc0805c7a10 x28: 0000000000000001 x27: ffffff804743a368
 x26: ffffff80428883c0 x25: 1ffffff8100b8f4e x24: ffffff804c1496d0
 x23: 0000000000000cc0 x22: 0000000000000000 x21: ffffff804743a008
 x20: 0000000000000000 x19: ffffff804c0ac000 x18: 0000000000000000
 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
 x14: 0000000000000000 x13: 746e696174206c65 x12: ffffffbde8d1dc29
 x11: 1ffffffde8d1dc28 x10: ffffffbde8d1dc28 x9 : dfffffc000000000
 x8 : 00000042172e23d8 x7 : ffffffef468ee147 x6 : 0000000000000001
 x5 : ffffffef468ee140 x4 : ffffffbde8d1dc29 x3 : ffffffef440c0924
 x2 : 0000000000000000 x1 : ffffff8040b6a0c0 x0 : 0000000000000001
 Call trace:
 cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19089) cfg80211
 brcmf_notify_rssi (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6651) brcmfmac
 brcmf_fweh_call_event_handler (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:109) brcmfmac
 brcmf_fweh_event_worker (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:268) brcmfmac
 process_one_work (/home/r/linux/./arch/arm64/include/asm/jump_label.h:21 /home/r/linux/./include/linux/jump_label.h:207 /home/r/linux/./include/trace/events/workqueue.h:108 /home/r/linux/kernel/workqueue.c:2602) 
 worker_thread (/home/r/linux/./include/linux/list.h:292 /home/r/linux/kernel/workqueue.c:2749) 
 kthread (/home/r/linux/kernel/kthread.c:389) 
 ret_from_fork (/home/r/linux/arch/arm64/kernel/entry.S:854) 
 Code: 9401bd4d f941b2b4 91001280 9401bd44 (b9400694)
All code
========
   0:	9401bd4d 	bl	0x6f534
   4:	f941b2b4 	ldr	x20, [x21, #864]
   8:	91001280 	add	x0, x20, #0x4
   c:	9401bd44 	bl	0x6f51c
  10:*	b9400694 	ldr	w20, [x20, #4]		<-- trapping instruction

Code starting with the faulting instruction
===========================================
   0:	b9400694 	ldr	w20, [x20, #4]
 ---[ end trace 0000000000000000 ]---
 brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=qtxpower, len=4, err=0
 brcmutil: data
 00000000: 7f 00 00 00                                      ....
 brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=mcast_list, len=22

Re: [PATCH] wifi: nl80211: avoid NULL-ptr deref after cfg80211_cqm_rssi_update

[Max Schulze]

Am 11.08.23 um 11:54 schrieb Johannes Berg:
> On Fri, 2023-08-11 at 09:30 +0200, Max Schulze wrote:
>> In cfg80211_cqm_rssi_notify, when calling cfg80211_cqm_rssi_update, this might free
>> the wdev->cqm_config . Check for this when it returns.
> 
> That doesn't seem right? How does cfg80211_cqm_rssi_update() free it?
> 


You are probably right, that it doesn't. I amended the original bug report where I have a trace of the free and it is not a descendant of _rssi_update().



>> This has been observed on brcmfmac, when a RSSI event is generated just right
>> after disconnecting from AP. Then probing for STA details returns nothing, as
>> evidenced i.e. by
>> "ieee80211 phy0: brcmf_cfg80211_get_station: GET STA INFO failed, -52".
> 
> I think the issue then isn't that this frees it but rather than a free
> of it races with the reporting?


I do not understand what you mean. I think there is a race, yes, somewhere else it get's freed, and when we access it, there is the null-ptr-deref.
The GET STA INFO failed is just the expected answer from the chip, because it currently is not associated with any station.


> 
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -19088,7 +19088,7 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
>>  
>>  		cfg80211_cqm_rssi_update(rdev, dev);
>>  
>> -		if (rssi_level == 0)
>> +		if (rssi_level == 0 && wdev->cqm_config)
>>  			rssi_level = wdev->cqm_config->last_rssi_event_value;
>>
> 
> But if it's a race, then this isn't actually going to really fix the
> issue, rather it just makes it (much) less likely.
> 

You have been right with your prediction. It made it around ~15x less likely but I have recorded a trace. It just has the null-ptr somewhere else.

> Since we can probably neither lock the wdev here nor require calls to
> this function with wdev lock held, it looks like we need to protect the
> pointer with RCU instead?
> 

I do not know how to do that and need help.

Max

[this is with my patch some lines above, no more crashes in _rssi_notify, but now somewhere else ].

13:15:03: brcmfmac: brcmf_is_linkdown Processing link down
13:15:03: brcmfmac: brcmf_notify_connect_status Linkdown
13:15:03: brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr xx:xx:xx:xx:00:50
13:15:03: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
13:15:03: brcmutil: event payload, len=12
13:15:03: 00000000: 00 00 00 00 00 00 00 00 00 00 00 00              ............
13:15:03: brcmfmac: brcmf_notify_rssi LOW rssi=0
13:15:03: brcmfmac: brcmf_fil_iovar_data_set ifidx=0, name=rssi_event, len=16
13:15:03: Unable to handle kernel NULL pointer dereference at virtual address 000000000000000c
13:15:03: brcmutil: data
13:15:03: Mem abort info:
13:15:03: 00000000: 00 00 00 00 03 00 00 7f 00 00 00 00 00 00 00 00  ................
13:15:03:   ESR = 0x0000000096000005
13:15:03:   EC = 0x25: DABT (current EL), IL = 32 bits
13:15:03:   SET = 0, FnV = 0
13:15:03:   EA = 0, S1PTW = 0
13:15:03:   FSC = 0x05: level 1 translation fault
13:15:03: Data abort info:
13:15:03:   ISV = 0, ISS = 0x00000005, ISS2 = 0x00000000
13:15:03:   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
13:15:03:   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
13:15:03: user pgtable: 4k pages, 39-bit VAs, pgdp=00000000456cf000
13:15:03: [000000000000000c] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000
13:15:03: Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP
13:15:03: Modules linked in: brcmfmac_wcc vc4 brcmfmac snd_soc_hdmi_codec snd_soc_core snd_pcm_dmaengine snd_pcm cfg80211 rpivid_hevc(C) bcm2835_isp(C) snd_timer bcm2835_mmal_vchiq(C) snd videobuf2_dma_contig videobuf2_memops v4l2_mem2mem xt_tcpudp rtc_pcf85063 nft_compat regmap_i2c nf_tables videobuf2_v4l2 v3d videodev drm_display_helper drm_shmem_helper nfnetlink drm_dma_helper gpu_sched i2c_mux_pinctrl i2c_mux gpio_keys drm_kms_helper raspberrypi_hwmon rfkill hid_microsoft joydev i2c_brcmstb ff_memless brcmutil i2c_bcm2835 videobuf2_common vc_sm_cma(C) mc cec nvmem_rmem uio_pdrv_genirq uio drm fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6
13:15:04: CPU: 1 PID: 57 Comm: kworker/1:2 Tainted: G         C         6.5.0-rc5-v8-g963e8f68ce3b #4
13:15:04: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
13:15:04: Workqueue: events brcmf_fweh_event_worker [brcmfmac]
13:15:04: pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
13:15:04: pc : cfg80211_cqm_rssi_update (/home/r/linux/net/wireless/nl80211.c:12838) cfg80211
13:15:04: lr : cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19088) cfg80211
13:15:04: sp : ffffffc08060baf0
13:15:04: x29: ffffffc08060baf0 x28: ffffff8052b798f0 x27: ffffff8052b798a0
13:15:04: x26: ffffff8055d4c8c0 x25: ffffff8055d4ca68 x24: ffffff8055d4c3c0
13:15:04: x23: ffffff8044a52008 x22: 0000000000000000 x21: ffffff8055d4c000
13:15:04: x20: ffffff8044a63000 x19: ffffff8044a52008 x18: 00000000fffffffe
13:15:04: x17: 2e2e2e2e2e202020 x16: ffffffddb73c9498 x15: 0000000000000020
13:15:04: x14: 0000000000000000 x13: 303d697373722057 x12: 4f4c20697373725f
13:15:04: x11: fffffffffffe39f8 x10: ffffff804018c700 x9 : ffffffdd39d20660
13:15:04: x8 : 000000005108ccd0 x7 : 0000000000000000 x6 : 000000005108ccd0
13:15:04: x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
13:15:04: x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000004
13:15:04: Call trace:
13:15:04: cfg80211_cqm_rssi_update (/home/r/linux/net/wireless/nl80211.c:12838) cfg80211
13:15:04: cfg80211_cqm_rssi_notify (/home/r/linux/net/wireless/nl80211.c:19088) cfg80211
13:15:04: brcmf_notify_rssi (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6651) brcmfmac
13:15:04: brcmf_fweh_call_event_handler (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:116) brcmfmac
13:15:04: brcmf_fweh_event_worker (/home/r/linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c:268) brcmfmac
13:15:04: process_one_work (/home/r/linux/./arch/arm64/include/asm/jump_label.h:21 /home/r/linux/./include/linux/jump_label.h:207 /home/r/linux/./include/trace/events/workqueue.h:108 /home/r/linux/kernel/workqueue.c:2602) 
13:15:04: worker_thread (/home/r/linux/./include/linux/list.h:292 /home/r/linux/kernel/workqueue.c:2749) 
13:15:04: kthread (/home/r/linux/kernel/kthread.c:389) 
13:15:04: ret_from_fork (/home/r/linux/arch/arm64/kernel/entry.S:854) 
13:15:04: Code: f941b264 0a020062 93407c45 8b22c883 (b9400c63)
All code
========
   0:	f941b264 	ldr	x4, [x19, #864]
   4:	0a020062 	and	w2, w3, w2
   8:	93407c45 	sxtw	x5, w2
   c:	8b22c883 	add	x3, x4, w2, sxtw #2
  10:*	b9400c63 	ldr	w3, [x3, #12]		<-- trapping instruction

Code starting with the faulting instruction
===========================================
   0:	b9400c63 	ldr	w3, [x3, #12]
13:15:04: ---[ end trace 0000000000000000 ]---
13:15:04: brcmfmac: brcmf_fil_iovar_data_get ifidx=0, name=qtxpower, len=4, err=0
13:15:04: brcmutil: data

[RFC PATCH] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Max Schulze reports crashes with brcmfmac. The reason seems
to be a race between userspace removing the CQM config and
the driver calling cfg80211_cqm_rssi_notify(), where if the
data is freed while cfg80211_cqm_rssi_notify() runs it will
crash since it assumes wdev->cqm_config is set. This can't
be fixed with a simple non-NULL check since there's nothing
we can do for locking easily, so use RCU instead to protect
the pointer.

Since we need to change the free anyway, also change it to
go back to the old settings if changing the settings fails.

Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
Closes: https://lore.kernel.org/r/ac96309a-8d8d-4435-36e6-6d152eb31876@online.de
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h |  2 +-
 net/wireless/core.c    | 11 ++++-----
 net/wireless/core.h    |  3 +--
 net/wireless/nl80211.c | 52 ++++++++++++++++++++++++------------------
 4 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d6fa7c8767ad..8de6e5c8c85e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -6014,7 +6014,7 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct cfg80211_cqm_config __rcu *cqm_config;
 
 	struct list_head pmsr_list;
 	spinlock_t pmsr_lock;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 25bc2e50a061..598da9451f0e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1181,16 +1181,11 @@ void wiphy_rfkill_set_hw_state_reason(struct wiphy *wiphy, bool blocked,
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state_reason);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
-{
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
-}
-
 static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 				      bool unregister_netdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 	unsigned int link_id;
 
 	ASSERT_RTNL();
@@ -1227,7 +1222,9 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 	kfree_sensitive(wdev->wext.keys);
 	wdev->wext.keys = NULL;
 #endif
-	cfg80211_cqm_config_free(wdev);
+	/* deleted from the list, so can't be found from nl80211 any more */
+	cqm_config = rcu_access_pointer(wdev->cqm_config);
+	kfree_rcu(cqm_config, rcu_head);
 
 	/*
 	 * Ensure that all events have been processed and
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 8a807b609ef7..62a91aa694a7 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -295,6 +295,7 @@ struct cfg80211_beacon_registration {
 };
 
 struct cfg80211_cqm_config {
+	struct rcu_head rcu_head;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
 	int n_rssi_thresholds;
@@ -566,8 +567,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
-
 void cfg80211_release_pmsr(struct wireless_dev *wdev, u32 portid);
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev);
 void cfg80211_pmsr_free_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8bcf8e293308..4f79c3543aed 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12796,7 +12796,8 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 }
 
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    struct cfg80211_cqm_config *cqm_config)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	s32 last, low, high;
@@ -12805,7 +12806,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!cqm_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -12814,7 +12815,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value &&
+	if (!cqm_config->last_rssi_event_value &&
 	    wdev->links[0].client.current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
@@ -12828,30 +12829,30 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 
 		cfg80211_sinfo_release_content(&sinfo);
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			cqm_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = cqm_config->last_rssi_event_value;
+	hyst = cqm_config->rssi_hyst;
+	n = cqm_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++) {
 		i = array_index_nospec(i, n);
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < cqm_config->rssi_thresholds[i])
 			break;
 	}
 
 	low_index = i - 1;
 	if (low_index >= 0) {
 		low_index = array_index_nospec(low_index, n);
-		low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+		low = cqm_config->rssi_thresholds[low_index] - hyst;
 	} else {
 		low = S32_MIN;
 	}
 	if (i < n) {
 		i = array_index_nospec(i, n);
-		high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+		high = cqm_config->rssi_thresholds[i] + hyst - 1;
 	} else {
 		high = S32_MAX;
 	}
@@ -12864,6 +12865,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 				u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct cfg80211_cqm_config *cqm_config = NULL, *old;
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int i, err;
@@ -12881,10 +12883,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
 		return -EOPNOTSUPP;
 
-	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
-	wdev_unlock(wdev);
-
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
 		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
 			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
@@ -12901,9 +12899,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		n_thresholds = 0;
 
 	wdev_lock(wdev);
+	old = rcu_dereference_protected(wdev->cqm_config,
+					lockdep_is_held(&wdev->mtx));
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
 		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
 						 n_thresholds),
 				     GFP_KERNEL);
@@ -12918,10 +12916,16 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		       flex_array_size(cqm_config, rssi_thresholds,
 				       n_thresholds));
 
-		wdev->cqm_config = cqm_config;
+		rcu_assign_pointer(wdev->cqm_config, cqm_config);
 	}
 
-	err = cfg80211_cqm_rssi_update(rdev, dev);
+	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
+	if (err) {
+		rcu_assign_pointer(wdev->cqm_config, old);
+		kfree(cqm_config);
+	} else {
+		kfree_rcu(old, rcu_head);
+	}
 
 unlock:
 	wdev_unlock(wdev);
@@ -19076,6 +19080,7 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 
 	trace_cfg80211_cqm_rssi_notify(dev, rssi_event, rssi_level);
 
@@ -19083,14 +19088,17 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	rcu_read_lock();
+	cqm_config = rcu_dereference(wdev->cqm_config);
+	if (cqm_config) {
+		cqm_config->last_rssi_event_value = rssi_level;
 
-		cfg80211_cqm_rssi_update(rdev, dev);
+		cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
 
 		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+			rssi_level = cqm_config->last_rssi_event_value;
 	}
+	rcu_read_unlock();
 
 	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
 	if (!msg)
-- 
2.41.0

Re: [RFC PATCH] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

Hello Johannes,

thanks for your patch.

While it works well in my lab setting, it crashes within minutes in the field. 

While the crashes look slightly different ("Unable to handle kernel pagign request"... descendant of is_swiotlb_active...) I think the notice beforehand is much more interesting: do you understand it?

: ------------[ cut here ]------------
: Voluntary context switch within RCU read-side critical section!
: WARNING: CPU: 0 PID: 319 at kernel/rcu/tree_plugin.h:320 rcu_note_context_switch+0x484/0x550
: Modules linked in: ov9281 rtc_pcf85063 regmap_i2c brcmfmac brcmutil v3d gpu_sched drm_shmem_helper cfg80211 gpio_keys i2c_mux_pinctrl i2c_mux vc4 rfkill raspberrypi_hwmon bcm2835_codec(C) bcm2835_v4l2(C) bcm2835_isp(C) bcm2835_unicam snd_soc_hdmi_codec v4l2_dv_timings bcm2835_mmal_vchiq(C) drm_display_helper rpivid_hevc(C) v4l2_fwnode v4l2_mem2mem videobuf2_vmalloc cec v4l2_async videobuf2_dma_contig i2c_brcmstb videobuf2_memops drm_dma_helper videobuf2_v4l2 i2c_bcm2835 drm_kms_helper snd_soc_core videobuf2_common videodev vc_sm_cma(C) snd_bcm2835(C) snd_compress snd_pcm_dmaengine snd_pcm snd_timer mc snd syscopyarea sysfillrect sysimgblt fb_sys_fops uio_pdrv_genirq nvmem_rmem uio drm drm_panel_orientation_quirks fuse backlight ip_tables x_tables ipv6
: CPU: 0 PID: 319 Comm: kworker/0:3 Tainted: G         C 6.1.45-v8-ged6bf58272ec #2
: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
: Workqueue: events brcmf_fweh_event_worker [brcmfmac]
: pstate: 600000c5 (nZCv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
: pc : rcu_note_context_switch+0x484/0x550
: lr : rcu_note_context_switch+0x484/0x550
: sp : ffffffc008d337c0
: x29: ffffffc008d337c0 x28: ffffff804765be00 x27: 000000000000001b
: x26: 0000000000000001 x25: 0000000000000000 x24: ffffff804765be00
: x23: ffffff804765be00 x22: 0000000000000000 x21: 0000000000000000
: x20: ffffffdcc58f4f00 x19: ffffff807fb5ff00 x18: 00000000fffffffc
: x17: 2e2e2e2e2e202020 x16: ffffffdcc53b9bb0 x15: 0000000000000020
: x14: ffffffdcc5c65358 x13: 216e6f6974636573 x12: 206c616369746972
: x11: 00000000000021c8 x10: 0000000000002180 x9 : ffffffdcc48fc5c4
: x8 : ffffffdcc5c0c788 x7 : ffffffdcc5c64788 x6 : 0000000000005538
: x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000027
: x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffffff804765be00
: Call trace:
:  rcu_note_context_switch+0x484/0x550
:  __schedule+0xc0/0xa10
:  schedule+0x60/0x100
:  schedule_timeout+0xa0/0x1c0
:  brcmf_sdio_bus_txctl+0xcc/0x1f4 [brcmfmac]
:  brcmf_proto_bcdc_msg+0xd4/0xf0 [brcmfmac]
:  brcmf_proto_bcdc_set_dcmd+0x88/0x124 [brcmfmac]
:  brcmf_fil_cmd_data+0x84/0x180 [brcmfmac]
:  brcmf_fil_iovar_data_set+0x11c/0x160 [brcmfmac]
:  brcmf_cfg80211_set_cqm_rssi_range_config+0xe4/0x130 [brcmfmac]
:  cfg80211_cqm_rssi_update+0x120/0x3f0 [cfg80211]
:  cfg80211_cqm_rssi_notify+0x78/0x1b4 [cfg80211]
:  brcmf_notify_rssi+0x10c/0x1b0 [brcmfmac]
:  brcmf_fweh_call_event_handler+0x40/0xa0 [brcmfmac]
:  brcmf_fweh_event_worker+0x1e4/0x4e0 [brcmfmac]
:  process_one_work+0x1dc/0x450
:  worker_thread+0x154/0x450
:  kthread+0x104/0x110
:  ret_from_fork+0x10/0x20
: ---[ end trace 0000000000000000 ]--- 

Re: [RFC PATCH] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

On Tue, 2023-08-15 at 12:56 +0200, Max Schulze wrote:
> Hello Johannes,
> 
> thanks for your patch.
> 
> While it works well in my lab setting, it crashes within minutes in the field. 
> 
> While the crashes look slightly different ("Unable to handle kernel pagign request"... descendant of is_swiotlb_active...) I think the notice beforehand is much more interesting: do you understand it?
> 
> : ------------[ cut here ]------------
> : Voluntary context switch within RCU read-side critical section!
[...]
> :  brcmf_sdio_bus_txctl+0xcc/0x1f4 [brcmfmac]
> :  brcmf_proto_bcdc_msg+0xd4/0xf0 [brcmfmac]
> :  brcmf_proto_bcdc_set_dcmd+0x88/0x124 [brcmfmac]
> :  brcmf_fil_cmd_data+0x84/0x180 [brcmfmac]
> :  brcmf_fil_iovar_data_set+0x11c/0x160 [brcmfmac]
> :  brcmf_cfg80211_set_cqm_rssi_range_config+0xe4/0x130 [brcmfmac]
> :  cfg80211_cqm_rssi_update+0x120/0x3f0 [cfg80211]
> :  cfg80211_cqm_rssi_notify+0x78/0x1b4 [cfg80211]
[...]

Oh, yeah, stupid me.

I did RCU protection around cfg80211_cqm_rssi_update() to have that
protected, but failed to realize that this will call back into the
driver too, which then promptly assumes it can sleep.

Well, OK, so this isn't how we can fix this.

That's really bad for multiple reasons though, because it also means we
call back into the driver from a driver call, which is generally not a
good idea since it can easily cause deadlocks.

Anyway, I guess I have to come up with something else. Thanks for
testing, and sorry I didn't realize that before.

johannes

[RFC PATCH v2] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Max Schulze reports crashes with brcmfmac. The reason seems
to be a race between userspace removing the CQM config and
the driver calling cfg80211_cqm_rssi_notify(), where if the
data is freed while cfg80211_cqm_rssi_notify() runs it will
crash since it assumes wdev->cqm_config is set. This can't
be fixed with a simple non-NULL check since there's nothing
we can do for locking easily, so use RCU instead to protect
the pointer, but that requires pulling the updates out into
an asynchronous worker so they can sleep and call back into
the driver.

Since we need to change the free anyway, also change it to
go back to the old settings if changing the settings fails.

Reported-by: Max Schulze <max.schulze@online.de>
Closes: https://lore.kernel.org/r/ac96309a-8d8d-4435-36e6-6d152eb31876@online.de
Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h |  3 +-
 net/wireless/core.c    | 14 +++----
 net/wireless/core.h    |  7 +++-
 net/wireless/nl80211.c | 84 ++++++++++++++++++++++++++++--------------
 4 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d6fa7c8767ad..c0250c81973e 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -6014,7 +6014,8 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct wiphy_work cqm_rssi_work;
+	struct cfg80211_cqm_config __rcu *cqm_config;
 
 	struct list_head pmsr_list;
 	spinlock_t pmsr_lock;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 25bc2e50a061..0f73e6373fe0 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1181,16 +1181,11 @@ void wiphy_rfkill_set_hw_state_reason(struct wiphy *wiphy, bool blocked,
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state_reason);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
-{
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
-}
-
 static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 				      bool unregister_netdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 	unsigned int link_id;
 
 	ASSERT_RTNL();
@@ -1227,7 +1222,10 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 	kfree_sensitive(wdev->wext.keys);
 	wdev->wext.keys = NULL;
 #endif
-	cfg80211_cqm_config_free(wdev);
+	/* deleted from the list, so can't be found from nl80211 any more */
+	cqm_config = rcu_access_pointer(wdev->cqm_config);
+	kfree_rcu(cqm_config, rcu_head);
+	wiphy_work_cancel(wdev->wiphy, &wdev->cqm_rssi_work);
 
 	/*
 	 * Ensure that all events have been processed and
@@ -1379,6 +1377,8 @@ void cfg80211_init_wdev(struct wireless_dev *wdev)
 	wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
 #endif
 
+	wiphy_work_init(&wdev->cqm_rssi_work, cfg80211_cqm_rssi_notify_work);
+
 	if (wdev->wiphy->flags & WIPHY_FLAG_PS_ON_BY_DEFAULT)
 		wdev->ps = true;
 	else
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 8a807b609ef7..86f209abc06a 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -295,12 +295,17 @@ struct cfg80211_beacon_registration {
 };
 
 struct cfg80211_cqm_config {
+	struct rcu_head rcu_head;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
+	enum nl80211_cqm_rssi_threshold_event last_rssi_event_type;
 	int n_rssi_thresholds;
 	s32 rssi_thresholds[];
 };
 
+void cfg80211_cqm_rssi_notify_work(struct wiphy *wiphy,
+				   struct wiphy_work *work);
+
 void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev);
 
 /* free object */
@@ -566,8 +571,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
-
 void cfg80211_release_pmsr(struct wireless_dev *wdev, u32 portid);
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev);
 void cfg80211_pmsr_free_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 8bcf8e293308..7513c549d577 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12796,7 +12796,8 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 }
 
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    struct cfg80211_cqm_config *cqm_config)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	s32 last, low, high;
@@ -12805,7 +12806,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!cqm_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -12814,7 +12815,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value &&
+	if (!cqm_config->last_rssi_event_value &&
 	    wdev->links[0].client.current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
@@ -12828,30 +12829,30 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 
 		cfg80211_sinfo_release_content(&sinfo);
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			cqm_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = cqm_config->last_rssi_event_value;
+	hyst = cqm_config->rssi_hyst;
+	n = cqm_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++) {
 		i = array_index_nospec(i, n);
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < cqm_config->rssi_thresholds[i])
 			break;
 	}
 
 	low_index = i - 1;
 	if (low_index >= 0) {
 		low_index = array_index_nospec(low_index, n);
-		low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+		low = cqm_config->rssi_thresholds[low_index] - hyst;
 	} else {
 		low = S32_MIN;
 	}
 	if (i < n) {
 		i = array_index_nospec(i, n);
-		high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+		high = cqm_config->rssi_thresholds[i] + hyst - 1;
 	} else {
 		high = S32_MAX;
 	}
@@ -12864,6 +12865,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 				u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct cfg80211_cqm_config *cqm_config = NULL, *old;
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int i, err;
@@ -12881,10 +12883,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
 		return -EOPNOTSUPP;
 
-	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
-	wdev_unlock(wdev);
-
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
 		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
 			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
@@ -12901,9 +12899,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		n_thresholds = 0;
 
 	wdev_lock(wdev);
+	old = rcu_dereference_protected(wdev->cqm_config,
+					lockdep_is_held(&wdev->mtx));
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
 		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
 						 n_thresholds),
 				     GFP_KERNEL);
@@ -12918,10 +12916,16 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		       flex_array_size(cqm_config, rssi_thresholds,
 				       n_thresholds));
 
-		wdev->cqm_config = cqm_config;
+		rcu_assign_pointer(wdev->cqm_config, cqm_config);
 	}
 
-	err = cfg80211_cqm_rssi_update(rdev, dev);
+	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
+	if (err) {
+		rcu_assign_pointer(wdev->cqm_config, old);
+		kfree(cqm_config);
+	} else {
+		kfree_rcu(old, rcu_head);
+	}
 
 unlock:
 	wdev_unlock(wdev);
@@ -19073,9 +19077,8 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      enum nl80211_cqm_rssi_threshold_event rssi_event,
 			      s32 rssi_level, gfp_t gfp)
 {
-	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 
 	trace_cfg80211_cqm_rssi_notify(dev, rssi_event, rssi_level);
 
@@ -19083,16 +19086,42 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	rcu_read_lock();
+	cqm_config = rcu_dereference(wdev->cqm_config);
+	if (cqm_config) {
+		cqm_config->last_rssi_event_value = rssi_level;
+		cqm_config->last_rssi_event_type = rssi_event;
+		wiphy_work_queue(wdev->wiphy, &wdev->cqm_rssi_work);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
-		cfg80211_cqm_rssi_update(rdev, dev);
+void cfg80211_cqm_rssi_notify_work(struct wiphy *wiphy, struct wiphy_work *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 cqm_rssi_work);
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	enum nl80211_cqm_rssi_threshold_event rssi_event;
+	struct cfg80211_cqm_config *cqm_config;
+	struct sk_buff *msg;
+	s32 rssi_level;
 
-		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+	wdev_lock(wdev);
+	cqm_config = rcu_dereference_protected(wdev->cqm_config,
+					       lockdep_is_held(&wdev->mtx));
+	if (!wdev->cqm_config) {
+		wdev_unlock(wdev);
+		return;
 	}
 
-	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
+	cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
+
+	rssi_level = cqm_config->last_rssi_event_value;
+	rssi_event = cqm_config->last_rssi_event_type;
+	wdev_unlock(wdev);
+
+	msg = cfg80211_prepare_cqm(wdev->netdev, NULL, GFP_KERNEL);
 	if (!msg)
 		return;
 
@@ -19104,14 +19133,13 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 				      rssi_level))
 		goto nla_put_failure;
 
-	cfg80211_send_cqm(msg, gfp);
+	cfg80211_send_cqm(msg, GFP_KERNEL);
 
 	return;
 
  nla_put_failure:
 	nlmsg_free(msg);
 }
-EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
 void cfg80211_cqm_txe_notify(struct net_device *dev,
 			     const u8 *peer, u32 num_packets,
-- 
2.41.0

Re: [RFC PATCH v2] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

Thanks again. I have to back-port it to 6.1.45 manually and will do testing.

The buildscript reports 2 things,


a)


Am 15.08.23 um 13:42 schrieb Johannes Berg:

> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index d6fa7c8767ad..c0250c81973e 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -6014,7 +6014,8 @@ struct wireless_dev {
>  	} wext;
>  #endif
>  
> -	struct cfg80211_cqm_config *cqm_config;
> +	struct wiphy_work cqm_rssi_work;
> +	struct cfg80211_cqm_config __rcu *cqm_config;

would *cqm_rssi_work be ok?


and b)


/root/linux-rpi/net/wireless/core.c: In function ‘_cfg80211_unregister_wdev’:
/root/linux-rpi/net/wireless/core.c:1167:9: error: implicit declaration of function ‘wiphy_work_cancel’ [-Werror=implicit-function-declaration]
 1167 |         wiphy_work_cancel(wdev->wiphy, &wdev->cqm_rssi_work);
      |         ^~~~~~~~~~~~~~~~~
/root/linux-rpi/net/wireless/core.c: In function ‘cfg80211_init_wdev’:
/root/linux-rpi/net/wireless/core.c:1319:2: error: implicit declaration of function ‘wiphy_work_init’; did you mean ‘wiphy_sysfs_init’? [-Werror=implicit-function-declaration]
 1319 |  wiphy_work_init(&wdev->cqm_rssi_work, cfg80211_cqm_rssi_notify_work);
      |  ^~~~~~~~~~~~~~~
      |  wiphy_sysfs_init


Is that something to worry about? (Building with warnings as error, I just disabled that for now)

Re: [RFC PATCH v2] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

On Tue, 2023-08-15 at 15:24 +0200, Max Schulze wrote:
> Thanks again. I have to back-port it to 6.1.45 manually and will do testing.
> 

Ah, you can't port this to 6.1 series, it depends on commit a3ee4dc84c4e
("wifi: cfg80211: add a work abstraction with special semantics") which
is only in 6.5-rc...

Maybe you can cherry-pick that patch too.

johannes

[RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Max Schulze reports crashes with brcmfmac. The reason seems
to be a race between userspace removing the CQM config and
the driver calling cfg80211_cqm_rssi_notify(), where if the
data is freed while cfg80211_cqm_rssi_notify() runs it will
crash since it assumes wdev->cqm_config is set. This can't
be fixed with a simple non-NULL check since there's nothing
we can do for locking easily, so use RCU instead to protect
the pointer, but that requires pulling the updates out into
an asynchronous worker so they can sleep and call back into
the driver.

Since we need to change the free anyway, also change it to
go back to the old settings if changing the settings fails.

Reported-by: Max Schulze <max.schulze@online.de>
Closes: https://lore.kernel.org/r/ac96309a-8d8d-4435-36e6-6d152eb31876@online.de
Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h |  3 +-
 net/wireless/core.c    | 14 +++----
 net/wireless/core.h    |  6 ++-
 net/wireless/nl80211.c | 84 ++++++++++++++++++++++++++++--------------
 4 files changed, 69 insertions(+), 38 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e09ff87146c1..0970eb47cb55 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5787,7 +5787,8 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct work_struct cqm_rssi_work;
+	struct cfg80211_cqm_config __rcu *cqm_config;
 
 	struct list_head pmsr_list;
 	spinlock_t pmsr_lock;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 609b79fe4a74..a86e3819dfd6 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1114,16 +1114,11 @@ void wiphy_rfkill_set_hw_state_reason(struct wiphy *wiphy, bool blocked,
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state_reason);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
-{
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
-}
-
 static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 				      bool unregister_netdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 	unsigned int link_id;
 
 	ASSERT_RTNL();
@@ -1166,7 +1161,10 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 	if (wdev->netdev)
 		flush_work(&wdev->disconnect_wk);
 
-	cfg80211_cqm_config_free(wdev);
+	cancel_work_sync(&wdev->cqm_rssi_work);
+	/* deleted from the list, so can't be found from nl80211 any more */
+	cqm_config = rcu_access_pointer(wdev->cqm_config);
+	kfree_rcu(cqm_config, rcu_head);
 
 	/*
 	 * Ensure that all events have been processed and
@@ -1318,6 +1316,8 @@ void cfg80211_init_wdev(struct wireless_dev *wdev)
 	wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
 #endif
 
+	INIT_WORK(&wdev->cqm_rssi_work, cfg80211_cqm_rssi_notify_work);
+
 	if (wdev->wiphy->flags & WIPHY_FLAG_PS_ON_BY_DEFAULT)
 		wdev->ps = true;
 	else
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 775e16cb99ed..bd63419fa8f8 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -287,12 +287,16 @@ struct cfg80211_beacon_registration {
 };
 
 struct cfg80211_cqm_config {
+	struct rcu_head rcu_head;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
+	enum nl80211_cqm_rssi_threshold_event last_rssi_event_type;
 	int n_rssi_thresholds;
 	s32 rssi_thresholds[];
 };
 
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work);
+
 void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev);
 
 /* free object */
@@ -556,8 +560,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
-
 void cfg80211_release_pmsr(struct wireless_dev *wdev, u32 portid);
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev);
 void cfg80211_pmsr_free_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 087c0c442e23..6ab534f058d0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12561,7 +12561,8 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 }
 
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    struct cfg80211_cqm_config *cqm_config)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	s32 last, low, high;
@@ -12570,7 +12571,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!cqm_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -12579,7 +12580,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value &&
+	if (!cqm_config->last_rssi_event_value &&
 	    wdev->links[0].client.current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
@@ -12593,30 +12594,30 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 
 		cfg80211_sinfo_release_content(&sinfo);
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			cqm_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = cqm_config->last_rssi_event_value;
+	hyst = cqm_config->rssi_hyst;
+	n = cqm_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++) {
 		i = array_index_nospec(i, n);
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < cqm_config->rssi_thresholds[i])
 			break;
 	}
 
 	low_index = i - 1;
 	if (low_index >= 0) {
 		low_index = array_index_nospec(low_index, n);
-		low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+		low = cqm_config->rssi_thresholds[low_index] - hyst;
 	} else {
 		low = S32_MIN;
 	}
 	if (i < n) {
 		i = array_index_nospec(i, n);
-		high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+		high = cqm_config->rssi_thresholds[i] + hyst - 1;
 	} else {
 		high = S32_MAX;
 	}
@@ -12629,6 +12630,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 				u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct cfg80211_cqm_config *cqm_config = NULL, *old;
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int i, err;
@@ -12646,10 +12648,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
 		return -EOPNOTSUPP;
 
-	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
-	wdev_unlock(wdev);
-
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
 		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
 			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
@@ -12666,9 +12664,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		n_thresholds = 0;
 
 	wdev_lock(wdev);
+	old = rcu_dereference_protected(wdev->cqm_config,
+					lockdep_is_held(&wdev->mtx));
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
 		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
 						 n_thresholds),
 				     GFP_KERNEL);
@@ -12683,10 +12681,16 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		       flex_array_size(cqm_config, rssi_thresholds,
 				       n_thresholds));
 
-		wdev->cqm_config = cqm_config;
+		rcu_assign_pointer(wdev->cqm_config, cqm_config);
 	}
 
-	err = cfg80211_cqm_rssi_update(rdev, dev);
+	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
+	if (err) {
+		rcu_assign_pointer(wdev->cqm_config, old);
+		kfree(cqm_config);
+	} else {
+		kfree_rcu(old, rcu_head);
+	}
 
 unlock:
 	wdev_unlock(wdev);
@@ -18715,9 +18719,8 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      enum nl80211_cqm_rssi_threshold_event rssi_event,
 			      s32 rssi_level, gfp_t gfp)
 {
-	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 
 	trace_cfg80211_cqm_rssi_notify(dev, rssi_event, rssi_level);
 
@@ -18725,16 +18728,42 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	rcu_read_lock();
+	cqm_config = rcu_dereference(wdev->cqm_config);
+	if (cqm_config) {
+		cqm_config->last_rssi_event_value = rssi_level;
+		cqm_config->last_rssi_event_type = rssi_event;
+		schedule_work(&wdev->cqm_rssi_work);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
-		cfg80211_cqm_rssi_update(rdev, dev);
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 cqm_rssi_work);
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	enum nl80211_cqm_rssi_threshold_event rssi_event;
+	struct cfg80211_cqm_config *cqm_config;
+	struct sk_buff *msg;
+	s32 rssi_level;
 
-		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+	wdev_lock(wdev);
+	cqm_config = rcu_dereference_protected(wdev->cqm_config,
+					       lockdep_is_held(&wdev->mtx));
+	if (!wdev->cqm_config) {
+		wdev_unlock(wdev);
+		return;
 	}
 
-	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
+	cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
+
+	rssi_level = cqm_config->last_rssi_event_value;
+	rssi_event = cqm_config->last_rssi_event_type;
+	wdev_unlock(wdev);
+
+	msg = cfg80211_prepare_cqm(wdev->netdev, NULL, GFP_KERNEL);
 	if (!msg)
 		return;
 
@@ -18746,14 +18775,13 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 				      rssi_level))
 		goto nla_put_failure;
 
-	cfg80211_send_cqm(msg, gfp);
+	cfg80211_send_cqm(msg, GFP_KERNEL);
 
 	return;
 
  nla_put_failure:
 	nlmsg_free(msg);
 }
-EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
 void cfg80211_cqm_txe_notify(struct net_device *dev,
 			     const u8 *peer, u32 num_packets,
-- 
2.41.0

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

Johannes, thank you very much.

However, bad luck again, I get the following crash. Scouring the logs, this time I have no "rcu notice" or any other error beforehand.

But the timing again suggest a strong connection with RSSI update in progress.


h63 wpa_supplicant[433]: wlan0: CTRL-EVENT-CONNECTED - Connection to xx:xx:xx:xx:00:91 completed [id=0 id_str=]
h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -57 dBm
h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -57 dBm
h63 wpa_supplicant[433]: wlan0: CTRL-EVENT-SUBNET-STATUS-UPDATE status=0
h63 NetworkManager[429]: <info>  [1692169608.6737] device (wlan0): supplicant interface state: associating -> completed
h63 NetworkManager[429]: <info>  [1692169608.6764] device (p2p-dev-wlan0): supplicant management interface state: associating -> completed
h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -57 dBm
h63 kernel: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000414022d6
h63 kernel: brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:00:00:00:00
h63 kernel: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0
h63 kernel: brcmutil: event payload, len=12
h63 kernel: 00000000: ff ff ff c6 00 00 00 00 ff ff ff a5 ............
h63 kernel: brcmfmac: brcmf_netdev_start_xmit wlan0: insufficient headroom (0)
h63 kernel: Unable to handle kernel paging request at virtual address ffff8061d9850028
h63 kernel: Mem abort info:
h63 kernel:   ESR = 0x0000000096000004
h63 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits
h63 kernel:   SET = 0, FnV = 0
h63 kernel:   EA = 0, S1PTW = 0
h63 kernel:   FSC = 0x04: level 0 translation fault
h63 kernel: Data abort info:
h63 kernel:   ISV = 0, ISS = 0x00000004
h63 kernel:   CM = 0, WnR = 0
h63 kernel: [ffff8061d9850028] address between user and kernel address ranges
h63 kernel: Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
h63 kernel: Modules linked in: rtc_pcf85063 regmap_i2c ov9281 v3d brcmfmac vc4 gpu_sched raspberrypi_hwmon drm_shmem_helper binfmt_misc brcmutil gpio_keys snd_soc_hdmi_codec i2c_mux_pinctrl>
h63 kernel: CPU: 3 PID: 493 Comm: Xorg Tainted: G         C 6.1.45-v8-gdc69f9d60872 #3
h63 kernel: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
h63 kernel: pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
h63 kernel: pc : dma_resv_add_fence+0x80/0x260
h63 kernel: lr : v3d_attach_fences_and_unlock_reservation+0x6c/0x1c0 [v3d]
h63 kernel: sp : ffffffc0093abb00
h63 kernel: x29: ffffffc0093abb00 x28: 0000000000000048 x27: ffffffc0093abd38
h63 kernel: x26: 0000000000000001 x25: ffffff80481e8e00 x24: ffffffc0093abc08
h63 kernel: x23: 0000000000000001 x22: ffffff805701ac40 x21: ffffffc0093abc20
h63 kernel: x20: ffffff804b995200 x19: ffff8061d9850000 x18: 0000000000000000
h63 kernel: x17: 0000000000000000 x16: ffffffdcdd1b9ba0 x15: 00000055b7e860c0
h63 kernel: x14: 0000000000000000 x13: 0000000000000202 x12: 0000000000000202
h63 kernel: x11: 0000000000000202 x10: 0000000000001a90 x9 : ffffffdc7c7bdecc
h63 kernel: x8 : 0000000000000228 x7 : ffffff804416caa8 x6 : 0000000000000000
h63 kernel: x5 : ffffff805701ac40 x4 : ffffff805701ac78 x3 : 0000000000000000
h63 kernel: x2 : ffffffdc7c685478 x1 : ffffffdcdd680ce8 x0 : ffffffdcdd680d30
h63 kernel: Call trace:
h63 kernel:  dma_resv_add_fence+0x80/0x260
h63 kernel:  v3d_attach_fences_and_unlock_reservation+0x6c/0x1c0 [v3d]
h63 kernel:  v3d_submit_cl_ioctl+0x540/0x70c [v3d]
h63 kernel:  drm_ioctl_kernel+0xcc/0x180 [drm]
h63 kernel:  drm_ioctl+0x210/0x440 [drm]
h63 kernel:  __arm64_sys_ioctl+0xb0/0xf4
h63 kernel:  invoke_syscall+0x50/0x120
h63 kernel:  el0_svc_common.constprop.0+0x68/0x124
h63 kernel:  do_el0_svc+0x34/0xd0
h63 kernel:  el0_svc+0x30/0x94
h63 kernel:  el0t_64_sync_handler+0xb8/0xbc
h63 kernel:  el0t_64_sync+0x18c/0x190
h63 kernel: Code: fa401044 54000061 d4210000 d503201f (f9401678)
h63 kernel: ---[ end trace 0000000000000000 ]---

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

Hi,

This is ... pretty odd?

> h63 kernel: Unable to handle kernel paging request at virtual address ffff8061d9850028
> h63 kernel: Mem abort info:
> h63 kernel:   ESR = 0x0000000096000004
> h63 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits
> h63 kernel:   SET = 0, FnV = 0
> h63 kernel:   EA = 0, S1PTW = 0
> h63 kernel:   FSC = 0x04: level 0 translation fault
> h63 kernel: Data abort info:
> h63 kernel:   ISV = 0, ISS = 0x00000004
> h63 kernel:   CM = 0, WnR = 0
> h63 kernel: [ffff8061d9850028] address between user and kernel address ranges
> h63 kernel: Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> h63 kernel: Modules linked in: [...]
> h63 kernel: CPU: 3 PID: 493 Comm: Xorg Tainted: G         C 6.1.45-v8-gdc69f9d60872 #3
> h63 kernel: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
> h63 kernel: pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> h63 kernel: pc : dma_resv_add_fence+0x80/0x260

I mean, that's in some other code entirely?

> h63 kernel: Call trace:
> h63 kernel:  dma_resv_add_fence+0x80/0x260
> h63 kernel:  v3d_attach_fences_and_unlock_reservation+0x6c/0x1c0 [v3d]
> h63 kernel:  v3d_submit_cl_ioctl+0x540/0x70c [v3d]
> h63 kernel:  drm_ioctl_kernel+0xcc/0x180 [drm]
> h63 kernel:  drm_ioctl+0x210/0x440 [drm]
> h63 kernel:  __arm64_sys_ioctl+0xb0/0xf4
> h63 kernel:  invoke_syscall+0x50/0x120
> h63 kernel:  el0_svc_common.constprop.0+0x68/0x124
> h63 kernel:  do_el0_svc+0x34/0xd0
> h63 kernel:  el0_svc+0x30/0x94
> h63 kernel:  el0t_64_sync_handler+0xb8/0xbc
> h63 kernel:  el0t_64_sync+0x18c/0x190
> h63 kernel: Code: fa401044 54000061 d4210000 d503201f (f9401678)

Unless my code caused some really bad corruption somewhere, I'm not sure
I see how that would happen?


Were you able to reproduce it multiple times? Maybe we can see a pattern
out of multiple reports.

johannes

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

You are right, I have no clear proof.

However, here is a second trace from the same system.
Another system has crashed, I have not recovered the trace yet.

The context, that there is a RSSI notification just beforehand, is exactly the same (as the initial bug report).

You have a clue?


Aug 16 11:03:32 h63 wpa_supplicant[389]: wlan0: Associated with 
xx:xx:xx:xx:00:91
Aug 16 11:03:32 h63 wpa_supplicant[389]: wlan0: CTRL-EVENT-CONNECTED - 
Connection to xx:xx:xx:xx:00:91 completed [id=0 id_str=]
Aug 16 11:03:32 h63 wpa_supplicant[389]: wlan0: 
CTRL-EVENT-SUBNET-STATUS-UPDATE status=0
Aug 16 11:03:32 h63 NetworkManager[373]: <info>  [1692176612.2898] 
device (wlan0): supplicant interface state: associating -> completed
Aug 16 11:03:32 h63 NetworkManager[373]: <info>  [1692176612.2933] 
device (p2p-dev-wlan0): supplicant management interface state: 
associating -> completed
Aug 16 11:03:32 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-63 dBm
Aug 16 11:03:32 h63 kernel: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: 
rxp=00000000b1aa24a9
Aug 16 11:03:32 h63 kernel: brcmfmac: brcmf_fweh_event_worker event RSSI 
(56) ifidx 0 bsscfg 0 addr 00:00:00:ab:c0:c1
Aug 16 11:03:32 h63 kernel: brcmfmac: brcmf_fweh_event_worker   version 
2 flags 0 status 0 reason 0
Aug 16 11:03:32 h63 kernel: brcmutil: event payload, len=12
Aug 16 11:03:32 h63 kernel: 00000000: ff ff ff c1 00 00 00 00 ff ff ff 
a5              ............
Aug 16 11:03:35 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-63 dBm
Aug 16 11:03:40 h63 kernel: Unable to handle kernel NULL pointer 
dereference at virtual address 00000000000000cc
Aug 16 11:03:40 h63 kernel: Mem abort info:
Aug 16 11:03:41 h63 kernel:   ESR = 0x0000000096000005
Aug 16 11:03:41 h63 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits
Aug 16 11:03:41 h63 kernel:   SET = 0, FnV = 0
Aug 16 11:03:41 h63 kernel:   EA = 0, S1PTW = 0
Aug 16 11:03:41 h63 kernel:   FSC = 0x05: level 1 translation fault
Aug 16 11:03:41 h63 kernel: Data abort info:
Aug 16 11:03:41 h63 kernel:   ISV = 0, ISS = 0x00000005
Aug 16 11:03:41 h63 kernel:   CM = 0, WnR = 0
Aug 16 11:03:41 h63 kernel: user pgtable: 4k pages, 39-bit VAs, 
pgdp=0000000049427000
Aug 16 11:03:41 h63 kernel: [00000000000000cc] pgd=0000000000000000, 
p4d=0000000000000000, pud=0000000000000000
Aug 16 11:03:41 h63 kernel: Internal error: Oops: 0000000096000005 [#1] 
PREEMPT SMP
Aug 16 11:03:41 h63 kernel: Modules linked in: rtc_pcf85063 ov9281 
regmap_i2c brcmfmac vc4 brcmutil cfg80211 snd_soc_hdmi_codec 
drm_display_helper cec drm_dma_helper drm_kms_helper v3d gpu_sched 
binfmt_mis>
Aug 16 11:03:41 h63 kernel: CPU: 2 PID: 491 Comm: Xorg Tainted: G 
 C         6.1.45-v8-gdc69f9d60872 #3
Aug 16 11:03:41 h63 kernel: Hardware name: Raspberry Pi Compute Module 4 
Rev 1.0 (DT)
Aug 16 11:03:41 h63 kernel: pstate: 80000005 (Nzcv daif -PAN -UAO -TCO 
-DIT -SSBS BTYPE=--)
Aug 16 11:03:41 h63 kernel: pc : drm_ioctl+0x284/0x440 [drm]
Aug 16 11:03:41 h63 kernel: lr : drm_ioctl+0xac/0x440 [drm]
Aug 16 11:03:41 h63 kernel: sp : ffffffc00935bca0
Aug 16 11:03:41 h63 kernel: x29: ffffffc00935bcb0 x28: ffffff8045499f00 
x27: 0000000000000000
Aug 16 11:03:41 h63 kernel: x26: 0000000000000000 x25: ffffff804806a800 
x24: 0000000000000041
Aug 16 11:03:41 h63 kernel: x23: ffffff804946d900 x22: 00000000c0106441 
x21: 0000007fc2a766f8
Aug 16 11:03:41 h63 kernel: x20: ffffff8040370000 x19: 0000000000000001 
x18: 0000000000000000
Aug 16 11:03:41 h63 kernel: x17: 0000000000000000 x16: ffffffe34ff1a2d0 
x15: 0000000000000000
Aug 16 11:03:41 h63 kernel: x14: 0000000000000000 x13: 0000000000000000 
x12: 0000000000000000
Aug 16 11:03:41 h63 kernel: x11: 0000000000000000 x10: 0000000000000000 
x9 : ffffffe315a9eed0
Aug 16 11:03:41 h63 kernel: x8 : 0000000000000000 x7 : 0000000000000000 
x6 : 0000000000159ba4
Aug 16 11:03:41 h63 kernel: x5 : 0000000000159ba5 x4 : 0000000000000000 
x3 : 0000000000000001
Aug 16 11:03:41 h63 kernel: x2 : ffffff8045499f00 x1 : ffffff9d2eca5000 
x0 : 0000000000000004
Aug 16 11:03:41 h63 kernel: Call trace:
Aug 16 11:03:41 h63 kernel:  drm_ioctl+0x284/0x440 [drm]
Aug 16 11:03:41 h63 kernel:  __arm64_sys_ioctl+0xb0/0xf4
Aug 16 11:03:41 h63 kernel:  invoke_syscall+0x50/0x120
Aug 16 11:03:41 h63 kernel:  el0_svc_common.constprop.0+0x68/0x124
Aug 16 11:03:41 h63 kernel:  do_el0_svc+0x34/0xd0
Aug 16 11:03:41 h63 kernel:  el0_svc+0x30/0x94
Aug 16 11:03:41 h63 kernel:  el0t_64_sync_handler+0xb8/0xbc
Aug 16 11:03:41 h63 kernel:  el0t_64_sync+0x18c/0x190
Aug 16 11:03:41 h63 kernel: Code: 35000455 a94673fb 17ffff7b f9401a80 
(b940c800)
Aug 16 11:03:41 h63 kernel: ---[ end trace 0000000000000000 ]---
Aug 16 11:03:41 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-67 dBm
Aug 16 11:03:45 h63 kernel: brcmfmac: brcmf_netdev_start_xmit wlan0: 
insufficient headroom (0)
Aug 16 11:03:45 h63 kernel: brcmfmac: brcmf_netdev_start_xmit wlan0: 
insufficient headroom (0)

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

Trace from the system I couldn't reach.

Looks similar to the first.

There must be something at odds with the patch.

Aug 16 09:27:03 h91 wpa_supplicant[367]: wlan0: Associated with 
xx:xx:xx:xx:a1:80
Aug 16 09:27:03 h91 wpa_supplicant[367]: wlan0: CTRL-EVENT-CONNECTED - 
Connection to xx:xx:xx:xx:a1:80 completed [id=0 id_str=]
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-54 dBm
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_setup_wiphybands nmode=1, 
vhtmode=1, bw_cap=(3, 7)
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_setup_wiphybands nchain=1
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-54 dBm
Aug 16 09:27:03 h91 wpa_supplicant[367]: wlan0: 
CTRL-EVENT-SUBNET-STATUS-UPDATE status=0
Aug 16 09:27:03 h91 wpa_supplicant[367]: wlan0: CTRL-EVENT-REGDOM-CHANGE 
init=COUNTRY_IE type=COUNTRY alpha2=US
Aug 16 09:27:03 h91 NetworkManager[342]: <info>  [1692170823.3861] 
device (wlan0): supplicant interface state: associating -> completed
Aug 16 09:27:03 h91 NetworkManager[342]: <info>  [1692170823.3862] 
device (p2p-dev-wlan0): supplicant management interface state: 
associating -> completed
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: 
rxp=000000007fc0b9df
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_fweh_event_worker event RSSI 
(56) ifidx 0 bsscfg 0 addr 00:00:00:00:00:00
Aug 16 09:27:03 h91 kernel: brcmfmac: brcmf_fweh_event_worker   version 
2 flags 0 status 0 reason 0
Aug 16 09:27:03 h91 kernel: brcmutil: event payload, len=12
Aug 16 09:27:03 h91 kernel: 00000000: ff ff ff ca 00 00 00 00 ff ff ff 
a5              ............
Aug 16 09:27:04 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-57 dBm
Aug 16 09:27:05 h91 kernel: Unable to handle kernel NULL pointer 
dereference at virtual address 00000000000000cf
Aug 16 09:27:05 h91 kernel: Mem abort info:
Aug 16 09:27:05 h91 kernel:   ESR = 0x0000000096000005
Aug 16 09:27:05 h91 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits
Aug 16 09:27:05 h91 kernel:   SET = 0, FnV = 0
Aug 16 09:27:05 h91 kernel:   EA = 0, S1PTW = 0
Aug 16 09:27:05 h91 kernel:   FSC = 0x05: level 1 translation fault
Aug 16 09:27:05 h91 kernel: Data abort info:
Aug 16 09:27:05 h91 kernel:   ISV = 0, ISS = 0x00000005
Aug 16 09:27:05 h91 kernel:   CM = 0, WnR = 0
Aug 16 09:27:05 h91 kernel: user pgtable: 4k pages, 39-bit VAs, 
pgdp=000000004b56b000
Aug 16 09:27:05 h91 kernel: [00000000000000cf] pgd=0000000000000000, 
p4d=0000000000000000, pud=0000000000000000
Aug 16 09:27:05 h91 kernel: Internal error: Oops: 0000000096000005 [#1] 
PREEMPT SMP
Aug 16 09:27:05 h91 kernel: Modules linked in: ov9281 rtc_pcf85063 
regmap_i2c vc4 snd_soc_hdmi_codec drm_display_helper cec drm_dma_helper 
v3d brcmfmac drm_kms_helper gpu_sched drm_shmem_helper brcmutil g>
Aug 16 09:27:05 h91 kernel: CPU: 3 PID: 488 Comm: Xorg Tainted: G 
 C         6.1.45-v8-gdc69f9d60872 #3
Aug 16 09:27:05 h91 kernel: Hardware name: Raspberry Pi Compute Module 4 
Rev 1.0 (DT)
Aug 16 09:27:05 h91 kernel: pstate: 80000005 (Nzcv daif -PAN -UAO -TCO 
-DIT -SSBS BTYPE=--)
Aug 16 09:27:05 h91 kernel: pc : drm_ioctl+0x284/0x440 [drm]
Aug 16 09:27:05 h91 kernel: lr : drm_ioctl+0xac/0x440 [drm]
Aug 16 09:27:05 h91 kernel: sp : ffffffc00933bca0
Aug 16 09:27:05 h91 kernel: x29: ffffffc00933bcb0 x28: ffffff8044acdd00 
x27: 0000000000000000
Aug 16 09:27:05 h91 kernel: x26: 0000000000000000 x25: ffffff804376a800 
x24: 0000000000000041
Aug 16 09:27:05 h91 kernel: x23: ffffff804e849200 x22: 00000000c0106441 
x21: 0000007ffd46ebf8
Aug 16 09:27:05 h91 kernel: x20: ffffff8040a4e000 x19: 0000000000000001 
x18: 0000000000000000
Aug 16 09:27:05 h91 kernel: x17: 0000000000000000 x16: ffffffea06d1a2d0 
x15: 0000000000000000
Aug 16 09:27:05 h91 kernel: x14: 0000000000000000 x13: 0000000000000000 
x12: 0000000000000000
Aug 16 09:27:05 h91 kernel: x11: 0000000000000000 x10: 0000000000000000 
x9 : ffffffe9a6773ed0
Aug 16 09:27:05 h91 kernel: x8 : 0000000000000000 x7 : 0000000000000000 
x6 : 000000000005e9e5
Aug 16 09:27:05 h91 kernel: x5 : 000000000005e9e5 x4 : 0000000000000000 
x3 : 0000000000000001
Aug 16 09:27:05 h91 kernel: x2 : ffffff8044acdd00 x1 : ffffff9677ec2000 
x0 : 0000000000000007
Aug 16 09:27:05 h91 kernel: Call trace:
Aug 16 09:27:05 h91 kernel:  drm_ioctl+0x284/0x440 [drm]
Aug 16 09:27:05 h91 kernel:  __arm64_sys_ioctl+0xb0/0xf4
Aug 16 09:27:05 h91 kernel:  invoke_syscall+0x50/0x120
Aug 16 09:27:05 h91 kernel:  el0_svc_common.constprop.0+0x68/0x124
Aug 16 09:27:05 h91 kernel:  do_el0_svc+0x34/0xd0
Aug 16 09:27:05 h91 kernel:  el0_svc+0x30/0x94
Aug 16 09:27:05 h91 kernel:  el0t_64_sync_handler+0xb8/0xbc
Aug 16 09:27:05 h91 kernel:  el0t_64_sync+0x18c/0x190
Aug 16 09:27:05 h91 kernel: Code: 35000455 a94673fb 17ffff7b f9401a80 
(b940c800)
Aug 16 09:27:05 h91 kernel: ---[ end trace 0000000000000000 ]---
Aug 16 09:27:10 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-58 dBm
Aug 16 09:27:16 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI 
-58 dBm

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

On Wed, 2023-08-16 at 15:08 +0200, Max Schulze wrote:
> Trace from the system I couldn't reach.
> 
> Looks similar to the first.

Thanks.

> There must be something at odds with the patch.

Yeah I guess I haven't made it any better, have I :)


> Aug 16 09:27:04 h91 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -57 dBm

It's kind of interesting that we see this preceding the crash - even if
the crash then is in something else entirely. Makes me think something
else got corrupted here. I'll take a look at the patch again, maybe I
can spot something.

johannes

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

On Tue, 2023-08-15 at 15:37 +0200, Johannes Berg wrote:
> 
>  
> -	err = cfg80211_cqm_rssi_update(rdev, dev);
> +	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
> +	if (err) {
> +		rcu_assign_pointer(wdev->cqm_config, old);
> +		kfree(cqm_config);


OK, that's wrong wrt. RCU handling, maybe that causes heap corruption if
someone sees wdev->cqm_config under RCU but it was freed and re-
allocated.

I think I'll just remove this failure path handling change entirely and
do it separately.

johannes

[RFC PATCH v3 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Max Schulze reports crashes with brcmfmac. The reason seems
to be a race between userspace removing the CQM config and
the driver calling cfg80211_cqm_rssi_notify(), where if the
data is freed while cfg80211_cqm_rssi_notify() runs it will
crash since it assumes wdev->cqm_config is set. This can't
be fixed with a simple non-NULL check since there's nothing
we can do for locking easily, so use RCU instead to protect
the pointer, but that requires pulling the updates out into
an asynchronous worker so they can sleep and call back into
the driver.

Since we need to change the free anyway, also change it to
go back to the old settings if changing the settings fails.

Reported-by: Max Schulze <max.schulze@online.de>
Closes: https://lore.kernel.org/r/ac96309a-8d8d-4435-36e6-6d152eb31876@online.de
Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h |  3 +-
 net/wireless/core.c    | 14 +++----
 net/wireless/core.h    |  6 ++-
 net/wireless/nl80211.c | 84 ++++++++++++++++++++++++++++--------------
 4 files changed, 69 insertions(+), 38 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e09ff87146c1..0970eb47cb55 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5787,7 +5787,8 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct work_struct cqm_rssi_work;
+	struct cfg80211_cqm_config __rcu *cqm_config;
 
 	struct list_head pmsr_list;
 	spinlock_t pmsr_lock;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 609b79fe4a74..a86e3819dfd6 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1114,16 +1114,11 @@ void wiphy_rfkill_set_hw_state_reason(struct wiphy *wiphy, bool blocked,
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state_reason);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
-{
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
-}
-
 static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 				      bool unregister_netdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 	unsigned int link_id;
 
 	ASSERT_RTNL();
@@ -1166,7 +1161,10 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 	if (wdev->netdev)
 		flush_work(&wdev->disconnect_wk);
 
-	cfg80211_cqm_config_free(wdev);
+	cancel_work_sync(&wdev->cqm_rssi_work);
+	/* deleted from the list, so can't be found from nl80211 any more */
+	cqm_config = rcu_access_pointer(wdev->cqm_config);
+	kfree_rcu(cqm_config, rcu_head);
 
 	/*
 	 * Ensure that all events have been processed and
@@ -1318,6 +1316,8 @@ void cfg80211_init_wdev(struct wireless_dev *wdev)
 	wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
 #endif
 
+	INIT_WORK(&wdev->cqm_rssi_work, cfg80211_cqm_rssi_notify_work);
+
 	if (wdev->wiphy->flags & WIPHY_FLAG_PS_ON_BY_DEFAULT)
 		wdev->ps = true;
 	else
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 775e16cb99ed..bd63419fa8f8 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -287,12 +287,16 @@ struct cfg80211_beacon_registration {
 };
 
 struct cfg80211_cqm_config {
+	struct rcu_head rcu_head;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
+	enum nl80211_cqm_rssi_threshold_event last_rssi_event_type;
 	int n_rssi_thresholds;
 	s32 rssi_thresholds[];
 };
 
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work);
+
 void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev);
 
 /* free object */
@@ -556,8 +560,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
-
 void cfg80211_release_pmsr(struct wireless_dev *wdev, u32 portid);
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev);
 void cfg80211_pmsr_free_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 087c0c442e23..7a8d3457ee99 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12561,7 +12561,8 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 }
 
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    struct cfg80211_cqm_config *cqm_config)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	s32 last, low, high;
@@ -12570,7 +12571,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!cqm_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -12579,7 +12580,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value &&
+	if (!cqm_config->last_rssi_event_value &&
 	    wdev->links[0].client.current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
@@ -12593,30 +12594,30 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 
 		cfg80211_sinfo_release_content(&sinfo);
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			cqm_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = cqm_config->last_rssi_event_value;
+	hyst = cqm_config->rssi_hyst;
+	n = cqm_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++) {
 		i = array_index_nospec(i, n);
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < cqm_config->rssi_thresholds[i])
 			break;
 	}
 
 	low_index = i - 1;
 	if (low_index >= 0) {
 		low_index = array_index_nospec(low_index, n);
-		low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+		low = cqm_config->rssi_thresholds[low_index] - hyst;
 	} else {
 		low = S32_MIN;
 	}
 	if (i < n) {
 		i = array_index_nospec(i, n);
-		high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+		high = cqm_config->rssi_thresholds[i] + hyst - 1;
 	} else {
 		high = S32_MAX;
 	}
@@ -12629,6 +12630,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 				u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct cfg80211_cqm_config *cqm_config = NULL, *old;
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int i, err;
@@ -12646,10 +12648,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
 		return -EOPNOTSUPP;
 
-	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
-	wdev_unlock(wdev);
-
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
 		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
 			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
@@ -12666,9 +12664,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		n_thresholds = 0;
 
 	wdev_lock(wdev);
+	old = rcu_dereference_protected(wdev->cqm_config,
+					lockdep_is_held(&wdev->mtx));
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
 		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
 						 n_thresholds),
 				     GFP_KERNEL);
@@ -12683,10 +12681,16 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		       flex_array_size(cqm_config, rssi_thresholds,
 				       n_thresholds));
 
-		wdev->cqm_config = cqm_config;
+		rcu_assign_pointer(wdev->cqm_config, cqm_config);
 	}
 
-	err = cfg80211_cqm_rssi_update(rdev, dev);
+	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
+	if (err) {
+		rcu_assign_pointer(wdev->cqm_config, old);
+		kfree_rcu(cqm_config, rcu_head);
+	} else {
+		kfree_rcu(old, rcu_head);
+	}
 
 unlock:
 	wdev_unlock(wdev);
@@ -18715,9 +18719,8 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      enum nl80211_cqm_rssi_threshold_event rssi_event,
 			      s32 rssi_level, gfp_t gfp)
 {
-	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 
 	trace_cfg80211_cqm_rssi_notify(dev, rssi_event, rssi_level);
 
@@ -18725,16 +18728,42 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	rcu_read_lock();
+	cqm_config = rcu_dereference(wdev->cqm_config);
+	if (cqm_config) {
+		cqm_config->last_rssi_event_value = rssi_level;
+		cqm_config->last_rssi_event_type = rssi_event;
+		schedule_work(&wdev->cqm_rssi_work);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
-		cfg80211_cqm_rssi_update(rdev, dev);
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 cqm_rssi_work);
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	enum nl80211_cqm_rssi_threshold_event rssi_event;
+	struct cfg80211_cqm_config *cqm_config;
+	struct sk_buff *msg;
+	s32 rssi_level;
 
-		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+	wdev_lock(wdev);
+	cqm_config = rcu_dereference_protected(wdev->cqm_config,
+					       lockdep_is_held(&wdev->mtx));
+	if (!wdev->cqm_config) {
+		wdev_unlock(wdev);
+		return;
 	}
 
-	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
+	cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
+
+	rssi_level = cqm_config->last_rssi_event_value;
+	rssi_event = cqm_config->last_rssi_event_type;
+	wdev_unlock(wdev);
+
+	msg = cfg80211_prepare_cqm(wdev->netdev, NULL, GFP_KERNEL);
 	if (!msg)
 		return;
 
@@ -18746,14 +18775,13 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 				      rssi_level))
 		goto nla_put_failure;
 
-	cfg80211_send_cqm(msg, gfp);
+	cfg80211_send_cqm(msg, GFP_KERNEL);
 
 	return;
 
  nla_put_failure:
 	nlmsg_free(msg);
 }
-EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
 void cfg80211_cqm_txe_notify(struct net_device *dev,
 			     const u8 *peer, u32 num_packets,
-- 
2.41.0

Re: [RFC PATCH v2 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

Look, this trace points to something in the worker structure/mm:

14:04:50 h63 wpa_supplicant[376]: wlan0: CTRL-EVENT-CONNECTED - Connection to xx:xx:xx:xx:c7:a8 completed [id=0 id_str=]

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams ACI 0 aifsn 3 acm 0 ecwmin 4 ecwmax 10

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams ACI 1 aifsn 7 acm 0 ecwmin 4 ecwmax 10

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams ACI 2 aifsn 2 acm 0 ecwmin 3 ecwmax 4

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams ACI 3 aifsn 2 acm 0 ecwmin 2 ecwmax 3

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams Adj prio BE 0->1, BK 1->0, BK 2->0, BE 3->1

14:04:50 h63 kernel: brcmfmac: brcmf_wifi_prioritize_acparams Adj prio VI 4->2, VI 5->2, VO 6->3, VO 7->3

14:04:50 h63 kernel: brcmfmac: brcmf_get_assoc_ies req len (76) resp len (33)

14:04:50 h63 kernel: brcmfmac: brcmf_inform_single_bss bssid: xx:xx:xx:xx:c7:a8

14:04:50 h63 kernel: brcmfmac: brcmf_inform_single_bss Channel: 3(2422)

14:04:50 h63 kernel: brcmfmac: brcmf_inform_single_bss Capability: 431

14:04:50 h63 kernel: brcmfmac: brcmf_inform_single_bss Beacon interval: 100

14:04:50 h63 kernel: brcmfmac: brcmf_inform_single_bss Signal: -5400

14:04:50 h63 kernel: brcmfmac: brcmf_bss_connect_done Report connect result - connection succeeded

14:04:50 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -54 dBm

14:04:50 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -54 dBm

14:04:50 h63 wpa_supplicant[376]: wlan0: CTRL-EVENT-SUBNET-STATUS-UPDATE status=0

14:04:50 h63 NetworkManager[362]: <info>  [1692187490.4879] device (wlan0): supplicant interface state: associating -> completed

14:04:50 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -54 dBm

14:04:50 h63 NetworkManager[362]: <info>  [1692187490.4907] device (p2p-dev-wlan0): supplicant management interface state: associating -> completed

14:04:50 h63 kernel: brcmfmac: brcmf_rx_event Enter: mmc1:0001:1: rxp=00000000f4393a20

14:04:50 h63 kernel: brcmfmac: brcmf_fweh_event_worker event RSSI (56) ifidx 0 bsscfg 0 addr 00:00:00:00:00:00

14:04:50 h63 kernel: brcmfmac: brcmf_fweh_event_worker   version 2 flags 0 status 0 reason 0

14:04:50 h63 kernel: brcmutil: event payload, len=12

14:04:50 h63 kernel: 00000000: ff ff ff ca 00 00 00 00 ff ff ff a5              ............

14:04:55 h63 kernel: brcmfmac: brcmf_cfg80211_get_station RSSI -47 dBm

14:04:59 h63 kernel: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000070

14:04:59 h63 kernel: Mem abort info:

14:04:59 h63 kernel:   ESR = 0x0000000096000005

14:04:59 h63 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits

14:04:59 h63 kernel:   SET = 0, FnV = 0

14:04:59 h63 kernel:   EA = 0, S1PTW = 0

14:04:59 h63 kernel:   FSC = 0x05: level 1 translation fault

14:04:59 h63 kernel: Data abort info:

14:04:59 h63 kernel:   ISV = 0, ISS = 0x00000005

14:04:59 h63 kernel:   CM = 0, WnR = 0

14:04:59 h63 kernel: user pgtable: 4k pages, 39-bit VAs, pgdp=00000000465b3000

14:04:59 h63 kernel: [0000000000000070] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000

14:04:59 h63 kernel: Internal error: Oops: 0000000096000005 [#1] PREEMPT SMP

14:04:59 h63 kernel: Modules linked in: ov9281 rtc_pcf85063 regmap_i2c vc4 brcmfmac snd_soc_hdmi_codec brcmutil drm_display_helper cec drm_dma_helper v3d cfg80211 gpio_keys i2c_mux_pinctrl drm_kms_helper i2c_mux gpu_sched drm_shmem_helper bcm2835_unicam bcm2835_codec(C) rpivid_hevc(C) snd_soc_core v4l2_dv_timings raspberrypi_hwmon v4l2_fwnode bcm2835_v4l2(C) v4l2_async v4l2_mem2mem bcm2835_isp(C) bcm2835_mmal_vchiq(C) videobuf2_vmalloc i2c_brcmstb videobuf2_dma_contig snd_compress videobuf2_memops snd_bcm2835(C) videobuf2_v4l2 videobuf2_common binfmt_misc snd_pcm_dmaengine videodev snd_pcm rfkill snd_timer i2c_bcm2835 snd vc_sm_cma(C) mc syscopyarea sysfillrect sysimgblt fb_sys_fops nvmem_rmem uio_pdrv_genirq uio drm dm_mod fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6

14:04:59 h63 kernel: CPU: 0 PID: 1184 Comm: kworker/0:0 Tainted: G         C         6.1.45-v8-gdc69f9d60872 #3

14:04:59 h63 kernel: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)

14:04:59 h63 kernel: pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)

14:04:59 h63 kernel: pc : wq_worker_running+0x20/0x94

14:04:59 h63 kernel: lr : wq_worker_running+0x1c/0x94

14:04:59 h63 kernel: sp : ffffffc00a8c3dd0

14:04:59 h63 kernel: x29: ffffffc00a8c3dd0 x28: 0000000000000000 x27: 0000000000000000

14:04:59 h63 kernel: x26: ffffffdf40bfe498 x25: ffffffdf40a060f0 x24: ffffffdf409e6000

14:04:59 h63 kernel: x23: ffffffdf409e69c0 x22: ffffff807fb5dc28 x21: ffffff80445c9630

14:04:59 h63 kernel: x20: ffffff8045395d00 x19: 0000000000000000 x18: 0000000000000000

14:04:59 h63 kernel: x17: 0000000000000001 x16: 0000000000000001 x15: 00cddc74a125b114

14:04:59 h63 kernel: x14: 006cbf5eacfa360a x13: ffffffdf401fbc00 x12: 00000000fa83b2da

14:04:59 h63 kernel: x11: 00000000000003f3 x10: 0000000000001a90 x9 : ffffffdf3f6ae7ac

14:04:59 h63 kernel: x8 : ffffff80453977f0 x7 : 0000000000000001 x6 : 0000000000000000

14:04:59 h63 kernel: x5 : ffffffdf409ee000 x4 : ffffffdf409ee118 x3 : 0000000000000000

14:04:59 h63 kernel: x2 : 0000000000000001 x1 : 0000000004208060 x0 : 0000000000000000

14:04:59 h63 kernel: Call trace:

14:04:59 h63 kernel:  wq_worker_running+0x20/0x94

14:04:59 h63 kernel:  schedule+0x8c/0x100

14:04:59 h63 kernel:  worker_thread+0x1d0/0x450

14:04:59 h63 kernel:  kthread+0x104/0x110

14:04:59 h63 kernel:  ret_from_fork+0x10/0x20

14:04:59 h63 kernel: Code: 910003fd f9000bf3 940018bb aa0003f3 (b9407000)

14:04:59 h63 kernel: ---[ end trace 0000000000000000 ]---

14:04:59 h63 kernel: Unable to handle kernel paging request at virtual address fffffffffffffff8

14:04:59 h63 kernel: Mem abort info:

14:04:59 h63 kernel:   ESR = 0x0000000096000005

14:04:59 h63 kernel:   EC = 0x25: DABT (current EL), IL = 32 bits

14:05:00 h63 kernel:   SET = 0, FnV = 0

14:05:00 h63 kernel:   EA = 0, S1PTW = 0

14:05:00 h63 kernel:   FSC = 0x05: level 1 translation fault

14:05:00 h63 kernel: Data abort info:

14:05:00 h63 kernel:   ISV = 0, ISS = 0x00000005

14:05:00 h63 kernel:   CM = 0, WnR = 0

14:05:00 h63 kernel: swapper pgtable: 4k pages, 39-bit VAs, pgdp=00000000011d6000

14:05:00 h63 kernel: [fffffffffffffff8] pgd=0000000000000000, p4d=0000000000000000, pud=0000000000000000

14:05:00 h63 kernel: Internal error: Oops: 0000000096000005 [#2] PREEMPT SMP

14:05:00 h63 kernel: Modules linked in: ov9281 rtc_pcf85063 regmap_i2c vc4 brcmfmac snd_soc_hdmi_codec brcmutil drm_display_helper cec drm_dma_helper v3d cfg80211 gpio_keys i2c_mux_pinctrl drm_kms_helper i2c_mux gpu_sched drm_shmem_helper bcm2835_unicam bcm2835_codec(C) rpivid_hevc(C) snd_soc_core v4l2_dv_timings raspberrypi_hwmon v4l2_fwnode bcm2835_v4l2(C) v4l2_async v4l2_mem2mem bcm2835_isp(C) bcm2835_mmal_vchiq(C) videobuf2_vmalloc i2c_brcmstb videobuf2_dma_contig snd_compress videobuf2_memops snd_bcm2835(C) videobuf2_v4l2 videobuf2_common binfmt_misc snd_pcm_dmaengine videodev snd_pcm rfkill snd_timer i2c_bcm2835 snd vc_sm_cma(C) mc syscopyarea sysfillrect sysimgblt fb_sys_fops nvmem_rmem uio_pdrv_genirq uio drm dm_mod fuse drm_panel_orientation_quirks backlight ip_tables x_tables ipv6

14:05:00 h63 kernel: CPU: 0 PID: 1184 Comm: kworker/0:0 Tainted: G      D  C         6.1.45-v8-gdc69f9d60872 #3

14:05:00 h63 kernel: Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)

14:05:00 h63 kernel: pstate: 000000c5 (nzcv daIF -PAN -UAO -TCO -DIT -SSBS BTYPE=--)

14:05:00 h63 kernel: pc : complete+0x58/0x94

14:05:00 h63 kernel: lr : complete+0x2c/0x94

14:05:00 h63 kernel: sp : ffffffc00a8c3930

14:05:00 h63 kernel: x29: ffffffc00a8c3930 x28: ffffffc00a8c3ac3 x27: ffffffdf40489dc0

14:05:00 h63 kernel: x26: ffffffdf40489db8 x25: 0000000000000001 x24: ffffffdf3f6ae7b4

14:05:00 h63 kernel: x23: 0000000000000000 x22: 000000000000000b x21: 0000000000000000

14:05:00 h63 kernel: x20: ffffff8040b55ac8 x19: 0000000000000000 x18: 0000000000000000

14:05:00 h63 kernel: x17: 3030303030303030 x16: 3030303030303020 x15: 0000000000000030

14:05:00 h63 kernel: x14: 0000000000000000 x13: 2d2d2d5d20303030 x12: 3030303030303030

14:05:00 h63 kernel: x11: fffffffffffecd58 x10: fffffffffffecd08 x9 : ffffffdf401bbb5c

14:05:00 h63 kernel: x8 : ffffffdf40a0c788 x7 : ffffffdf40a64788 x6 : 0000000000000000

14:05:00 h63 kernel: x5 : ffffffc00a8c4000 x4 : ffffffc00a8c0000 x3 : 0000000000000000

14:05:00 h63 kernel: x2 : 0000000000000001 x1 : 0000000000000000 x0 : ffffff8040b55ad0

14:05:00 h63 kernel: Call trace:

14:05:00 h63 kernel:  complete+0x58/0x94

14:05:00 h63 kernel:  mm_release+0x58/0xd0

14:05:00 h63 kernel:  exit_mm_release+0x30/0x40

14:05:00 h63 kernel:  do_exit+0x214/0x9d0

14:05:00 h63 kernel:  make_task_dead+0xa8/0x1b0

14:05:00 h63 kernel:  die+0x1dc/0x218

14:05:00 h63 kernel:  die_kernel_fault+0x280/0x338

14:05:00 h63 kernel:  __do_kernel_fault+0x14c/0x1f4

14:05:00 h63 kernel:  do_page_fault+0xd8/0x3f4

14:05:00 h63 kernel:  do_translation_fault+0xb4/0xdc

14:05:00 h63 kernel:  do_mem_abort+0x4c/0xa0

14:05:00 h63 kernel:  el1_abort+0x44/0x74

14:05:00 h63 kernel:  el1h_64_sync_handler+0xd8/0xe4

14:05:00 h63 kernel:  el1h_64_sync+0x64/0x68

14:05:00 h63 kernel:  wq_worker_running+0x20/0x94

14:05:00 h63 kernel:  schedule+0x8c/0x100

14:05:00 h63 kernel:  worker_thread+0x1d0/0x450

14:05:00 h63 kernel:  kthread+0x104/0x110

14:05:00 h63 kernel:  ret_from_fork+0x10/0x20

14:05:00 h63 kernel: Code: 91002280 eb00003f 54000120 f9400a73 (f85f8260)

14:05:00 h63 kernel: ---[ end trace 0000000000000000 ]---

14:05:00 h63 kernel: note: kworker/0:0[1184] exited with irqs disabled

14:05:00 h63 kernel: note: kworker/0:0[1184] exited with preempt_count 2

14:05:00 h63 kernel: Fixing recursive fault but reboot is needed!

14:05:04 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:04 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:04 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:04 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:10 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:10 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:10 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:10 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:16 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:16 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:16 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:16 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:22 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:22 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:22 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:22 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:28 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:28 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:28 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:28 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:34 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:34 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:34 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:34 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:40 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:40 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:40 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:40 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:46 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:46 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:46 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:46 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:52 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:52 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:52 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:52 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:05:58 h63 kernel: brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout

14:05:58 h63 kernel: brcmfmac: brcmf_sdio_readshared sdpcm_shared address 0x00206A70

14:05:58 h63 kernel: brcmfmac: brcmf_sdio_checkdied firmware not built with -assert

14:05:58 h63 kernel: ieee80211 phy0: brcmf_cfg80211_dump_station: BRCMF_C_GET_ASSOCLIST failed, err=-110

14:06:04 h63 kernel: ieee80211 phy0: brcmf_proto_bcdc_query_dcmd: brcmf_proto_bcdc_msg failed w/status -110

Re: [RFC PATCH v3 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

Oh, wait, there's another bug here ... One that makes more sense.

On Wed, 2023-08-16 at 15:32 +0200, Johannes Berg wrote:
> 
> @@ -12629,6 +12630,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
>  				u32 hysteresis)
>  {
>  	struct cfg80211_registered_device *rdev = info->user_ptr[0];
> +	struct cfg80211_cqm_config *cqm_config = NULL, *old;
>  	struct net_device *dev = info->user_ptr[1];
>  	struct wireless_dev *wdev = dev->ieee80211_ptr;
>  	int i, err;
> @@ -12646,10 +12648,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
>  	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
>  		return -EOPNOTSUPP;
>  
> -	wdev_lock(wdev);
> -	cfg80211_cqm_config_free(wdev);

This used to NULL out the value on freeing

> -	wdev_unlock(wdev);
> -
>  	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
>  		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
>  			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
> @@ -12666,9 +12664,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
>  		n_thresholds = 0;
>  
>  	wdev_lock(wdev);
> +	old = rcu_dereference_protected(wdev->cqm_config,
> +					lockdep_is_held(&wdev->mtx));
>  	if (n_thresholds) {
> -		struct cfg80211_cqm_config *cqm_config;
> -
>  		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
>  						 n_thresholds),
>  				     GFP_KERNEL);
> @@ -12683,10 +12681,16 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
>  		       flex_array_size(cqm_config, rssi_thresholds,
>  				       n_thresholds));
>  
> -		wdev->cqm_config = cqm_config;
> +		rcu_assign_pointer(wdev->cqm_config, cqm_config);
>  	}
>  
> -	err = cfg80211_cqm_rssi_update(rdev, dev);
> +	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
> +	if (err) {
> +		rcu_assign_pointer(wdev->cqm_config, old);
> +		kfree_rcu(cqm_config, rcu_head);
> +	} else {
> +		kfree_rcu(old, rcu_head);

But I didn't put that here! So you obviously have UAF when removing a
CQM config.

johannes

[RFC PATCH v4 6.1] wifi: cfg80211: fix cqm_config access race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Max Schulze reports crashes with brcmfmac. The reason seems
to be a race between userspace removing the CQM config and
the driver calling cfg80211_cqm_rssi_notify(), where if the
data is freed while cfg80211_cqm_rssi_notify() runs it will
crash since it assumes wdev->cqm_config is set. This can't
be fixed with a simple non-NULL check since there's nothing
we can do for locking easily, so use RCU instead to protect
the pointer, but that requires pulling the updates out into
an asynchronous worker so they can sleep and call back into
the driver.

Since we need to change the free anyway, also change it to
go back to the old settings if changing the settings fails.

Reported-by: Max Schulze <max.schulze@online.de>
Closes: https://lore.kernel.org/r/ac96309a-8d8d-4435-36e6-6d152eb31876@online.de
Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: change approach to use async worker
v3: fix kfree in error path
v4: NULL out value on config removal
---
 include/net/cfg80211.h |  3 +-
 net/wireless/core.c    | 14 +++----
 net/wireless/core.h    |  6 ++-
 net/wireless/nl80211.c | 86 ++++++++++++++++++++++++++++--------------
 4 files changed, 71 insertions(+), 38 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e09ff87146c1..0970eb47cb55 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5787,7 +5787,8 @@ struct wireless_dev {
 	} wext;
 #endif
 
-	struct cfg80211_cqm_config *cqm_config;
+	struct work_struct cqm_rssi_work;
+	struct cfg80211_cqm_config __rcu *cqm_config;
 
 	struct list_head pmsr_list;
 	spinlock_t pmsr_lock;
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 609b79fe4a74..a86e3819dfd6 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1114,16 +1114,11 @@ void wiphy_rfkill_set_hw_state_reason(struct wiphy *wiphy, bool blocked,
 }
 EXPORT_SYMBOL(wiphy_rfkill_set_hw_state_reason);
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev)
-{
-	kfree(wdev->cqm_config);
-	wdev->cqm_config = NULL;
-}
-
 static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 				      bool unregister_netdev)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 	unsigned int link_id;
 
 	ASSERT_RTNL();
@@ -1166,7 +1161,10 @@ static void _cfg80211_unregister_wdev(struct wireless_dev *wdev,
 	if (wdev->netdev)
 		flush_work(&wdev->disconnect_wk);
 
-	cfg80211_cqm_config_free(wdev);
+	cancel_work_sync(&wdev->cqm_rssi_work);
+	/* deleted from the list, so can't be found from nl80211 any more */
+	cqm_config = rcu_access_pointer(wdev->cqm_config);
+	kfree_rcu(cqm_config, rcu_head);
 
 	/*
 	 * Ensure that all events have been processed and
@@ -1318,6 +1316,8 @@ void cfg80211_init_wdev(struct wireless_dev *wdev)
 	wdev->wext.connect.auth_type = NL80211_AUTHTYPE_AUTOMATIC;
 #endif
 
+	INIT_WORK(&wdev->cqm_rssi_work, cfg80211_cqm_rssi_notify_work);
+
 	if (wdev->wiphy->flags & WIPHY_FLAG_PS_ON_BY_DEFAULT)
 		wdev->ps = true;
 	else
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 775e16cb99ed..bd63419fa8f8 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -287,12 +287,16 @@ struct cfg80211_beacon_registration {
 };
 
 struct cfg80211_cqm_config {
+	struct rcu_head rcu_head;
 	u32 rssi_hyst;
 	s32 last_rssi_event_value;
+	enum nl80211_cqm_rssi_threshold_event last_rssi_event_type;
 	int n_rssi_thresholds;
 	s32 rssi_thresholds[];
 };
 
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work);
+
 void cfg80211_destroy_ifaces(struct cfg80211_registered_device *rdev);
 
 /* free object */
@@ -556,8 +560,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 #define CFG80211_DEV_WARN_ON(cond)	({bool __r = (cond); __r; })
 #endif
 
-void cfg80211_cqm_config_free(struct wireless_dev *wdev);
-
 void cfg80211_release_pmsr(struct wireless_dev *wdev, u32 portid);
 void cfg80211_pmsr_wdev_down(struct wireless_dev *wdev);
 void cfg80211_pmsr_free_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 087c0c442e23..65f6c4304ad2 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -12561,7 +12561,8 @@ static int nl80211_set_cqm_txe(struct genl_info *info,
 }
 
 static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
-				    struct net_device *dev)
+				    struct net_device *dev,
+				    struct cfg80211_cqm_config *cqm_config)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	s32 last, low, high;
@@ -12570,7 +12571,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	int err;
 
 	/* RSSI reporting disabled? */
-	if (!wdev->cqm_config)
+	if (!cqm_config)
 		return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
 
 	/*
@@ -12579,7 +12580,7 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 	 * connection is established and enough beacons received to calculate
 	 * the average.
 	 */
-	if (!wdev->cqm_config->last_rssi_event_value &&
+	if (!cqm_config->last_rssi_event_value &&
 	    wdev->links[0].client.current_bss &&
 	    rdev->ops->get_station) {
 		struct station_info sinfo = {};
@@ -12593,30 +12594,30 @@ static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev,
 
 		cfg80211_sinfo_release_content(&sinfo);
 		if (sinfo.filled & BIT_ULL(NL80211_STA_INFO_BEACON_SIGNAL_AVG))
-			wdev->cqm_config->last_rssi_event_value =
+			cqm_config->last_rssi_event_value =
 				(s8) sinfo.rx_beacon_signal_avg;
 	}
 
-	last = wdev->cqm_config->last_rssi_event_value;
-	hyst = wdev->cqm_config->rssi_hyst;
-	n = wdev->cqm_config->n_rssi_thresholds;
+	last = cqm_config->last_rssi_event_value;
+	hyst = cqm_config->rssi_hyst;
+	n = cqm_config->n_rssi_thresholds;
 
 	for (i = 0; i < n; i++) {
 		i = array_index_nospec(i, n);
-		if (last < wdev->cqm_config->rssi_thresholds[i])
+		if (last < cqm_config->rssi_thresholds[i])
 			break;
 	}
 
 	low_index = i - 1;
 	if (low_index >= 0) {
 		low_index = array_index_nospec(low_index, n);
-		low = wdev->cqm_config->rssi_thresholds[low_index] - hyst;
+		low = cqm_config->rssi_thresholds[low_index] - hyst;
 	} else {
 		low = S32_MIN;
 	}
 	if (i < n) {
 		i = array_index_nospec(i, n);
-		high = wdev->cqm_config->rssi_thresholds[i] + hyst - 1;
+		high = cqm_config->rssi_thresholds[i] + hyst - 1;
 	} else {
 		high = S32_MAX;
 	}
@@ -12629,6 +12630,7 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 				u32 hysteresis)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct cfg80211_cqm_config *cqm_config = NULL, *old;
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int i, err;
@@ -12646,10 +12648,6 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 	    wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
 		return -EOPNOTSUPP;
 
-	wdev_lock(wdev);
-	cfg80211_cqm_config_free(wdev);
-	wdev_unlock(wdev);
-
 	if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
 		if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
 			return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
@@ -12666,9 +12664,9 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		n_thresholds = 0;
 
 	wdev_lock(wdev);
+	old = rcu_dereference_protected(wdev->cqm_config,
+					lockdep_is_held(&wdev->mtx));
 	if (n_thresholds) {
-		struct cfg80211_cqm_config *cqm_config;
-
 		cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
 						 n_thresholds),
 				     GFP_KERNEL);
@@ -12683,10 +12681,18 @@ static int nl80211_set_cqm_rssi(struct genl_info *info,
 		       flex_array_size(cqm_config, rssi_thresholds,
 				       n_thresholds));
 
-		wdev->cqm_config = cqm_config;
+		rcu_assign_pointer(wdev->cqm_config, cqm_config);
+	} else {
+		RCU_INIT_POINTER(wdev->cqm_config, NULL);
 	}
 
-	err = cfg80211_cqm_rssi_update(rdev, dev);
+	err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
+	if (err) {
+		rcu_assign_pointer(wdev->cqm_config, old);
+		kfree_rcu(cqm_config, rcu_head);
+	} else {
+		kfree_rcu(old, rcu_head);
+	}
 
 unlock:
 	wdev_unlock(wdev);
@@ -18715,9 +18721,8 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      enum nl80211_cqm_rssi_threshold_event rssi_event,
 			      s32 rssi_level, gfp_t gfp)
 {
-	struct sk_buff *msg;
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	struct cfg80211_cqm_config *cqm_config;
 
 	trace_cfg80211_cqm_rssi_notify(dev, rssi_event, rssi_level);
 
@@ -18725,16 +18730,42 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 		    rssi_event != NL80211_CQM_RSSI_THRESHOLD_EVENT_HIGH))
 		return;
 
-	if (wdev->cqm_config) {
-		wdev->cqm_config->last_rssi_event_value = rssi_level;
+	rcu_read_lock();
+	cqm_config = rcu_dereference(wdev->cqm_config);
+	if (cqm_config) {
+		cqm_config->last_rssi_event_value = rssi_level;
+		cqm_config->last_rssi_event_type = rssi_event;
+		schedule_work(&wdev->cqm_rssi_work);
+	}
+	rcu_read_unlock();
+}
+EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
-		cfg80211_cqm_rssi_update(rdev, dev);
+void cfg80211_cqm_rssi_notify_work(struct work_struct *work)
+{
+	struct wireless_dev *wdev = container_of(work, struct wireless_dev,
+						 cqm_rssi_work);
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
+	enum nl80211_cqm_rssi_threshold_event rssi_event;
+	struct cfg80211_cqm_config *cqm_config;
+	struct sk_buff *msg;
+	s32 rssi_level;
 
-		if (rssi_level == 0)
-			rssi_level = wdev->cqm_config->last_rssi_event_value;
+	wdev_lock(wdev);
+	cqm_config = rcu_dereference_protected(wdev->cqm_config,
+					       lockdep_is_held(&wdev->mtx));
+	if (!wdev->cqm_config) {
+		wdev_unlock(wdev);
+		return;
 	}
 
-	msg = cfg80211_prepare_cqm(dev, NULL, gfp);
+	cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
+
+	rssi_level = cqm_config->last_rssi_event_value;
+	rssi_event = cqm_config->last_rssi_event_type;
+	wdev_unlock(wdev);
+
+	msg = cfg80211_prepare_cqm(wdev->netdev, NULL, GFP_KERNEL);
 	if (!msg)
 		return;
 
@@ -18746,14 +18777,13 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 				      rssi_level))
 		goto nla_put_failure;
 
-	cfg80211_send_cqm(msg, gfp);
+	cfg80211_send_cqm(msg, GFP_KERNEL);
 
 	return;
 
  nla_put_failure:
 	nlmsg_free(msg);
 }
-EXPORT_SYMBOL(cfg80211_cqm_rssi_notify);
 
 void cfg80211_cqm_txe_notify(struct net_device *dev,
 			     const u8 *peer, u32 num_packets,
-- 
2.41.0

Re: [RFC PATCH v4 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

This has been running for two weeks on multiple systems.
It fixes the bug and does not show any other effects.

So, offering a 


Tested-by: Max Schulze <max.schulze@online.de>


Thanks!

Re: [RFC PATCH v4 6.1] wifi: cfg80211: fix cqm_config access race

[Max Schulze]

This has been running for two weeks on multiple systems.
It fixes the bug and does not show any other effects.

So, offering a

Tested-by: Max Schulze <max.schulze@online.de>


Thanks!