Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Sven Eckelmann <sven@narfation.org>
Date:   Tue, 01 Aug 2023 19:38:17 +0200
Subject: [PATCH RFC] ath11k: Don't drop tx_status when peer cannot be found
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-Id: <20230801-ath11k-ack_status_leak-v1-1-539cb72c55bc@narfation.org>
To:     Kalle Valo <kvalo@kernel.org>,
        Jeff Johnson <quic_jjohnson@quicinc.com>,
        Pradeep Kumar Chitrapu <quic_pradeepc@quicinc.com>
Cc:     Kalle Valo <quic_kvalo@quicinc.com>, ath11k@lists.infradead.org,
        linux-wireless@vger.kernel.org, Felix Fietkau <nbd@nbd.name>,
        Sven Eckelmann <sven@narfation.org>
Precedence: bulk

When a station idles for a long time, hostapd will try to send a QoS Null
frame to the station as "poll". NL80211_CMD_PROBE_CLIENT is used for this
purpose. And the skb will be added to ack_status_frame - waiting for a
tx_complete via ieee80211_tx_status*();

But when the peer was already removed before the tx_complete arrives, the
peer will be missing and thus the entry will not be removed from
ack_status_frame. This IDR will therefore run full after 8K clients which
disappeared this way - the access point will then just stall and not allow
any new clients because idr_alloc for ack_status_frame will fail.

Tested-on: IPQ6018 hw1.0 WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1

Fixes: 6257c702264c ("wifi: ath11k: fix tx status reporting in encap offload mode")
Fixes: 94739d45c388 ("ath11k: switch to using ieee80211_tx_status_ext()")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
This problem can be seen with QCA's ath11k fork as:

  attach ack fail -28

when new clients try to connect - and connection attempt will obviously
fail. Most likely with an "deauthenticated due to inactivity (timer
DEAUTH/REMOVE)" by hostapd.

And the fix (required for both platches) would then be something like:

  --- a/drivers/net/wireless/ath/ath11k/dp_tx.c
  +++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
  @@ -629,8 +629,14 @@ static void ath11k_dp_tx_complete_msdu(struct ath11k *ar,
   			   "dp_tx: failed to find the peer with peer_id %d\n",
   			    ts->peer_id);
   		spin_unlock_bh(&ab->base_lock);
  -		dev_kfree_skb_any(msdu);
  -		goto exit;
  +		rcu_read_unlock();
  +
  +		if (skb_cb->flags & ATH11K_SKB_HW_80211_ENCAP)
  +			ieee80211_tx_status_8023(ar->hw, skb_cb->vif, msdu);
  +		else
  +			ieee80211_tx_status(ar->hw, msdu);
  +
  +		return;
   	}
   	arsta = (struct ath11k_sta *)peer->sta->drv_priv;
   	status.sta = peer->sta;

But this is not possible any longer because Felix Fietkau removed
ieee80211_tx_status_8023 in commit 9ae708f00161 ("wifi: mac80211: remove
ieee80211_tx_status_8023") - and the function ieee80211_lookup_ra_sta
(required for this task) is currently not exported. And the sta information
is required to reach the ieee80211_sta_tx_notify code section in
ieee80211_tx_status_ext()
---
 drivers/net/wireless/ath/ath11k/dp_tx.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_tx.c b/drivers/net/wireless/ath/ath11k/dp_tx.c
index a34833de7c67..c25283a11d27 100644
--- a/drivers/net/wireless/ath/ath11k/dp_tx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_tx.c
@@ -369,7 +369,14 @@ ath11k_dp_tx_htt_tx_complete_buf(struct ath11k_base *ab,
 			   "dp_tx: failed to find the peer with peer_id %d\n",
 			    ts->peer_id);
 		spin_unlock_bh(&ab->base_lock);
-		dev_kfree_skb_any(msdu);
+
+		/* report back to clean up resources - even when the peer
+		 * is not known by ath11k.
+		 *
+		 * TODO handle also for ATH11K_SKB_HW_80211_ENCAP
+		 */
+		if (!(skb_cb->flags & ATH11K_SKB_HW_80211_ENCAP))
+			ieee80211_tx_status(ar->hw, msdu);
 		return;
 	}
 	spin_unlock_bh(&ab->base_lock);
@@ -624,7 +631,14 @@ static void ath11k_dp_tx_complete_msdu(struct ath11k *ar,
 			   "dp_tx: failed to find the peer with peer_id %d\n",
 			    ts->peer_id);
 		spin_unlock_bh(&ab->base_lock);
-		dev_kfree_skb_any(msdu);
+
+		/* report back to clean up resources - even when the peer
+		 * is not known by ath11k.
+		 *
+		 * TODO handle also for ATH11K_SKB_HW_80211_ENCAP
+		 */
+		if (!(skb_cb->flags & ATH11K_SKB_HW_80211_ENCAP))
+			ieee80211_tx_status(ar->hw, msdu);
 		return;
 	}
 	arsta = (struct ath11k_sta *)peer->sta->drv_priv;

---
base-commit: 1d7dd5aa35474e553b8671b58579e0749b560779
change-id: 20230801-ath11k-ack_status_leak-2338eda5a721

Best regards,
-- 
Sven Eckelmann <sven@narfation.org>