wcn36xx: Introduce mutual exclusion of fw configuration

commit 39efc7cc7ccf82d1cd946580cdb70760f347305a upstream.

As the association status changes the driver needs to configure the
hardware. This is done based on information in the "sta" acquired by
ieee80211_find_sta(), which requires the caller to ensure that the "sta"
is valid while its being used; generally by entering an rcu read
section.

But the operations acting on the "sta" has to communicate with the
firmware and may therefor sleep, resulting in the following report:

[   31.418190] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:238
[   31.425919] in_atomic(): 0, irqs_disabled(): 0, pid: 34, name:
kworker/u8:1
[   31.434609] CPU: 0 PID: 34 Comm: kworker/u8:1 Tainted: G        W
4.12.0-rc4-next-20170607+ #993
[   31.441002] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC
(DT)
[   31.450380] Workqueue: phy0 ieee80211_iface_work
[   31.457226] Call trace:
[   31.461830] [<ffffff8008088c58>] dump_backtrace+0x0/0x260
[   31.464004] [<ffffff8008088f7c>] show_stack+0x14/0x20
[   31.469557] [<ffffff8008392e70>] dump_stack+0x98/0xb8
[   31.474592] [<ffffff80080e4330>] ___might_sleep+0xf0/0x118
[   31.479626] [<ffffff80080e43a8>] __might_sleep+0x50/0x88
[   31.485010] [<ffffff80088ff9a4>] mutex_lock+0x24/0x60
[   31.490479] [<ffffff8008595c38>] wcn36xx_smd_set_link_st+0x30/0x130
[   31.495428] [<ffffff8008591ed8>] wcn36xx_bss_info_changed+0x148/0x448
[   31.501504] [<ffffff80088ab3c4>]
ieee80211_bss_info_change_notify+0xbc/0x118
[   31.508102] [<ffffff80088f841c>] ieee80211_assoc_success+0x664/0x7f8
[   31.515220] [<ffffff80088e13d4>]
ieee80211_rx_mgmt_assoc_resp+0x144/0x2d8
[   31.521555] [<ffffff80088e1e20>]
ieee80211_sta_rx_queued_mgmt+0x190/0x698
[   31.528239] [<ffffff80088bc44c>] ieee80211_iface_work+0x234/0x368
[   31.535011] [<ffffff80080d81ac>] process_one_work+0x1cc/0x340
[   31.541086] [<ffffff80080d8368>] worker_thread+0x48/0x430
[   31.546814] [<ffffff80080de448>] kthread+0x108/0x138
[   31.552195] [<ffffff8008082ec0>] ret_from_fork+0x10/0x50

In order to ensure that the "sta" remains alive (and consistent) for the
duration of bss_info_changed() mutual exclusion has to be ensured with
sta_remove().

This is done by introducing a mutex to cover firmware configuration
changes, which is made to also ensure mutual exclusion between other
operations changing the state or configuration of the firmware. With
this we can drop the rcu read lock.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
[bwh: Backported to 3.16:
 - Drop changes in wcn36xx_configure_filter(), which doesn't change state
 - Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

2 files changed, 50 insertions, 2 deletions

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 4ab5370ab7a6..aa39821ada74 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -339,6 +339,8 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed)
 
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
 		int ch = WCN36XX_HW_CHANNEL(wcn);
 		wcn36xx_dbg(WCN36XX_DBG_MAC, "wcn36xx_config channel switch=%d\n",
@@ -351,6 +353,8 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed)
 		}
 	}
 
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -398,6 +402,8 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 			 key_conf->key,
 			 key_conf->keylen);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	switch (key_conf->cipher) {
 	case WLAN_CIPHER_SUITE_WEP40:
 		vif_priv->encrypt_type = WCN36XX_HAL_ED_WEP40;
@@ -492,6 +498,8 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	}
 
 out:
+	mutex_unlock(&wcn->conf_mutex);
+
 	return ret;
 }
 
@@ -591,6 +599,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac bss info changed vif %p changed 0x%08x\n",
 		    vif, changed);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	if (changed & BSS_CHANGED_BEACON_INFO) {
 		wcn36xx_dbg(WCN36XX_DBG_MAC,
 			    "mac bss changed dtim period %d\n",
@@ -651,7 +661,13 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 				     vif->addr,
 				     bss_conf->aid);
 
-			rcu_read_lock();
+
+			/*
+			 * Holding conf_mutex ensures mutal exclusion with
+			 * wcn36xx_sta_remove() and as such ensures that sta
+			 * won't be freed while we're operating on it. As such
+			 * we do not need to hold the rcu_read_lock().
+			 */
 			sta = ieee80211_find_sta(vif, bss_conf->bssid);
 			if (!sta) {
 				wcn36xx_err("sta %pM is not found\n",
@@ -675,7 +691,6 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 			 * place where AID is available.
 			 */
 			wcn36xx_smd_config_sta(wcn, vif, sta);
-			rcu_read_unlock();
 		} else {
 			wcn36xx_dbg(WCN36XX_DBG_MAC,
 				    "disassociated bss %pM vif %pM AID=%d\n",
@@ -736,6 +751,9 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 		}
 	}
 out:
+
+	mutex_unlock(&wcn->conf_mutex);
+
 	return;
 }
 
@@ -745,7 +763,10 @@ static int wcn36xx_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	struct wcn36xx *wcn = hw->priv;
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac set RTS threshold %d\n", value);
 
+	mutex_lock(&wcn->conf_mutex);
 	wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_RTS_THRESHOLD, value);
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -756,8 +777,12 @@ static void wcn36xx_remove_interface(struct ieee80211_hw *hw,
 	struct wcn36xx_vif *vif_priv = (struct wcn36xx_vif *)vif->drv_priv;
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac remove interface vif %p\n", vif);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	list_del(&vif_priv->list);
 	wcn36xx_smd_delete_sta_self(wcn, vif->addr);
+
+	mutex_unlock(&wcn->conf_mutex);
 }
 
 static int wcn36xx_add_interface(struct ieee80211_hw *hw,
@@ -778,9 +803,13 @@ static int wcn36xx_add_interface(struct ieee80211_hw *hw,
 		return -EOPNOTSUPP;
 	}
 
+	mutex_lock(&wcn->conf_mutex);
+
 	list_add(&vif_priv->list, &wcn->vif_list);
 	wcn36xx_smd_add_sta_self(wcn, vif);
 
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -793,6 +822,8 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta add vif %p sta %pM\n",
 		    vif, sta->addr);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	vif_priv->sta = sta_priv;
 	sta_priv->vif = vif_priv;
 	/*
@@ -804,6 +835,9 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		sta_priv->aid = sta->aid;
 		wcn36xx_smd_config_sta(wcn, vif, sta);
 	}
+
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -818,9 +852,14 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw,
 	wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta remove vif %p sta %pM index %d\n",
 		    vif, sta->addr, sta_priv->sta_index);
 
+	mutex_lock(&wcn->conf_mutex);
+
 	wcn36xx_smd_delete_sta(wcn, sta_priv->sta_index);
 	vif_priv->sta = NULL;
 	sta_priv->vif = NULL;
+
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -864,6 +903,8 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
 
 	sta_priv = (struct wcn36xx_sta *)sta->drv_priv;
 
+	mutex_lock(&wcn->conf_mutex);
+
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
 		sta_priv->tid = tid;
@@ -892,6 +933,8 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw,
 		wcn36xx_err("Unknown AMPDU action\n");
 	}
 
+	mutex_unlock(&wcn->conf_mutex);
+
 	return 0;
 }
 
@@ -1022,6 +1065,7 @@ static int wcn36xx_probe(struct platform_device *pdev)
 	wcn->dev = &pdev->dev;
 	wcn->ctrl_ops = pdev->dev.platform_data;
 
+	mutex_init(&wcn->conf_mutex);
 	mutex_init(&wcn->hal_mutex);
 
 	if (!wcn->ctrl_ops->get_hw_mac(addr)) {
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index f0fb81dfd17b..0578d430fa1d 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -191,6 +191,10 @@ struct wcn36xx {
 	void __iomem		*mmio;
 
 	struct wcn36xx_platform_ctrl_ops *ctrl_ops;
+
+	/* prevents concurrent FW reconfiguration */
+	struct mutex		conf_mutex;
+
 	/*
 	 * smd_buf must be protected with smd_mutex to garantee
 	 * that all messages are sent one after another