diff mbox series

[RFC] mac80211: fix rx blockack session race condition

Message ID DB9PR01MB735472EB045E560573F9C369E4849@DB9PR01MB7354.eurprd01.prod.exchangelabs.com (mailing list archive)
State RFC
Delegated to: Johannes Berg
Headers show
Series [RFC] mac80211: fix rx blockack session race condition | expand

Commit Message

Jean-Pierre TOSONI Oct. 26, 2021, 2:04 p.m. UTC
When used with ath10k, the following may happen:
a) radio card firmware receives ADDBA-REQ from peer
b) radio sends back ADDBA-RESP to peer and signals the ath10k driver
c) ath10k calls back ieee80211_manage_rx_ba_offl() in mac80211
   to signal rx ba offloading
d) mac80211::agg-rx.c::ieee80211_manage_rx_ba_offl()
  d1) sets a flag: sta->ampdu_mlme.tid_rx_manage_offl
  d2) queues a call to ht.c::ieee80211_ba_session_work()
e) ...scheduler runs...
f) ht.c::ieee80211_ba_session_work() checks the flag, clears it
   and sets up the rx ba session.

During (e), a fast peer may already have sent a BAREQ which is
propagated to rx.c::ieee80211_rx_h_ctrl(). Since the session is not
yet established, mac80211 sends back a DELBA to the peer, which can
hang the BA session.

The phenomenon can be observed between two QCA988X fw 10.2.4 radios,
using a loop of associate/arping from client to AP/disconnect. After
a few thousand loops, arping does not get a response and a sniffer
detects a DELBA action frame from the client, following an ADDBA.

Fix:
1) check the offload flag in addition to the check for a valid
   aggregation session
2) surround the checks with the existing dedicated mutex, to avoid
   interference from ieee80211_ba_session_work() during the check.

Note 1: there is another dubious DELBA generation in
ieee80211_rx_reorder_ampdu(), where the same kind of fix should fit,
but I did not fix it since I knew no easy way to test.

Note 2: this fix applies to wireless backports from 5.4-rc8.
---
--
quilt 0.63
diff mbox series

Patch

Index: b/net/mac80211/rx.c
===================================================================
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2992,6 +2992,7 @@  ieee80211_rx_h_ctrl(struct ieee80211_rx_
 		return RX_CONTINUE;
 
 	if (ieee80211_is_back_req(bar->frame_control)) {
+int ii = 99;
 		struct {
 			__le16 control, start_seq_num;
 		} __packed bar_data;
@@ -3008,11 +3009,19 @@  ieee80211_rx_h_ctrl(struct ieee80211_rx_
 
 		tid = le16_to_cpu(bar_data.control) >> 12;
 
+		mutex_lock(&rx->sta->ampdu_mlme.mtx);
 		if (!test_bit(tid, rx->sta->ampdu_mlme.agg_session_valid) &&
-		    !test_and_set_bit(tid, rx->sta->ampdu_mlme.unexpected_agg))
+		    /* back_req is allowed if the fw just received addba */
+		    !(ii=test_bit(tid, rx->sta->ampdu_mlme.tid_rx_manage_offl)) &&
+		    !test_and_set_bit(tid, rx->sta->ampdu_mlme.unexpected_agg)) {
+			mutex_unlock(&rx->sta->ampdu_mlme.mtx);
 			ieee80211_send_delba(rx->sdata, rx->sta->sta.addr, tid,
 					     WLAN_BACK_RECIPIENT,
 					     WLAN_REASON_QSTA_REQUIRE_SETUP);
+		} else {
+			mutex_unlock(&rx->sta->ampdu_mlme.mtx);
+if (ii != 99) printk(KERN_ERR "JPT delba avoided, rx ba offload=%d", ii);
+		}
 
 		tid_agg_rx = rcu_dereference(rx->sta->ampdu_mlme.tid_rx[tid]);
 		if (!tid_agg_rx)