]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
wifi: mac80211: fix tid_tx use-after-free on BA session stop
authorZhao Li <enderaoelyther@gmail.com>
Tue, 28 Jul 2026 11:21:56 +0000 (19:21 +0800)
committerJohannes Berg <johannes.berg@intel.com>
Tue, 28 Jul 2026 13:03:42 +0000 (15:03 +0200)
ieee80211_stop_tx_ba_cb() hands tid_tx to kfree_rcu() through
ieee80211_remove_tid_tx(), and then reads tid_tx->ndp after dropping
sta->lock:

ieee80211_remove_tid_tx(sta, tid); /* kfree_rcu(tid_tx, rcu_head) */
...
spin_unlock_bh(&sta->lock);

if (start_txq)
ieee80211_agg_start_txq(sta, tid, false);

if (send_delba)
ieee80211_send_delba(..., tid_tx->ndp);

That read is not covered by an RCU read-side critical section, and it runs
in preemptible process context: both callers hold the wiphy mutex, reaching
it either from the ieee80211_ba_session_work() wiphy work or from
ieee80211_sta_tear_down_BA_sessions() during station teardown.
Softirqs can run in that window too, both from the local_bh_enable() that
ends ieee80211_agg_start_txq() and from any interrupt exit, so the RCU
callback can free tid_tx before the read.

Driving the function from a test module with the grace period forced into
that window, KASAN reports the read, and the free arrives on the ordinary
RCU softirq path:

  BUG: KASAN: slab-use-after-free in ieee80211_stop_tx_ba_cb+0x3cd/0x400
  Read of size 1 at addr ffff888002b9f52e by task kworker/0:1/10
  [...]
  Freed by task 57:
   __kasan_slab_free+0x47/0x70
   __rcu_free_sheaf_prepare+0x70/0x250
   rcu_free_sheaf_nobarn+0x18/0x40
   rcu_core+0x426/0x1310
   handle_softirqs+0x144/0x590
   __irq_exit_rcu+0xea/0x150
   irq_exit_rcu+0x9/0x20
   sysvec_apic_timer_interrupt+0x6b/0x80
   asm_sysvec_apic_timer_interrupt+0x1a/0x20

send_delba is only set when tx_stop is set, which happens for
AGG_STOP_LOCAL_REQUEST alone, so this is reached on local teardown -
session idle timeout, PTK rekey, suspend, HW reconfig - and not from a
peer's DELBA.

Read ndp into a local before the session is freed, while sta->lock is still
held. tid_tx->ndp has a single writer, in
ieee80211_tx_ba_session_handle_start(), which cannot run concurrently here:
both paths are serialised by the wiphy mutex, and the session is already
marked HT_AGG_STATE_STOPPING at this point. tid_tx->ndp is also the only
tid_tx dereference left after ieee80211_remove_tid_tx() in this function.

Fixes: 98acd4c1d9f7 ("wifi: mac80211: add support for NDP ADDBA/DELBA for S1G")
Assisted-by: Codex:gpt-5.6-sol
Assisted-by: Kimi:K3
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
Link: https://patch.msgid.link/20260728112156.96822-1-enderaoelyther@gmail.com
[move/change the comment a bit to be more general not just on ndp,
 initialize ndp directly]
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
net/mac80211/agg-tx.c

index 4833b46770b62c77598c56f55494cecd36eda037..0832213430f4319d58386fca7d9258f65174f694 100644 (file)
@@ -915,6 +915,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
                             struct tid_ampdu_tx *tid_tx)
 {
        struct ieee80211_sub_if_data *sdata = sta->sdata;
+       bool ndp = ndp = tid_tx->ndp;
        bool send_delba = false;
        bool start_txq = false;
 
@@ -934,6 +935,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
                send_delba = true;
 
        ieee80211_remove_tid_tx(sta, tid);
+       /* tid_tx is now invalid since ieee80211_remove_tid_tx() frees it */
        start_txq = true;
 
  unlock_sta:
@@ -946,7 +948,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
                ieee80211_send_delba(sdata, sta->sta.addr, tid,
                                     WLAN_BACK_INITIATOR,
                                     WLAN_REASON_QSTA_NOT_USE,
-                                    tid_tx->ndp);
+                                    ndp);
 }
 
 void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,