Skip to content

Commit

Permalink
mac80211: lock rate control
Browse files Browse the repository at this point in the history
Both minstrel (reported by Sven Eckelmann) and the iwlwifi rate
control aren't properly taking concurrency into account. It's
likely that the same is true for other rate control algorithms.

In the case of minstrel this manifests itself in crashes when an
update and other data access are run concurrently, for example
when the stations change bandwidth or similar. In iwlwifi, this
can cause firmware crashes.

Since fixing all rate control algorithms will be very difficult,
just provide locking for invocations. This protects the internal
data structures the algorithms maintain.

I've manipulated hostapd to test this, by having it change its
advertised bandwidth roughly ever 150ms. At the same time, I'm
running a flood ping between the client and the AP, which causes
this race of update vs. get_rate/status to easily happen on the
client. With this change, the system survives this test.

Reported-by: Sven Eckelmann <sven@open-mesh.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Johannes Berg committed Apr 20, 2015
1 parent 48bf6be commit 35c347a
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
8 changes: 7 additions & 1 deletion net/mac80211/rate.c
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,13 @@ void rate_control_get_rate(struct ieee80211_sub_if_data *sdata,
if (sdata->local->hw.flags & IEEE80211_HW_HAS_RATE_CONTROL)
return;

ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
if (ista) {
spin_lock_bh(&sta->rate_ctrl_lock);
ref->ops->get_rate(ref->priv, ista, priv_sta, txrc);
spin_unlock_bh(&sta->rate_ctrl_lock);
} else {
ref->ops->get_rate(ref->priv, NULL, NULL, txrc);
}

if (sdata->local->hw.flags & IEEE80211_HW_SUPPORTS_RC_TABLE)
return;
Expand Down
14 changes: 11 additions & 3 deletions net/mac80211/rate.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,12 @@ static inline void rate_control_tx_status(struct ieee80211_local *local,
if (!ref || !test_sta_flag(sta, WLAN_STA_RATE_CONTROL))
return;

spin_lock_bh(&sta->rate_ctrl_lock);
if (ref->ops->tx_status)
ref->ops->tx_status(ref->priv, sband, ista, priv_sta, skb);
else
ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
spin_unlock_bh(&sta->rate_ctrl_lock);
}

static inline void
Expand All @@ -64,7 +66,9 @@ rate_control_tx_status_noskb(struct ieee80211_local *local,
if (WARN_ON_ONCE(!ref->ops->tx_status_noskb))
return;

spin_lock_bh(&sta->rate_ctrl_lock);
ref->ops->tx_status_noskb(ref->priv, sband, ista, priv_sta, info);
spin_unlock_bh(&sta->rate_ctrl_lock);
}

static inline void rate_control_rate_init(struct sta_info *sta)
Expand All @@ -91,8 +95,10 @@ static inline void rate_control_rate_init(struct sta_info *sta)

sband = local->hw.wiphy->bands[chanctx_conf->def.chan->band];

spin_lock_bh(&sta->rate_ctrl_lock);
ref->ops->rate_init(ref->priv, sband, &chanctx_conf->def, ista,
priv_sta);
spin_unlock_bh(&sta->rate_ctrl_lock);
rcu_read_unlock();
set_sta_flag(sta, WLAN_STA_RATE_CONTROL);
}
Expand All @@ -115,18 +121,20 @@ static inline void rate_control_rate_update(struct ieee80211_local *local,
return;
}

spin_lock_bh(&sta->rate_ctrl_lock);
ref->ops->rate_update(ref->priv, sband, &chanctx_conf->def,
ista, priv_sta, changed);
spin_unlock_bh(&sta->rate_ctrl_lock);
rcu_read_unlock();
}
drv_sta_rc_update(local, sta->sdata, &sta->sta, changed);
}

static inline void *rate_control_alloc_sta(struct rate_control_ref *ref,
struct ieee80211_sta *sta,
gfp_t gfp)
struct sta_info *sta, gfp_t gfp)
{
return ref->ops->alloc_sta(ref->priv, sta, gfp);
spin_lock_init(&sta->rate_ctrl_lock);
return ref->ops->alloc_sta(ref->priv, &sta->sta, gfp);
}

static inline void rate_control_free_sta(struct sta_info *sta)
Expand Down
2 changes: 1 addition & 1 deletion net/mac80211/sta_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ static int sta_prepare_rate_control(struct ieee80211_local *local,

sta->rate_ctrl = local->rate_ctrl;
sta->rate_ctrl_priv = rate_control_alloc_sta(sta->rate_ctrl,
&sta->sta, gfp);
sta, gfp);
if (!sta->rate_ctrl_priv)
return -ENOMEM;

Expand Down
1 change: 1 addition & 0 deletions net/mac80211/sta_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,7 @@ struct sta_info {
u8 ptk_idx;
struct rate_control_ref *rate_ctrl;
void *rate_ctrl_priv;
spinlock_t rate_ctrl_lock;
spinlock_t lock;

struct work_struct drv_deliver_wk;
Expand Down

0 comments on commit 35c347a

Please sign in to comment.