Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jean-Pierre TOSONI <jp.tosoni@acksys.fr>
To:     "Johannes Berg (johannes@sipsolutions.net)" 
        <johannes@sipsolutions.net>
CC:     "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Subject: [RFC v2] mac80211: fix rx blockack session race condition
Thread-Topic: [RFC v2] mac80211: fix rx blockack session race condition
Thread-Index: AdfKc+9SA/Z+MZzkSXqeBIjUqq1f9w==
Date:   Tue, 26 Oct 2021 14:19:20 +0000
Message-ID: <DB9PR01MB73541EC2ECFCEF6521B86AFFE4849@DB9PR01MB7354.eurprd01.prod.exchangelabs.com>
Accept-Language: fr-FR, en-US
Content-Language: fr-FR
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?L4wky4C2xrmHwtsNvJAWTVGhrMAn27cXeIeV1+ZnbC7gE81ngqzu/MhzTPOl?=
 =?us-ascii?Q?nK9gUHUZe6drKtLnW4Ri3EZNoa1IJRPkgyHtu31ujgJLhCyShHcHvKhpiIA4?=
 =?us-ascii?Q?+zxJuSf+E+aiGOwZY8zTyLyWDkF/nuIhlp3+VljVRH/q6vzRvgU1dg73fd04?=
 =?us-ascii?Q?U2734PxvfL21+jhkpNe8b/uIDDLAiPfnxcymjYEpUPC8figIrWNuNloAeqjq?=
 =?us-ascii?Q?kwJJj0Ayq3at0k17Un+1apYRC2R00M4fDZXoH7/Sw0YMWANgiPiiwKhmV1j8?=
 =?us-ascii?Q?y/2bAourXoSor0UR1zFyiGRX5mkyYDwVaFqYKDxxNwp9cRQ8aUwfrk6ntqWT?=
 =?us-ascii?Q?AL8FILImiI7qOoZ3BdVJW2FVELe02dJ+EuR56Gpg+t8FS6xYXByRIQ3X59qT?=
 =?us-ascii?Q?4Tnh5up4nJtrzzrDj2/uWvnzrUak35CUaSWIA3fkUFCGS5iAmPwKwdq/nXBP?=
 =?us-ascii?Q?0pgWw6Hhxn28k9BWpEga6OR/xHoc1AUeRctBuilanQimXPofVxuRH00UuAIC?=
 =?us-ascii?Q?NKWdGW7ZaxeNBCI/ZrSKeVwjaTGTasyDDSV2r/x4FIYIMaQtvTZfcXtBnH6G?=
 =?us-ascii?Q?zMQsjQ37AlEttIZwmHHB8YYKggCNtaA2TzPuWuPWhzxPlTjs1atprUrxXiA7?=
 =?us-ascii?Q?+phl6OQUMEw7JFSd0UilpNl1/7bASA0jhvdJOKq4yBrzfx3OfFFxMv0gqpbM?=
 =?us-ascii?Q?vB6BiGJP8PMcEyu8seFAyWE56OznPVatIZQxW9+Xv4Ny7Rn4ZLrulXJvdtON?=
 =?us-ascii?Q?eFhZAnK9tMqyyCg5esUCjiqggMjdQ00q+82MHl4gS5BdBJBOK5tmIyKfZadw?=
 =?us-ascii?Q?eT2fVb+N4O5yfTey6ZOqpraebkjcAmlnekCy0jfwh3crndO/bYcQlhSuAxHM?=
 =?us-ascii?Q?LNCfW0pFncxyMJp75i/O18j4VuyQYC4PpGBqdL+pz0JH5P/KVIHA/nL2BBnq?=
 =?us-ascii?Q?iAwqi2iiJuXsN7jRpvHVoK6azhFHXprikiok2vkKltz/HRk6MUDxWdwhGTqM?=
 =?us-ascii?Q?e0lQaKyJRWOtsr+Fhnd1435QMxvCQGk5Ox/x8mcOzTSdMb4TFO/q3S9QJPfY?=
 =?us-ascii?Q?sRN0FmyNP7vMnfdGSNPNTlWTM0j9sbqz1bD9ZCgBiaW1FlCrQXKBydikEK30?=
 =?us-ascii?Q?cvpvHxDwcoKT8S6o/e3k8Y7M2l2LshM/eRB9hCo2MtWizDkEId5QgeaH6gGR?=
 =?us-ascii?Q?f6EFOmAHHZhYrqvChFdbY9uH5sc5rB9PkczuVfqphqhnlPppPO9QYA2vljE3?=
 =?us-ascii?Q?VYBu2HvKKk3bp43xsOIbytNbp7BXVpDqa2QrXNY8HC/Fj+jDwqYZIG8gz3GO?=
 =?us-ascii?Q?UAyZrohrI2CqdJYPbW3pspwBZ4X6GE8f6iXJXdp7laINdvFMx3k8mq7qsQU6?=
 =?us-ascii?Q?izvWzP/9hYwEOQpMg3MWr1nOO3eE7cFgRBY0dPvfCHr6TwOuEn+F7pGP0wax?=
 =?us-ascii?Q?wg6eUF7wUX3cyBV9/JUYaD/8ehnW4aCdcI+bfpD1/5BXHXJ0fAzkSP2frZXt?=
 =?us-ascii?Q?LZHK2MNhTlXDJk9sijfn1ZVWpA4NmxPuTauaH6Y4V/+TMvXInvOHM5vyh7uh?=
 =?us-ascii?Q?Sn9s3v3xYmIKmHM76lA6KxUCocB+fUQAtHTwT4+O6tZYdZrSCxf9lpzZQIO9?=
 =?us-ascii?Q?2A=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: acksys.fr
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DB9PR01MB7354.eurprd01.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 65ce50b8-db89-41f3-4616-08d9988b9a48
X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Oct 2021 14:19:20.4695
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: f18a6414-d5f3-4b5c-9345-f30c01d87e32
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ot1FcXUixVGI+9QAuYTCdnSXHX013JE06iYg85IzWszZV2wSmZ7nUqgLCrnS55npyazNT5Ik822uFFYNHEAPjw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB7PR01MB3993
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

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.
---
V2: remove debugging code leftovers, sorry for that

Index: b/net/mac80211/rx.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
Index: bp/net/mac80211/rx.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- bp.orig/net/mac80211/rx.c
+++ bp/net/mac80211/rx.c
@@ -3008,11 +3008,18 @@ ieee80211_rx_h_ctrl(struct ieee80211_rx_
=20
 		tid =3D le16_to_cpu(bar_data.control) >> 12;
=20
+		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 */
+		    !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);
+		}
=20
 		tid_agg_rx =3D rcu_dereference(rx->sta->ampdu_mlme.tid_rx[tid]);
 		if (!tid_agg_rx)
--
quilt 0.63