Search Linux Wireless

Re: Missing wiphy lock in ieee80211_color_collision_detection_work

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/09/2024 10:15, Nicolas Escande wrote:
Hi guys,

Running a pretty hacked up kernel under lockdep, I've had a few splats like this

	[   75.453180] Call trace:
	[   75.455595]  cfg80211_bss_color_notify+0x220/0x260
	[   75.460341]  ieee80211_color_collision_detection_work+0x4c/0x5c
	[   75.466205]  process_one_work+0x434/0x6ec
	[   75.470169]  worker_thread+0x9c/0x624
	[   75.473794]  kthread+0x1a4/0x1ac
	[   75.476987]  ret_from_fork+0x10/0x20

Which shows we are calling cfg80211_obss_color_collision_notify and thus
cfg80211_bss_color_notify from ieee80211_color_collision_detection_work without
holding the wiphy's mtx.

It seems that we should either explicitely lock the phy using something like

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 847304a3a29a..481e1550cb21 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -4833,9 +4833,12 @@ void ieee80211_color_collision_detection_work(struct work_struct *work)
  		container_of(delayed_work, struct ieee80211_link_data,
  			     color_collision_detect_work);
  	struct ieee80211_sub_if_data *sdata = link->sdata;
+	struct ieee80211_local *local = sdata->local;
+ wiphy_lock(local->hw.wiphy);
  	cfg80211_obss_color_collision_notify(sdata->dev, link->color_bitmap,
  					     link->link_id);
+	wiphy_unlock(local->hw.wiphy);
  }
void ieee80211_color_change_finish(struct ieee80211_vif *vif, u8 link_id)

Or switch to using the wiphy_work_queue infrastructure.

Did I miss something ? Which one should we do ? I'm not sure of all the
implications of switching to the wiphy work queue and why it did not get
converted like the color_change_finalize_work stuff ?

ieee80211_color_collision_detection_work() used to lock the wdev mutex, now it does not hold anything since 076fc8775da("wifi: cfg80211: remove wdev mutex")

Also the rate limiting uses delayed_work_pending(), There is no wiphy work queue equivalent AFAIK, so the explicit lock is probably the way to go.




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux