| From 52e04b4ce5d03775b6a78f3ed1097480faacc9fd Mon Sep 17 00:00:00 2001 |
| From: Sumit Garg <sumit.garg@linaro.org> |
| Date: Tue, 7 Apr 2020 15:40:55 +0530 |
| Subject: mac80211: fix race in ieee80211_register_hw() |
| |
| From: Sumit Garg <sumit.garg@linaro.org> |
| |
| commit 52e04b4ce5d03775b6a78f3ed1097480faacc9fd upstream. |
| |
| A race condition leading to a kernel crash is observed during invocation |
| of ieee80211_register_hw() on a dragonboard410c device having wcn36xx |
| driver built as a loadable module along with a wifi manager in user-space |
| waiting for a wifi device (wlanX) to be active. |
| |
| Sequence diagram for a particular kernel crash scenario: |
| |
| user-space ieee80211_register_hw() ieee80211_tasklet_handler() |
| ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
| | | | |
| |<---phy0----wiphy_register() | |
| |-----iwd if_add---->| | |
| | |<---IRQ----(RX packet) |
| | Kernel crash | |
| | due to unallocated | |
| | workqueue. | |
| | | | |
| | alloc_ordered_workqueue() | |
| | | | |
| | Misc wiphy init. | |
| | | | |
| | ieee80211_if_add() | |
| | | | |
| |
| As evident from above sequence diagram, this race condition isn't specific |
| to a particular wifi driver but rather the initialization sequence in |
| ieee80211_register_hw() needs to be fixed. So re-order the initialization |
| sequence and the updated sequence diagram would look like: |
| |
| user-space ieee80211_register_hw() ieee80211_tasklet_handler() |
| ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
| | | | |
| | alloc_ordered_workqueue() | |
| | | | |
| | Misc wiphy init. | |
| | | | |
| |<---phy0----wiphy_register() | |
| |-----iwd if_add---->| | |
| | |<---IRQ----(RX packet) |
| | | | |
| | ieee80211_if_add() | |
| | | | |
| |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Sumit Garg <sumit.garg@linaro.org> |
| Link: https://lore.kernel.org/r/1586254255-28713-1-git-send-email-sumit.garg@linaro.org |
| [Johannes: fix rtnl imbalances] |
| Signed-off-by: Johannes Berg <johannes.berg@intel.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| net/mac80211/main.c | 24 +++++++++++++----------- |
| 1 file changed, 13 insertions(+), 11 deletions(-) |
| |
| --- a/net/mac80211/main.c |
| +++ b/net/mac80211/main.c |
| @@ -1045,7 +1045,7 @@ int ieee80211_register_hw(struct ieee802 |
| local->hw.wiphy->signal_type = CFG80211_SIGNAL_TYPE_UNSPEC; |
| if (hw->max_signal <= 0) { |
| result = -EINVAL; |
| - goto fail_wiphy_register; |
| + goto fail_workqueue; |
| } |
| } |
| |
| @@ -1107,7 +1107,7 @@ int ieee80211_register_hw(struct ieee802 |
| |
| result = ieee80211_init_cipher_suites(local); |
| if (result < 0) |
| - goto fail_wiphy_register; |
| + goto fail_workqueue; |
| |
| if (!local->ops->remain_on_channel) |
| local->hw.wiphy->max_remain_on_channel_duration = 5000; |
| @@ -1133,10 +1133,6 @@ int ieee80211_register_hw(struct ieee802 |
| |
| local->hw.wiphy->max_num_csa_counters = IEEE80211_MAX_CSA_COUNTERS_NUM; |
| |
| - result = wiphy_register(local->hw.wiphy); |
| - if (result < 0) |
| - goto fail_wiphy_register; |
| - |
| /* |
| * We use the number of queues for feature tests (QoS, HT) internally |
| * so restrict them appropriately. |
| @@ -1192,9 +1188,9 @@ int ieee80211_register_hw(struct ieee802 |
| goto fail_flows; |
| |
| rtnl_lock(); |
| - |
| result = ieee80211_init_rate_ctrl_alg(local, |
| hw->rate_control_algorithm); |
| + rtnl_unlock(); |
| if (result < 0) { |
| wiphy_debug(local->hw.wiphy, |
| "Failed to initialize rate control algorithm\n"); |
| @@ -1248,6 +1244,12 @@ int ieee80211_register_hw(struct ieee802 |
| local->sband_allocated |= BIT(band); |
| } |
| |
| + result = wiphy_register(local->hw.wiphy); |
| + if (result < 0) |
| + goto fail_wiphy_register; |
| + |
| + rtnl_lock(); |
| + |
| /* add one default STA interface if supported */ |
| if (local->hw.wiphy->interface_modes & BIT(NL80211_IFTYPE_STATION) && |
| !ieee80211_hw_check(hw, NO_AUTO_VIF)) { |
| @@ -1287,17 +1289,17 @@ int ieee80211_register_hw(struct ieee802 |
| #if defined(CONFIG_INET) || defined(CONFIG_IPV6) |
| fail_ifa: |
| #endif |
| + wiphy_unregister(local->hw.wiphy); |
| + fail_wiphy_register: |
| rtnl_lock(); |
| rate_control_deinitialize(local); |
| ieee80211_remove_interfaces(local); |
| - fail_rate: |
| rtnl_unlock(); |
| + fail_rate: |
| fail_flows: |
| ieee80211_led_exit(local); |
| destroy_workqueue(local->workqueue); |
| fail_workqueue: |
| - wiphy_unregister(local->hw.wiphy); |
| - fail_wiphy_register: |
| if (local->wiphy_ciphers_allocated) |
| kfree(local->hw.wiphy->cipher_suites); |
| kfree(local->int_scan_req); |
| @@ -1347,8 +1349,8 @@ void ieee80211_unregister_hw(struct ieee |
| skb_queue_purge(&local->skb_queue_unreliable); |
| skb_queue_purge(&local->skb_queue_tdls_chsw); |
| |
| - destroy_workqueue(local->workqueue); |
| wiphy_unregister(local->hw.wiphy); |
| + destroy_workqueue(local->workqueue); |
| ieee80211_led_exit(local); |
| kfree(local->int_scan_req); |
| } |