From: Tomas Winkler <tomas.winkler@intel.com>
Date: Tue, 27 May 2008 14:50:52 +0000 (+0300)
Subject: mac80211: fix deadlock in sta->lock
X-Git-Tag: v2.6.27-rc1~969^2~337^2~56
X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b83f4e15e65d94f6f56924b0b06a77a7ca2b4d8a;p=linux-2.6

mac80211: fix deadlock in sta->lock

This patch fixes a deadlock of sta->lock use, occurring while changing
tx aggregation states, as dev_queue_xmit end up in new function
test_and_clear_sta_flags that uses that lock thus leading to deadlock

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
Signed-off-by: Ron Rindjunsky <ron.rindjunsky@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 97d4a537ca..f79f6b9938 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -589,8 +589,8 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 	sta = sta_info_get(local, ra);
 	if (!sta) {
 		printk(KERN_DEBUG "Could not find the station\n");
-		rcu_read_unlock();
-		return -ENOENT;
+		ret = -ENOENT;
+		goto exit;
 	}
 
 	spin_lock_bh(&sta->lock);
@@ -598,7 +598,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 	/* we have tried too many times, receiver does not want A-MPDU */
 	if (sta->ampdu_mlme.addba_req_num[tid] > HT_AGG_MAX_RETRIES) {
 		ret = -EBUSY;
-		goto start_ba_exit;
+		goto err_unlock_sta;
 	}
 
 	state = &sta->ampdu_mlme.tid_state_tx[tid];
@@ -609,7 +609,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 				 "idle on tid %u\n", tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
 		ret = -EAGAIN;
-		goto start_ba_exit;
+		goto err_unlock_sta;
 	}
 
 	/* prepare A-MPDU MLME for Tx aggregation */
@@ -620,7 +620,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 			printk(KERN_ERR "allocate tx mlme to tid %d failed\n",
 					tid);
 		ret = -ENOMEM;
-		goto start_ba_exit;
+		goto err_unlock_sta;
 	}
 	/* Tx timer */
 	sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.function =
@@ -643,7 +643,7 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 		printk(KERN_DEBUG "BA request denied - queue unavailable for"
 					" tid %d\n", tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
-		goto start_ba_err;
+		goto err_unlock_queue;
 	}
 	sdata = sta->sdata;
 
@@ -665,12 +665,13 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 					" tid %d\n", tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
 		*state = HT_AGG_STATE_IDLE;
-		goto start_ba_err;
+		goto err_unlock_queue;
 	}
 
 	/* Will put all the packets in the new SW queue */
 	ieee80211_requeue(local, ieee802_1d_to_ac[tid]);
 	spin_unlock_bh(&local->mdev->queue_lock);
+	spin_unlock_bh(&sta->lock);
 
 	/* send an addBA request */
 	sta->ampdu_mlme.dialog_token_allocator++;
@@ -678,25 +679,26 @@ int ieee80211_start_tx_ba_session(struct ieee80211_hw *hw, u8 *ra, u16 tid)
 			sta->ampdu_mlme.dialog_token_allocator;
 	sta->ampdu_mlme.tid_tx[tid]->ssn = start_seq_num;
 
+
 	ieee80211_send_addba_request(sta->sdata->dev, ra, tid,
 			 sta->ampdu_mlme.tid_tx[tid]->dialog_token,
 			 sta->ampdu_mlme.tid_tx[tid]->ssn,
 			 0x40, 5000);
-
 	/* activate the timer for the recipient's addBA response */
 	sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer.expires =
 				jiffies + ADDBA_RESP_INTERVAL;
 	add_timer(&sta->ampdu_mlme.tid_tx[tid]->addba_resp_timer);
 	printk(KERN_DEBUG "activated addBA response timer on tid %d\n", tid);
-	goto start_ba_exit;
+	goto exit;
 
-start_ba_err:
+err_unlock_queue:
 	kfree(sta->ampdu_mlme.tid_tx[tid]);
 	sta->ampdu_mlme.tid_tx[tid] = NULL;
 	spin_unlock_bh(&local->mdev->queue_lock);
 	ret = -EBUSY;
-start_ba_exit:
+err_unlock_sta:
 	spin_unlock_bh(&sta->lock);
+exit:
 	rcu_read_unlock();
 	return ret;
 }
@@ -835,10 +837,11 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u8 tid)
 	}
 	state = &sta->ampdu_mlme.tid_state_tx[tid];
 
-	spin_lock_bh(&sta->lock);
+	/* NOTE: no need to use sta->lock in this state check, as
+	 * ieee80211_stop_tx_ba_session will let only
+	 * one stop call to pass through per sta/tid */
 	if ((*state & HT_AGG_STATE_REQ_STOP_BA_MSK) == 0) {
 		printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n");
-		spin_unlock_bh(&sta->lock);
 		rcu_read_unlock();
 		return;
 	}
@@ -861,6 +864,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee80211_hw *hw, u8 *ra, u8 tid)
 	 * ieee80211_wake_queue is not used here as this queue is not
 	 * necessarily stopped */
 	netif_schedule(local->mdev);
+	spin_lock_bh(&sta->lock);
 	*state = HT_AGG_STATE_IDLE;
 	sta->ampdu_mlme.addba_req_num[tid] = 0;
 	kfree(sta->ampdu_mlme.tid_tx[tid]);