mirror of
https://github.com/opnsense/src.git
synced 2026-03-11 02:31:16 -04:00
fecc36e328
As indicated in 11db70b6057e there was another panic on key removal which could no longer be reproduced. As originally assumed the problem was "hidden" by commit 9763fec11b83 as mentioned in 11db70b6057e. Said commit had logic inverted and 27bf5c405bf2 fixed that and with that the possible panic came back. The problem exists because some code paths out of net80211 are locked while others are not. This opens a possible race in net80211 which was tracked by extra logging in (*iv_key_update_begin)() (log lines shortend): key_update_begin: tid 100112 vap X nt Y unlocked key_update_begin: tid 100133 vap X nt Y locked One thread can be wpa_supplicant, the other is driven from the driver net80211 taskq. Further LinuxKPI needs to unlock (conditionally in case the lock is held) as a downcall to the driver/FW may sleep. This opens up possibilities for said race even further so that we observe it more reliably. This all leads to one thread calling down into the driver/firmware (unlocked) while the other will get to the same place (after acquiring the wiphy lock) before the nt re-lock happens and thus state checks did not catch this either. For LinuxKPI work around the problem utilizing (*iv_key_update_begin/end)() and taking the wiphy_lock() there holding it over the entire operation. Given we still have to conditionally unlock we need to keep track from _begin to _end on whether we have to re-lock. The checks for this need to be done under the wiphy_lock(). While a bool would suffice we use a refcount to make any future debugging easier. This isn't the most elegant solution but having the wiphy lock covering the full operation allows the 2nd thread to later run through the same code path and find the key gone (which we already checked). It remains questionable if (*iv_key_update_begin/end)() is the correct solution (as there are futher callers covering which would not need the unlock cycle) or if it should be done in the current code. The former will allow us to cover a full key store which we will need in case we will implement suspend/resume beyond what is done in native drivers/net80211 currently, if we will factor out the crypto locking for good, and fix the inconsistent locking of the nt (NODE) lock in net80211. Alternate solutions were discussed on freebsd-wireless today (2025-03-06, in the thread "Re: HEADS UP! Do not update on main currently (panic - on boot)"). Sponsored by: The FreeBSD Foundation X-MFC with: 27bf5c405bf2 Differential Revision: https://reviews.freebsd.org/D49256 (cherry picked from commit b8dfc3ecf7031a0a7764cdb67d85ebe0c03d5d93) |
||
|---|---|---|
| .. | ||
| linux_80211.c | ||
| linux_80211.h | ||
| linux_80211_macops.c | ||
| linux_acpi.c | ||
| linux_aperture.c | ||
| linux_cmdline.c | ||
| linux_compat.c | ||
| linux_current.c | ||
| linux_devres.c | ||
| linux_dmi.c | ||
| linux_domain.c | ||
| linux_firmware.c | ||
| linux_fpu.c | ||
| linux_hdmi.c | ||
| linux_hrtimer.c | ||
| linux_i2c.c | ||
| linux_i2cbb.c | ||
| linux_idr.c | ||
| linux_interrupt.c | ||
| linux_kmod.c | ||
| linux_kobject.c | ||
| linux_kthread.c | ||
| linux_lock.c | ||
| linux_mhi.c | ||
| linux_netdev.c | ||
| linux_page.c | ||
| linux_pci.c | ||
| linux_radix.c | ||
| linux_rcu.c | ||
| linux_schedule.c | ||
| linux_seq_file.c | ||
| linux_shmemfs.c | ||
| linux_shrinker.c | ||
| linux_simple_attr.c | ||
| linux_skbuff.c | ||
| linux_slab.c | ||
| linux_tasklet.c | ||
| linux_usb.c | ||
| linux_work.c | ||
| linux_xarray.c | ||
| linuxkpi_hdmikmod.c | ||
| linuxkpi_videokmod.c | ||
| lkpi_iic_if.m | ||