mac80211: fix deadlock with multiple interfaces

The locking around ieee80211_recalc_smps is
buggy -- it cannot acquire another interface's
mutex while the iflist mutex is held because
another code path could be holding the iface
mutex and trying to acquire the iflist mutex.

But the locking is also unnecessary, we only
check "ifmgd->associated" as a bool, and don't
use the pointer (in check_mgd_smps).

Reported-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a7a78f28ff6f4819c84fc1e85fc56fe1e51ae706..94bf550bd4c9ebec756bff41e42c926792e73af5 100644 (file)
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1394,7 +1394,7 @@ int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata,
        if (!sdata->u.mgd.associated ||
            sdata->vif.bss_conf.channel_type == NL80211_CHAN_NO_HT) {
                mutex_lock(&sdata->local->iflist_mtx);
-               ieee80211_recalc_smps(sdata->local, sdata);
+               ieee80211_recalc_smps(sdata->local);
                mutex_unlock(&sdata->local->iflist_mtx);
                return 0;
        }

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 55d79db985fc4f24e3ed56345b65e9a92f1ad24e..08509e21284159ad48bfed54f58246149db6308c 100644 (file)
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1297,8 +1297,7 @@ u32 ieee80211_sta_get_rates(struct ieee80211_local *local,
                            enum ieee80211_band band);
 int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata,
                             enum ieee80211_smps_mode smps_mode);
-void ieee80211_recalc_smps(struct ieee80211_local *local,
-                          struct ieee80211_sub_if_data *forsdata);
+void ieee80211_recalc_smps(struct ieee80211_local *local);
 
 size_t ieee80211_ie_split(const u8 *ies, size_t ielen,
                          const u8 *ids, int n_ids, size_t offset);

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 9c2f3f934c74e6d0b7864720a9efcb31a1272e3e..e3717092115f1720e133ef656c103a45e50c679d 100644 (file)
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -333,7 +333,7 @@ static void ieee80211_recalc_smps_work(struct work_struct *work)
                container_of(work, struct ieee80211_local, recalc_smps);
 
        mutex_lock(&local->iflist_mtx);
-       ieee80211_recalc_smps(local, NULL);
+       ieee80211_recalc_smps(local);
        mutex_unlock(&local->iflist_mtx);
 }
 

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index c37086a12f5195ec7f37a87d19d9a95a2cec315f..2b2982782bcd8057d8d2504b352981ac04fc2d02 100644 (file)
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -913,7 +913,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 
        mutex_lock(&local->iflist_mtx);
        ieee80211_recalc_ps(local, -1);
-       ieee80211_recalc_smps(local, sdata);
+       ieee80211_recalc_smps(local);
        mutex_unlock(&local->iflist_mtx);
 
        netif_tx_start_all_queues(sdata->dev);

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index aba025d748e92cd8bdfa7e4c7347a4ea7e8ea18e..4ee8f2b53cb7c2980cb5ba1c54b70314c0d5b4cd 100644 (file)
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1297,16 +1297,12 @@ static int check_mgd_smps(struct ieee80211_if_managed *ifmgd,
 }
 
 /* must hold iflist_mtx */
-void ieee80211_recalc_smps(struct ieee80211_local *local,
-                          struct ieee80211_sub_if_data *forsdata)
+void ieee80211_recalc_smps(struct ieee80211_local *local)
 {
        struct ieee80211_sub_if_data *sdata;
        enum ieee80211_smps_mode smps_mode = IEEE80211_SMPS_OFF;
        int count = 0;
 
-       if (forsdata)
-               lockdep_assert_held(&forsdata->u.mgd.mtx);
-
        lockdep_assert_held(&local->iflist_mtx);
 
        /*
@@ -1324,18 +1320,8 @@ void ieee80211_recalc_smps(struct ieee80211_local *local,
                        continue;
                if (sdata->vif.type != NL80211_IFTYPE_STATION)
                        goto set;
-               if (sdata != forsdata) {
-                       /*
-                        * This nested is ok -- we are holding the iflist_mtx
-                        * so can't get here twice or so. But it's required
-                        * since normally we acquire it first and then the
-                        * iflist_mtx.
-                        */
-                       mutex_lock_nested(&sdata->u.mgd.mtx, SINGLE_DEPTH_NESTING);
-                       count += check_mgd_smps(&sdata->u.mgd, &smps_mode);
-                       mutex_unlock(&sdata->u.mgd.mtx);
-               } else
-                       count += check_mgd_smps(&sdata->u.mgd, &smps_mode);
+
+               count += check_mgd_smps(&sdata->u.mgd, &smps_mode);
 
                if (count > 1) {
                        smps_mode = IEEE80211_SMPS_OFF;