mac80211: proper IBSS locking

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@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 090e344..c691780 100644 (file)
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -43,6 +43,8 @@ static void ieee80211_rx_mgmt_auth_ibss(struct ieee80211_sub_if_data *sdata,
 {
        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(struct ieee80211_sub_if_data *sdata,
        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(struct ieee80211_sub_if_data *sdata,
        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(struct ieee80211_sub_if_data *sdata)
        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(struct ieee80211_sub_if_data *sdata)
 {
        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(struct ieee80211_sub_if_data *sdata)
        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(struct ieee80211_sub_if_data *sdata)
        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_sub_if_data *sdata,
        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(struct ieee80211_sub_if_data *sdata,
        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(struct ieee80211_sub_if_data *sdata,
                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 ieee80211_sub_if_data *sdata)
                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(unsigned long data)
                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 ieee80211_sub_if_data *sdata)
 
        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_completed(struct ieee80211_local *local)
                        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_sub_if_data *sdata,
 {
        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_sub_if_data *sdata,
                        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 ieee80211_sub_if_data *sdata)
        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 ieee80211_sub_if_data *sdata)
        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);
 

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f9251d5..c6b5c2d 100644 (file)
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -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;