mac80211: fix aggregation timer lockups

As far as I can tell, there are possible lockups because both the RX
session_timer and TX addba_resp_timer are del_timer_sync'ed under
the sta spinlock which both timer functions take. Additionally, the
TX agg code seems to leak memory when TX aggregation is not disabled
before the sta_info is freed.

Fix this by making the free code a little smarter in the RX agg case,
and actually make the sta_info_destroy code free the TX agg info in
the TX agg case. We won't notify the peer, but it'll notice something
is wrong anyway, and normally this only happens after we've told it
in some other way we will no longer talk to it.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index d7afd0956970568a559ba11a647b8c55f1d9487c..4b571b2116258df96b330e6a9808cd6690ecb7f6 100644 (file)
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -78,11 +78,18 @@ void ieee80211_sta_stop_rx_ba_session(struct ieee80211_sub_if_data *sdata, u8 *r
                        sta->ampdu_mlme.tid_rx[tid]->reorder_buf[i] = NULL;
                }
        }
+
+       spin_lock_bh(&sta->lock);
        /* free resources */
        kfree(sta->ampdu_mlme.tid_rx[tid]->reorder_buf);
-       kfree(sta->ampdu_mlme.tid_rx[tid]);
-       sta->ampdu_mlme.tid_rx[tid] = NULL;
+
+       if (!sta->ampdu_mlme.tid_rx[tid]->shutdown) {
+               kfree(sta->ampdu_mlme.tid_rx[tid]);
+               sta->ampdu_mlme.tid_rx[tid] = NULL;
+       }
+
        sta->ampdu_mlme.tid_state_rx[tid] = HT_AGG_STATE_IDLE;
+       spin_unlock_bh(&sta->lock);
 
        rcu_read_unlock();
 }

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 10c5539c20abdcafdd8fc1461914b2b9b43ba3ff..634f65c0130e2ca0f21c8a2645d2d4f4352d7dd8 100644 (file)
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -194,12 +194,41 @@ void sta_info_destroy(struct sta_info *sta)
                dev_kfree_skb_any(skb);
 
        for (i = 0; i <  STA_TID_NUM; i++) {
+               struct tid_ampdu_rx *tid_rx;
+               struct tid_ampdu_tx *tid_tx;
+
                spin_lock_bh(&sta->lock);
-               if (sta->ampdu_mlme.tid_rx[i])
-                 del_timer_sync(&sta->ampdu_mlme.tid_rx[i]->session_timer);
-               if (sta->ampdu_mlme.tid_tx[i])
-                 del_timer_sync(&sta->ampdu_mlme.tid_tx[i]->addba_resp_timer);
+               tid_rx = sta->ampdu_mlme.tid_rx[i];
+               /* Make sure timer won't free the tid_rx struct, see below */
+               if (tid_rx)
+                       tid_rx->shutdown = true;
                spin_unlock_bh(&sta->lock);
+
+               /*
+                * Outside spinlock - shutdown is true now so that the timer
+                * won't free tid_rx, we have to do that now. Can't let the
+                * timer do it because we have to sync the timer outside the
+                * lock that it takes itself.
+                */
+               if (tid_rx) {
+                       del_timer_sync(&tid_rx->session_timer);
+                       kfree(tid_rx);
+               }
+
+               /*
+                * No need to do such complications for TX agg sessions, the
+                * path leading to freeing the tid_tx struct goes via a call
+                * from the driver, and thus needs to look up the sta struct
+                * again, which cannot be found when we get here. Hence, we
+                * just need to delete the timer and free the aggregation
+                * info; we won't be telling the peer about it then but that
+                * doesn't matter if we're not talking to it again anyway.
+                */
+               tid_tx = sta->ampdu_mlme.tid_tx[i];
+               if (tid_tx) {
+                       del_timer_sync(&tid_tx->addba_resp_timer);
+                       kfree(tid_tx);
+               }
        }
 
        __sta_info_free(local, sta);

diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index a070bd929e00aaf4693759edaa452c2b34b1b991..d9653231992fb1e882429ada4e871ba50e1a435d 100644 (file)
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -100,6 +100,7 @@ struct tid_ampdu_rx {
        u16 buf_size;
        u16 timeout;
        u8 dialog_token;
+       bool shutdown;
 };
 
 /**