From 79ea1e12c0b8540100e89b32afb9f0e6503fad35 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 4 Jun 2020 12:04:20 +0200 Subject: [PATCH 1/5] cfg80211: fix management registrations deadlock Lockdep reports that we may deadlock because we take the RTNL on the work struct, but flush it under RTNL. Clearly, it's correct. In practice, this can happen when doing rfkill on an active device. Fix this by moving the work struct to the wiphy (registered dev) layer, and iterate over all the wdevs inside there. This then means we need to track which one of them has work to do, so we don't update to the driver for all wdevs all the time. Also fix a locking bug I noticed while working on this - the registrations list is iterated as if it was an RCU list, but it isn't handle that way - and we need to lock now for the update flag anyway, so remove the RCU. Fixes: 6cd536fe62ef ("cfg80211: change internal management frame registration API") Reported-by: Markus Theil Reported-and-tested-by: Kenneth R. Crudup Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20200604120420.b1dc540a7e26.I55dcca56bb5bdc5d7ad66a36a0b42afd7034d8be@changeid Signed-off-by: Johannes Berg --- include/net/cfg80211.h | 5 +++-- net/wireless/core.c | 6 +++--- net/wireless/core.h | 2 ++ net/wireless/mlme.c | 26 +++++++++++++++++++++----- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index b58ad1a3f695..fc7e8807838d 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -5075,7 +5075,8 @@ struct cfg80211_cqm_config; * by cfg80211 on change_interface * @mgmt_registrations: list of registrations for management frames * @mgmt_registrations_lock: lock for the list - * @mgmt_registrations_update_wk: update work to defer from atomic context + * @mgmt_registrations_need_update: mgmt registrations were updated, + * need to propagate the update to the driver * @mtx: mutex used to lock data in this struct, may be used by drivers * and some API functions require it held * @beacon_interval: beacon interval used on this device for transmitting @@ -5121,7 +5122,7 @@ struct wireless_dev { struct list_head mgmt_registrations; spinlock_t mgmt_registrations_lock; - struct work_struct mgmt_registrations_update_wk; + u8 mgmt_registrations_need_update:1; struct mutex mtx; diff --git a/net/wireless/core.c b/net/wireless/core.c index f0226ae9561c..c623d9bf5096 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -497,6 +497,8 @@ use_default_name: INIT_WORK(&rdev->propagate_radar_detect_wk, cfg80211_propagate_radar_detect_wk); INIT_WORK(&rdev->propagate_cac_done_wk, cfg80211_propagate_cac_done_wk); + INIT_WORK(&rdev->mgmt_registrations_update_wk, + cfg80211_mgmt_registrations_update_wk); #ifdef CONFIG_CFG80211_DEFAULT_PS rdev->wiphy.flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT; @@ -1047,6 +1049,7 @@ void wiphy_unregister(struct wiphy *wiphy) flush_work(&rdev->sched_scan_stop_wk); flush_work(&rdev->propagate_radar_detect_wk); flush_work(&rdev->propagate_cac_done_wk); + flush_work(&rdev->mgmt_registrations_update_wk); #ifdef CONFIG_PM if (rdev->wiphy.wowlan_config && rdev->ops->set_wakeup) @@ -1108,7 +1111,6 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync) rdev->devlist_generation++; cfg80211_mlme_purge_registrations(wdev); - flush_work(&wdev->mgmt_registrations_update_wk); switch (wdev->iftype) { case NL80211_IFTYPE_P2P_DEVICE: @@ -1253,8 +1255,6 @@ void cfg80211_init_wdev(struct cfg80211_registered_device *rdev, spin_lock_init(&wdev->event_lock); INIT_LIST_HEAD(&wdev->mgmt_registrations); spin_lock_init(&wdev->mgmt_registrations_lock); - INIT_WORK(&wdev->mgmt_registrations_update_wk, - cfg80211_mgmt_registrations_update_wk); INIT_LIST_HEAD(&wdev->pmsr_list); spin_lock_init(&wdev->pmsr_lock); INIT_WORK(&wdev->pmsr_free_wk, cfg80211_pmsr_free_wk); diff --git a/net/wireless/core.h b/net/wireless/core.h index e0e5b3ee9699..67b0389fca4d 100644 --- a/net/wireless/core.h +++ b/net/wireless/core.h @@ -99,6 +99,8 @@ struct cfg80211_registered_device { struct cfg80211_chan_def cac_done_chandef; struct work_struct propagate_cac_done_wk; + struct work_struct mgmt_registrations_update_wk; + /* must be last because of the way we do wiphy_priv(), * and it should at least be aligned to NETDEV_ALIGN */ struct wiphy wiphy __aligned(NETDEV_ALIGN); diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c index 189334314cba..a6c61a2e6569 100644 --- a/net/wireless/mlme.c +++ b/net/wireless/mlme.c @@ -440,9 +440,15 @@ static void cfg80211_mgmt_registrations_update(struct wireless_dev *wdev) ASSERT_RTNL(); + spin_lock_bh(&wdev->mgmt_registrations_lock); + if (!wdev->mgmt_registrations_need_update) { + spin_unlock_bh(&wdev->mgmt_registrations_lock); + return; + } + rcu_read_lock(); list_for_each_entry_rcu(tmp, &rdev->wiphy.wdev_list, list) { - list_for_each_entry_rcu(reg, &tmp->mgmt_registrations, list) { + list_for_each_entry(reg, &tmp->mgmt_registrations, list) { u32 mask = BIT(le16_to_cpu(reg->frame_type) >> 4); u32 mcast_mask = 0; @@ -460,16 +466,23 @@ static void cfg80211_mgmt_registrations_update(struct wireless_dev *wdev) } rcu_read_unlock(); + wdev->mgmt_registrations_need_update = 0; + spin_unlock_bh(&wdev->mgmt_registrations_lock); + rdev_update_mgmt_frame_registrations(rdev, wdev, &upd); } void cfg80211_mgmt_registrations_update_wk(struct work_struct *wk) { - struct wireless_dev *wdev = container_of(wk, struct wireless_dev, - mgmt_registrations_update_wk); + struct cfg80211_registered_device *rdev; + struct wireless_dev *wdev; + + rdev = container_of(wk, struct cfg80211_registered_device, + mgmt_registrations_update_wk); rtnl_lock(); - cfg80211_mgmt_registrations_update(wdev); + list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list) + cfg80211_mgmt_registrations_update(wdev); rtnl_unlock(); } @@ -557,6 +570,7 @@ int cfg80211_mlme_register_mgmt(struct wireless_dev *wdev, u32 snd_portid, nreg->multicast_rx = multicast_rx; list_add(&nreg->list, &wdev->mgmt_registrations); } + wdev->mgmt_registrations_need_update = 1; spin_unlock_bh(&wdev->mgmt_registrations_lock); cfg80211_mgmt_registrations_update(wdev); @@ -585,7 +599,8 @@ void cfg80211_mlme_unregister_socket(struct wireless_dev *wdev, u32 nlportid) list_del(®->list); kfree(reg); - schedule_work(&wdev->mgmt_registrations_update_wk); + wdev->mgmt_registrations_need_update = 1; + schedule_work(&rdev->mgmt_registrations_update_wk); } spin_unlock_bh(&wdev->mgmt_registrations_lock); @@ -608,6 +623,7 @@ void cfg80211_mlme_purge_registrations(struct wireless_dev *wdev) list_del(®->list); kfree(reg); } + wdev->mgmt_registrations_need_update = 1; spin_unlock_bh(&wdev->mgmt_registrations_lock); cfg80211_mgmt_registrations_update(wdev); From 523f3ec030aa5bf4818ec8dee35b2646abf367fa Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 3 Jun 2020 11:15:03 +0200 Subject: [PATCH 2/5] mac80211: initialize return flags in HE 6 GHz operation parsing Dan points out that if ieee80211_chandef_he_6ghz_oper() succeeds, we don't initialize 'ret'. Initialize it to 0 in this case, since everything went fine and nothing has to be disabled. Reported-by: Dan Carpenter Fixes: 57fa5e85d53c ("mac80211: determine chandef from HE 6 GHz operation") Signed-off-by: Johannes Berg Link: https://lore.kernel.org/r/20200603111500.bd2a5ff37b83.I2c3f338ce343b581db493eb9a0d988d1b626c8fb@changeid Signed-off-by: Johannes Berg --- net/mac80211/mlme.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 5820ef02a587..b2a9d47cf86d 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -167,6 +167,8 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata, ret = IEEE80211_STA_DISABLE_HT | IEEE80211_STA_DISABLE_VHT | IEEE80211_STA_DISABLE_HE; + else + ret = 0; vht_chandef = *chandef; goto out; } From 327cdb98fb3eed8373777465759c44f8d123c227 Mon Sep 17 00:00:00 2001 From: Flavio Suligoi Date: Fri, 5 Jun 2020 17:41:04 +0200 Subject: [PATCH 3/5] doc: networking: wireless: fix wiki website url In the files: - regulatory.rst - mac80211-injection.rst the wiki url is still the old "wireless.kernel.org" instead of the new "wireless.wiki.kernel.org" Signed-off-by: Flavio Suligoi Link: https://lore.kernel.org/r/20200605154112.16277-2-f.suligoi@asem.it Signed-off-by: Johannes Berg --- Documentation/networking/mac80211-injection.rst | 2 +- Documentation/networking/regulatory.rst | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/networking/mac80211-injection.rst b/Documentation/networking/mac80211-injection.rst index be65f886ff1f..63ba6611fdff 100644 --- a/Documentation/networking/mac80211-injection.rst +++ b/Documentation/networking/mac80211-injection.rst @@ -101,6 +101,6 @@ interface), along the following lines::: You can also find a link to a complete inject application here: -http://wireless.kernel.org/en/users/Documentation/packetspammer +https://wireless.wiki.kernel.org/en/users/Documentation/packetspammer Andy Green diff --git a/Documentation/networking/regulatory.rst b/Documentation/networking/regulatory.rst index 8701b91e81ee..16782a95b74a 100644 --- a/Documentation/networking/regulatory.rst +++ b/Documentation/networking/regulatory.rst @@ -9,7 +9,7 @@ regulatory infrastructure works. More up to date information can be obtained at the project's web page: -http://wireless.kernel.org/en/developers/Regulatory +https://wireless.wiki.kernel.org/en/developers/Regulatory Keeping regulatory domains in userspace --------------------------------------- @@ -37,7 +37,7 @@ expected regulatory domains will be respected by the kernel. A currently available userspace agent which can accomplish this is CRDA - central regulatory domain agent. Its documented here: -http://wireless.kernel.org/en/developers/Regulatory/CRDA +https://wireless.wiki.kernel.org/en/developers/Regulatory/CRDA Essentially the kernel will send a udev event when it knows it needs a new regulatory domain. A udev rule can be put in place @@ -58,7 +58,7 @@ Who asks for regulatory domains? Users can use iw: -http://wireless.kernel.org/en/users/Documentation/iw +https://wireless.wiki.kernel.org/en/users/Documentation/iw An example:: From 97eda66421c44f1449e8d087fd05eab5d466afb7 Mon Sep 17 00:00:00 2001 From: Flavio Suligoi Date: Fri, 5 Jun 2020 17:41:11 +0200 Subject: [PATCH 4/5] include: fix wiki website url in netlink interface header The wiki url is still the old "wireless.kernel.org" instead of the new "wireless.wiki.kernel.org" Signed-off-by: Flavio Suligoi Link: https://lore.kernel.org/r/20200605154112.16277-9-f.suligoi@asem.it Signed-off-by: Johannes Berg --- include/uapi/linux/nl80211.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index dad8c8f8581f..4e6339ab1fce 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -794,7 +794,7 @@ * various triggers. These triggers can be configured through this * command with the %NL80211_ATTR_WOWLAN_TRIGGERS attribute. For * more background information, see - * http://wireless.kernel.org/en/users/Documentation/WoWLAN. + * https://wireless.wiki.kernel.org/en/users/Documentation/WoWLAN. * The @NL80211_CMD_SET_WOWLAN command can also be used as a notification * from the driver reporting the wakeup reason. In this case, the * @NL80211_ATTR_WOWLAN_TRIGGERS attribute will contain the reason From 59d4bfc1e2c09435d91c980b03f7b72ce6e9f24e Mon Sep 17 00:00:00 2001 From: Flavio Suligoi Date: Fri, 5 Jun 2020 17:41:12 +0200 Subject: [PATCH 5/5] net: fix wiki website url mac80211 and wireless files In the files: - net/mac80211/rx.c - net/wireless/Kconfig the wiki url is still the old "wireless.kernel.org" instead of the new "wireless.wiki.kernel.org" Signed-off-by: Flavio Suligoi Link: https://lore.kernel.org/r/20200605154112.16277-10-f.suligoi@asem.it Signed-off-by: Johannes Berg --- net/mac80211/rx.c | 2 +- net/wireless/Kconfig | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 21854a61a2b7..a88ab6fb16f2 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -4694,7 +4694,7 @@ void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta, * rate_idx is MCS index, which can be [0-76] * as documented on: * - * http://wireless.kernel.org/en/developers/Documentation/ieee80211/802.11n + * https://wireless.wiki.kernel.org/en/developers/Documentation/ieee80211/802.11n * * Anything else would be some sort of driver or * hardware error. The driver should catch hardware diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig index 813e93644ae7..d69558487041 100644 --- a/net/wireless/Kconfig +++ b/net/wireless/Kconfig @@ -31,7 +31,7 @@ config CFG80211 For more information refer to documentation on the wireless wiki: - http://wireless.kernel.org/en/developers/Documentation/cfg80211 + https://wireless.wiki.kernel.org/en/developers/Documentation/cfg80211 When built as a module it will be called cfg80211.