Search Linux Wireless

[PATCH] mac80211: proper IBSS locking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Johannes Berg <johannes.berg@xxxxxxxxx>

IBSS has never had locking, instead relying on some
memory barriers etc. That's hard to get right, and
I think we had it wrong too until the previous patch.
Since this is not performance sensitive, it doesn't
make sense to have the maintenance overhead of that,
so add proper locking.

Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
---
 net/mac80211/ibss.c        |   97 ++++++++++++++++++++++++---------------------
 net/mac80211/ieee80211_i.h |    7 ---
 2 files changed, 54 insertions(+), 50 deletions(-)

--- wireless-testing.orig/net/mac80211/ibss.c	2010-07-21 11:23:19.000000000 +0200
+++ wireless-testing/net/mac80211/ibss.c	2010-07-21 11:23:21.000000000 +0200
@@ -43,6 +43,8 @@ static void ieee80211_rx_mgmt_auth_ibss(
 {
 	u16 auth_alg, auth_transaction, status_code;
 
+	lockdep_assert_held(&sdata->u.ibss.mtx);
+
 	if (len < 24 + 6)
 		return;
 
@@ -78,6 +80,8 @@ static void __ieee80211_sta_join_ibss(st
 	u32 bss_change;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
 
+	lockdep_assert_held(&ifibss->mtx);
+
 	/* Reset own TSF to allow time synchronization work. */
 	drv_reset_tsf(local);
 
@@ -205,6 +209,8 @@ static void ieee80211_sta_join_ibss(stru
 	int i, j;
 	u16 beacon_int = cbss->beacon_interval;
 
+	lockdep_assert_held(&sdata->u.ibss.mtx);
+
 	if (beacon_int < 10)
 		beacon_int = 10;
 
@@ -449,6 +455,8 @@ static int ieee80211_sta_active_ibss(str
 	int active = 0;
 	struct sta_info *sta;
 
+	lockdep_assert_held(&sdata->u.ibss.mtx);
+
 	rcu_read_lock();
 
 	list_for_each_entry_rcu(sta, &local->sta_list, list) {
@@ -473,6 +481,8 @@ static void ieee80211_sta_merge_ibss(str
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
+	lockdep_assert_held(&ifibss->mtx);
+
 	mod_timer(&ifibss->timer,
 		  round_jiffies(jiffies + IEEE80211_IBSS_MERGE_INTERVAL));
 
@@ -505,6 +515,8 @@ static void ieee80211_sta_create_ibss(st
 	u16 capability;
 	int i;
 
+	lockdep_assert_held(&ifibss->mtx);
+
 	if (ifibss->fixed_bssid) {
 		memcpy(bssid, ifibss->bssid, ETH_ALEN);
 	} else {
@@ -549,6 +561,8 @@ static void ieee80211_sta_find_ibss(stru
 	int active_ibss;
 	u16 capability;
 
+	lockdep_assert_held(&ifibss->mtx);
+
 	active_ibss = ieee80211_sta_active_ibss(sdata);
 #ifdef CONFIG_MAC80211_IBSS_DEBUG
 	printk(KERN_DEBUG "%s: sta_find_ibss (active_ibss=%d)\n",
@@ -637,6 +651,8 @@ static void ieee80211_rx_mgmt_probe_req(
 	struct ieee80211_mgmt *resp;
 	u8 *pos, *end;
 
+	lockdep_assert_held(&ifibss->mtx);
+
 	if (ifibss->state != IEEE80211_IBSS_MLME_JOINED ||
 	    len < 24 + 2 || !ifibss->presp)
 		return;
@@ -740,6 +756,8 @@ void ieee80211_ibss_rx_queued_mgmt(struc
 	mgmt = (struct ieee80211_mgmt *) skb->data;
 	fc = le16_to_cpu(mgmt->frame_control);
 
+	mutex_lock(&sdata->u.ibss.mtx);
+
 	switch (fc & IEEE80211_FCTL_STYPE) {
 	case IEEE80211_STYPE_PROBE_REQ:
 		ieee80211_rx_mgmt_probe_req(sdata, mgmt, skb->len);
@@ -756,14 +774,23 @@ void ieee80211_ibss_rx_queued_mgmt(struc
 		ieee80211_rx_mgmt_auth_ibss(sdata, mgmt, skb->len);
 		break;
 	}
+
+	mutex_unlock(&sdata->u.ibss.mtx);
 }
 
 void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
 
-	if (!test_and_clear_bit(IEEE80211_IBSS_REQ_RUN, &ifibss->request))
-		return;
+	mutex_lock(&ifibss->mtx);
+
+	/*
+	 * Work could be scheduled after scan or similar
+	 * when we aren't even joined (or trying) with a
+	 * network.
+	 */
+	if (!ifibss->ssid_len)
+		goto out;
 
 	switch (ifibss->state) {
 	case IEEE80211_IBSS_MLME_SEARCH:
@@ -776,15 +803,9 @@ void ieee80211_ibss_work(struct ieee8021
 		WARN_ON(1);
 		break;
 	}
-}
 
-static void ieee80211_queue_ibss_work(struct ieee80211_sub_if_data *sdata)
-{
-	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
-	struct ieee80211_local *local = sdata->local;
-
-	set_bit(IEEE80211_IBSS_REQ_RUN, &ifibss->request);
-	ieee80211_queue_work(&local->hw, &sdata->work);
+ out:
+	mutex_unlock(&ifibss->mtx);
 }
 
 static void ieee80211_ibss_timer(unsigned long data)
@@ -799,7 +820,7 @@ static void ieee80211_ibss_timer(unsigne
 		return;
 	}
 
-	ieee80211_queue_ibss_work(sdata);
+	ieee80211_queue_work(&local->hw, &sdata->work);
 }
 
 #ifdef CONFIG_PM
@@ -828,6 +849,7 @@ void ieee80211_ibss_setup_sdata(struct i
 
 	setup_timer(&ifibss->timer, ieee80211_ibss_timer,
 		    (unsigned long) sdata);
+	mutex_init(&ifibss->mtx);
 }
 
 /* scan finished notification */
@@ -841,10 +863,8 @@ void ieee80211_ibss_notify_scan_complete
 			continue;
 		if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
 			continue;
-		if (!sdata->u.ibss.ssid_len)
-			continue;
 		sdata->u.ibss.last_scan_completed = jiffies;
-		ieee80211_queue_ibss_work(sdata);
+		ieee80211_queue_work(&local->hw, &sdata->work);
 	}
 	mutex_unlock(&local->iflist_mtx);
 }
@@ -854,6 +874,17 @@ int ieee80211_ibss_join(struct ieee80211
 {
 	struct sk_buff *skb;
 
+	skb = dev_alloc_skb(sdata->local->hw.extra_tx_headroom +
+			    36 /* bitrates */ +
+			    34 /* SSID */ +
+			    3  /* DS params */ +
+			    4  /* IBSS params */ +
+			    params->ie_len);
+	if (!skb)
+		return -ENOMEM;
+
+	mutex_lock(&sdata->u.ibss.mtx);
+
 	if (params->bssid) {
 		memcpy(sdata->u.ibss.bssid, params->bssid, ETH_ALEN);
 		sdata->u.ibss.fixed_bssid = true;
@@ -882,35 +913,19 @@ int ieee80211_ibss_join(struct ieee80211
 			sdata->u.ibss.ie_len = params->ie_len;
 	}
 
-	skb = dev_alloc_skb(sdata->local->hw.extra_tx_headroom +
-			    36 /* bitrates */ +
-			    34 /* SSID */ +
-			    3  /* DS params */ +
-			    4  /* IBSS params */ +
-			    params->ie_len);
-	if (!skb)
-		return -ENOMEM;
-
 	sdata->u.ibss.skb = skb;
 	sdata->u.ibss.state = IEEE80211_IBSS_MLME_SEARCH;
 	sdata->u.ibss.ibss_join_req = jiffies;
 
 	memcpy(sdata->u.ibss.ssid, params->ssid, IEEE80211_MAX_SSID_LEN);
-
-	/*
-	 * The ssid_len setting below is used to see whether
-	 * we are active, and we need all other settings
-	 * before that may get visible.
-	 */
-	mb();
-
 	sdata->u.ibss.ssid_len = params->ssid_len;
 
 	ieee80211_recalc_idle(sdata->local);
 
-	set_bit(IEEE80211_IBSS_REQ_RUN, &sdata->u.ibss.request);
 	ieee80211_queue_work(&sdata->local->hw, &sdata->work);
 
+	mutex_unlock(&sdata->u.ibss.mtx);
+
 	return 0;
 }
 
@@ -921,7 +936,9 @@ int ieee80211_ibss_leave(struct ieee8021
 	struct ieee80211_local *local = sdata->local;
 	struct cfg80211_bss *cbss;
 	u16 capability;
-	int active_ibss = 0;
+	int active_ibss;
+
+	mutex_lock(&sdata->u.ibss.mtx);
 
 	active_ibss = ieee80211_sta_active_ibss(sdata);
 
@@ -959,19 +976,9 @@ int ieee80211_ibss_leave(struct ieee8021
 	memset(sdata->u.ibss.bssid, 0, ETH_ALEN);
 	sdata->u.ibss.ssid_len = 0;
 
-	/*
-	 * ssid_len indicates active or not, so needs to be visible to
-	 * everybody, especially ieee80211_ibss_notify_scan_completed,
-	 * so it won't restart the timer after we remove it here.
-	 */
-	mb();
-
 	del_timer_sync(&sdata->u.ibss.timer);
-	clear_bit(IEEE80211_IBSS_REQ_RUN, &sdata->u.ibss.request);
-	/*
-	 * Since the REQ_RUN bit is clear, the work won't do
-	 * anything if it runs after this.
-	 */
+
+	mutex_unlock(&sdata->u.ibss.mtx);
 
 	ieee80211_recalc_idle(sdata->local);
 
--- wireless-testing.orig/net/mac80211/ieee80211_i.h	2010-07-21 11:22:31.000000000 +0200
+++ wireless-testing/net/mac80211/ieee80211_i.h	2010-07-21 11:23:21.000000000 +0200
@@ -377,14 +377,11 @@ struct ieee80211_if_managed {
 	int last_cqm_event_signal;
 };
 
-enum ieee80211_ibss_request {
-	IEEE80211_IBSS_REQ_RUN	= 0,
-};
-
 struct ieee80211_if_ibss {
 	struct timer_list timer;
 
-	unsigned long request;
+	struct mutex mtx;
+
 	unsigned long last_scan_completed;
 
 	u32 basic_rates;


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux