Commit 025e6be2 authored by Johannes Berg's avatar Johannes Berg Committed by John W. Linville

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: default avatarBen Greear <greearb@candelatech.com>
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent 67748893
...@@ -1394,7 +1394,7 @@ int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata, ...@@ -1394,7 +1394,7 @@ int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata,
if (!sdata->u.mgd.associated || if (!sdata->u.mgd.associated ||
sdata->vif.bss_conf.channel_type == NL80211_CHAN_NO_HT) { sdata->vif.bss_conf.channel_type == NL80211_CHAN_NO_HT) {
mutex_lock(&sdata->local->iflist_mtx); mutex_lock(&sdata->local->iflist_mtx);
ieee80211_recalc_smps(sdata->local, sdata); ieee80211_recalc_smps(sdata->local);
mutex_unlock(&sdata->local->iflist_mtx); mutex_unlock(&sdata->local->iflist_mtx);
return 0; return 0;
} }
......
...@@ -1297,8 +1297,7 @@ u32 ieee80211_sta_get_rates(struct ieee80211_local *local, ...@@ -1297,8 +1297,7 @@ u32 ieee80211_sta_get_rates(struct ieee80211_local *local,
enum ieee80211_band band); enum ieee80211_band band);
int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata, int __ieee80211_request_smps(struct ieee80211_sub_if_data *sdata,
enum ieee80211_smps_mode smps_mode); enum ieee80211_smps_mode smps_mode);
void ieee80211_recalc_smps(struct ieee80211_local *local, void ieee80211_recalc_smps(struct ieee80211_local *local);
struct ieee80211_sub_if_data *forsdata);
size_t ieee80211_ie_split(const u8 *ies, size_t ielen, size_t ieee80211_ie_split(const u8 *ies, size_t ielen,
const u8 *ids, int n_ids, size_t offset); const u8 *ids, int n_ids, size_t offset);
......
...@@ -333,7 +333,7 @@ static void ieee80211_recalc_smps_work(struct work_struct *work) ...@@ -333,7 +333,7 @@ static void ieee80211_recalc_smps_work(struct work_struct *work)
container_of(work, struct ieee80211_local, recalc_smps); container_of(work, struct ieee80211_local, recalc_smps);
mutex_lock(&local->iflist_mtx); mutex_lock(&local->iflist_mtx);
ieee80211_recalc_smps(local, NULL); ieee80211_recalc_smps(local);
mutex_unlock(&local->iflist_mtx); mutex_unlock(&local->iflist_mtx);
} }
......
...@@ -913,7 +913,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata, ...@@ -913,7 +913,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
mutex_lock(&local->iflist_mtx); mutex_lock(&local->iflist_mtx);
ieee80211_recalc_ps(local, -1); ieee80211_recalc_ps(local, -1);
ieee80211_recalc_smps(local, sdata); ieee80211_recalc_smps(local);
mutex_unlock(&local->iflist_mtx); mutex_unlock(&local->iflist_mtx);
netif_tx_start_all_queues(sdata->dev); netif_tx_start_all_queues(sdata->dev);
......
...@@ -1297,16 +1297,12 @@ static int check_mgd_smps(struct ieee80211_if_managed *ifmgd, ...@@ -1297,16 +1297,12 @@ static int check_mgd_smps(struct ieee80211_if_managed *ifmgd,
} }
/* must hold iflist_mtx */ /* must hold iflist_mtx */
void ieee80211_recalc_smps(struct ieee80211_local *local, void ieee80211_recalc_smps(struct ieee80211_local *local)
struct ieee80211_sub_if_data *forsdata)
{ {
struct ieee80211_sub_if_data *sdata; struct ieee80211_sub_if_data *sdata;
enum ieee80211_smps_mode smps_mode = IEEE80211_SMPS_OFF; enum ieee80211_smps_mode smps_mode = IEEE80211_SMPS_OFF;
int count = 0; int count = 0;
if (forsdata)
lockdep_assert_held(&forsdata->u.mgd.mtx);
lockdep_assert_held(&local->iflist_mtx); lockdep_assert_held(&local->iflist_mtx);
/* /*
...@@ -1324,18 +1320,8 @@ void ieee80211_recalc_smps(struct ieee80211_local *local, ...@@ -1324,18 +1320,8 @@ void ieee80211_recalc_smps(struct ieee80211_local *local,
continue; continue;
if (sdata->vif.type != NL80211_IFTYPE_STATION) if (sdata->vif.type != NL80211_IFTYPE_STATION)
goto set; goto set;
if (sdata != forsdata) {
/* count += check_mgd_smps(&sdata->u.mgd, &smps_mode);
* 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);
if (count > 1) { if (count > 1) {
smps_mode = IEEE80211_SMPS_OFF; smps_mode = IEEE80211_SMPS_OFF;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment