iwlwifi: fix and add missing sta_lock usage

There are a few places where sta_lock is used, but the
station information protected by it is accessed outside
of the lock. Address this in two ways, if the access
won't sleep then just move the access into the lock, if
the access can sleep then copy the needed station
information to the stack to be accessed without risk of
it changing while access in progress.

Additionally, a number of other places access station
station information without holding the sta_lock, fix
those as well.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

1 files changed, 32 insertions, 24 deletions

diff --git a/drivers/net/wireless/iwlwifi/iwl-sta.c b/drivers/net/wireless/iwlwifi/iwl-sta.c
index 2eafba6..4d878d8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-sta.c
+++ b/drivers/net/wireless/iwlwifi/iwl-sta.c
@@ -311,10 +311,10 @@ int iwl_add_station_common(struct iwl_priv *priv, const u8 *addr,
 				  struct ieee80211_sta_ht_cap *ht_info,
 				  u8 *sta_id_r)
 {
-	struct iwl_station_entry *station;
 	unsigned long flags_spin;
 	int ret = 0;
 	u8 sta_id;
+	struct iwl_addsta_cmd sta_cmd;
 
 	*sta_id_r = 0;
 	spin_lock_irqsave(&priv->sta_lock, flags_spin);
@@ -347,14 +347,15 @@ int iwl_add_station_common(struct iwl_priv *priv, const u8 *addr,
 	}
 
 	priv->stations[sta_id].used |= IWL_STA_UCODE_INPROGRESS;
-	station = &priv->stations[sta_id];
+	memcpy(&sta_cmd, &priv->stations[sta_id].sta, sizeof(struct iwl_addsta_cmd));
 	spin_unlock_irqrestore(&priv->sta_lock, flags_spin);
 
 	/* Add station to device's station table */
-	ret = iwl_send_add_sta(priv, &station->sta, CMD_SYNC);
+	ret = iwl_send_add_sta(priv, &sta_cmd, CMD_SYNC);
 	if (ret) {
-		IWL_ERR(priv, "Adding station %pM failed.\n", station->sta.sta.addr);
 		spin_lock_irqsave(&priv->sta_lock, flags_spin);
+		IWL_ERR(priv, "Adding station %pM failed.\n",
+			priv->stations[sta_id].sta.sta.addr);
 		priv->stations[sta_id].used &= ~IWL_STA_DRIVER_ACTIVE;
 		priv->stations[sta_id].used &= ~IWL_STA_UCODE_INPROGRESS;
 		spin_unlock_irqrestore(&priv->sta_lock, flags_spin);
@@ -488,7 +489,7 @@ static void iwl_sta_ucode_deactivate(struct iwl_priv *priv, u8 sta_id)
 }
 
 static int iwl_send_remove_station(struct iwl_priv *priv,
-				   struct iwl_station_entry *station)
+				   const u8 *addr, int sta_id)
 {
 	struct iwl_rx_packet *pkt;
 	int ret;
@@ -505,7 +506,7 @@ static int iwl_send_remove_station(struct iwl_priv *priv,
 
 	memset(&rm_sta_cmd, 0, sizeof(rm_sta_cmd));
 	rm_sta_cmd.num_sta = 1;
-	memcpy(&rm_sta_cmd.addr, &station->sta.sta.addr , ETH_ALEN);
+	memcpy(&rm_sta_cmd.addr, addr, ETH_ALEN);
 
 	cmd.flags |= CMD_WANT_SKB;
 
@@ -525,7 +526,7 @@ static int iwl_send_remove_station(struct iwl_priv *priv,
 		switch (pkt->u.rem_sta.status) {
 		case REM_STA_SUCCESS_MSK:
 			spin_lock_irqsave(&priv->sta_lock, flags_spin);
-			iwl_sta_ucode_deactivate(priv, station->sta.sta.sta_id);
+			iwl_sta_ucode_deactivate(priv, sta_id);
 			spin_unlock_irqrestore(&priv->sta_lock, flags_spin);
 			IWL_DEBUG_ASSOC(priv, "REPLY_REMOVE_STA PASSED\n");
 			break;
@@ -546,7 +547,6 @@ static int iwl_send_remove_station(struct iwl_priv *priv,
 int iwl_remove_station(struct iwl_priv *priv, const u8 sta_id,
 		       const u8 *addr)
 {
-	struct iwl_station_entry *station;
 	unsigned long flags;
 
 	if (!iwl_is_ready(priv)) {
@@ -592,10 +592,9 @@ int iwl_remove_station(struct iwl_priv *priv, const u8 sta_id,
 
 	BUG_ON(priv->num_stations < 0);
 
-	station = &priv->stations[sta_id];
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 
-	return iwl_send_remove_station(priv, station);
+	return iwl_send_remove_station(priv, addr, sta_id);
 out_err:
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 	return -EINVAL;
@@ -643,11 +642,13 @@ EXPORT_SYMBOL(iwl_clear_ucode_stations);
  */
 void iwl_restore_stations(struct iwl_priv *priv)
 {
-	struct iwl_station_entry *station;
+	struct iwl_addsta_cmd sta_cmd;
+	struct iwl_link_quality_cmd lq;
 	unsigned long flags_spin;
 	int i;
 	bool found = false;
 	int ret;
+	bool send_lq;
 
 	if (!iwl_is_ready(priv)) {
 		IWL_DEBUG_INFO(priv, "Not ready yet, not restoring any stations.\n");
@@ -669,13 +670,20 @@ void iwl_restore_stations(struct iwl_priv *priv)
 
 	for (i = 0; i < priv->hw_params.max_stations; i++) {
 		if ((priv->stations[i].used & IWL_STA_UCODE_INPROGRESS)) {
+			memcpy(&sta_cmd, &priv->stations[i].sta,
+			       sizeof(struct iwl_addsta_cmd));
+			send_lq = false;
+			if (priv->stations[i].lq) {
+				memcpy(&lq, priv->stations[i].lq,
+				       sizeof(struct iwl_link_quality_cmd));
+				send_lq = true;
+			}
 			spin_unlock_irqrestore(&priv->sta_lock, flags_spin);
-			station = &priv->stations[i];
-			ret = iwl_send_add_sta(priv, &priv->stations[i].sta, CMD_SYNC);
+			ret = iwl_send_add_sta(priv, &sta_cmd, CMD_SYNC);
 			if (ret) {
-				IWL_ERR(priv, "Adding station %pM failed.\n",
-					station->sta.sta.addr);
 				spin_lock_irqsave(&priv->sta_lock, flags_spin);
+				IWL_ERR(priv, "Adding station %pM failed.\n",
+					priv->stations[i].sta.sta.addr);
 				priv->stations[i].used &= ~IWL_STA_DRIVER_ACTIVE;
 				priv->stations[i].used &= ~IWL_STA_UCODE_INPROGRESS;
 				spin_unlock_irqrestore(&priv->sta_lock, flags_spin);
@@ -684,8 +692,8 @@ void iwl_restore_stations(struct iwl_priv *priv)
 			 * Rate scaling has already been initialized, send
 			 * current LQ command
 			 */
-			if (station->lq)
-				iwl_send_lq_cmd(priv, station->lq, CMD_SYNC, true);
+			if (send_lq)
+				iwl_send_lq_cmd(priv, &lq, CMD_SYNC, true);
 			spin_lock_irqsave(&priv->sta_lock, flags_spin);
 			priv->stations[i].used &= ~IWL_STA_UCODE_INPROGRESS;
 		}
@@ -1269,9 +1277,8 @@ void iwl_sta_tx_modify_enable_tid(struct iwl_priv *priv, int sta_id, int tid)
 	priv->stations[sta_id].sta.sta.modify_mask = STA_MODIFY_TID_DISABLE_TX;
 	priv->stations[sta_id].sta.tid_disable_tx &= cpu_to_le16(~(1 << tid));
 	priv->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
-	spin_unlock_irqrestore(&priv->sta_lock, flags);
-
 	iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
+	spin_unlock_irqrestore(&priv->sta_lock, flags);
 }
 EXPORT_SYMBOL(iwl_sta_tx_modify_enable_tid);
 
@@ -1302,7 +1309,7 @@ int iwl_sta_rx_agg_stop(struct iwl_priv *priv, struct ieee80211_sta *sta,
 			int tid)
 {
 	unsigned long flags;
-	int sta_id;
+	int sta_id, ret;
 
 	sta_id = iwl_sta_id(sta);
 	if (sta_id == IWL_INVALID_STATION) {
@@ -1315,10 +1322,11 @@ int iwl_sta_rx_agg_stop(struct iwl_priv *priv, struct ieee80211_sta *sta,
 	priv->stations[sta_id].sta.sta.modify_mask = STA_MODIFY_DELBA_TID_MSK;
 	priv->stations[sta_id].sta.remove_immediate_ba_tid = (u8)tid;
 	priv->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
+	ret = iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 
-	return iwl_send_add_sta(priv, &priv->stations[sta_id].sta,
-					CMD_ASYNC);
+	return ret;
+
 }
 EXPORT_SYMBOL(iwl_sta_rx_agg_stop);
 
@@ -1332,9 +1340,9 @@ void iwl_sta_modify_ps_wake(struct iwl_priv *priv, int sta_id)
 	priv->stations[sta_id].sta.sta.modify_mask = 0;
 	priv->stations[sta_id].sta.sleep_tx_count = 0;
 	priv->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
+	iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 
-	iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 }
 EXPORT_SYMBOL(iwl_sta_modify_ps_wake);
 
@@ -1349,9 +1357,9 @@ void iwl_sta_modify_sleep_tx_count(struct iwl_priv *priv, int sta_id, int cnt)
 					STA_MODIFY_SLEEP_TX_COUNT_MSK;
 	priv->stations[sta_id].sta.sleep_tx_count = cpu_to_le16(cnt);
 	priv->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
+	iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 	spin_unlock_irqrestore(&priv->sta_lock, flags);
 
-	iwl_send_add_sta(priv, &priv->stations[sta_id].sta, CMD_ASYNC);
 }
 EXPORT_SYMBOL(iwl_sta_modify_sleep_tx_count);