mac80211: lock rate control

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>

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index d53355b011f5cf6407367e3c5f1a3e513d1cc08e..de69adf24f53f8ab8213c1b0f4ab32e77400bf23 100644 (file)
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -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;

diff --git a/net/mac80211/rate.h b/net/mac80211/rate.h
index 38652f09feaf250ec0517e23c8f147c8921f7499..25c9be5dd7fd811b32d13c792cf50c661f6f1e44 100644 (file)
--- a/net/mac80211/rate.h
+++ b/net/mac80211/rate.h
@@ -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
@@ -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)
@@ -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);
 }
@@ -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)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 53ab4bd1a44c56d50a3ed1be79821158996584fd..0800e02cce05fa4720d0251a4b8a148bccb1708c 100644 (file)
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -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;
 

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 40ad52e447ec5e5977be883d7a9bc7104c9f12f0..e4c4f871ffeeee65257d913752872840d783b220 100644 (file)
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -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;