wifi: mac80211: lock wiphy in IP address notifier
authorJohannes Berg <johannes.berg@intel.com>
Mon, 28 Aug 2023 11:59:41 +0000 (13:59 +0200)
committerJohannes Berg <johannes.berg@intel.com>
Mon, 11 Sep 2023 09:27:20 +0000 (11:27 +0200)
Lock the wiphy in the IP address notifier as another
place that should have it locked before calling into
the driver. This needs a bit of attention since the
notifier can be called while the wiphy is already
locked, when we remove an interface. Handle this by
not running the notifier in this case, and instead
calling out to the driver directly.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
net/mac80211/iface.c
net/mac80211/main.c

index 42e130a6aee90688baa593148ce5123185e5e6a8..c0539e78a34e6ff705934c7d13277835db0b1f63 100644 (file)
@@ -2284,6 +2284,20 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
        list_for_each_entry_safe(sdata, tmp, &unreg_list, list) {
                bool netdev = sdata->dev;
 
+               /*
+                * Remove IP addresses explicitly, since the notifier will
+                * skip the callbacks if wdev->registered is false, since
+                * we can't acquire the wiphy_lock() again there if already
+                * inside this locked section.
+                */
+               sdata_lock(sdata);
+               sdata->vif.cfg.arp_addr_cnt = 0;
+               if (sdata->vif.type == NL80211_IFTYPE_STATION &&
+                   sdata->u.mgd.associated)
+                       ieee80211_vif_cfg_change_notify(sdata,
+                                                       BSS_CHANGED_ARP_FILTER);
+               sdata_unlock(sdata);
+
                list_del(&sdata->list);
                cfg80211_unregister_wdev(&sdata->wdev);
 
index b6c089648441af7344514d0d9d595042fef4d316..f6860889fbdd75af289d6eecbb625c1ed695dbe5 100644 (file)
@@ -437,7 +437,7 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
        if (!wdev)
                return NOTIFY_DONE;
 
-       if (wdev->wiphy != local->hw.wiphy)
+       if (wdev->wiphy != local->hw.wiphy || !wdev->registered)
                return NOTIFY_DONE;
 
        sdata = IEEE80211_DEV_TO_SUB_IF(ndev);
@@ -452,6 +452,25 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
                return NOTIFY_DONE;
 
        ifmgd = &sdata->u.mgd;
+
+       /*
+        * The nested here is needed to convince lockdep that this is
+        * all OK. Yes, we lock the wiphy mutex here while we already
+        * hold the notifier rwsem, that's the normal case. And yes,
+        * we also acquire the notifier rwsem again when unregistering
+        * a netdev while we already hold the wiphy mutex, so it does
+        * look like a typical ABBA deadlock.
+        *
+        * However, both of these things happen with the RTNL held
+        * already. Therefore, they can't actually happen, since the
+        * lock orders really are ABC and ACB, which is fine due to
+        * the RTNL (A).
+        *
+        * We still need to prevent recursion, which is accomplished
+        * by the !wdev->registered check above.
+        */
+       mutex_lock_nested(&local->hw.wiphy->mtx, 1);
+       __acquire(&local->hw.wiphy->mtx);
        sdata_lock(sdata);
 
        /* Copy the addresses to the vif config list */
@@ -470,6 +489,7 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
                ieee80211_vif_cfg_change_notify(sdata, BSS_CHANGED_ARP_FILTER);
 
        sdata_unlock(sdata);
+       wiphy_unlock(local->hw.wiphy);
 
        return NOTIFY_OK;
 }