linux-kernel - [35/80] mac80211: fix race between the AGG SM and the Tx data path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20111207160825.541355918@clark.kroah.org>
Date:	Wed, 07 Dec 2011 08:06:55 -0800
From:	Greg KH <gregkh@...e.de>
To:	<linux-kernel@...r.kernel.org>, <stable@...r.kernel.org>
Cc:	<torvalds@...ux-foundation.org>, <akpm@...ux-foundation.org>,
	<alan@...rguk.ukuu.org.uk>,
	Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
	"John W. Linville" <linville@...driver.com>
Subject: [35/80] mac80211: fix race between the AGG SM and the Tx data path

3.0-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Emmanuel Grumbach <emmanuel.grumbach@...el.com>

commit 2a1e0fd175dcfd72096ba9291d31e3b1b5342e60 upstream.

When a packet is supposed to sent be as an a-MPDU, mac80211 sets
IEEE80211_TX_CTL_AMPDU to let the driver know. On the other
hand, mac80211 configures the driver for aggregration with the
ampdu_action callback.
There is race between these two mechanisms since the following
scenario can occur when the BA agreement is torn down:

Tx softIRQ	 			drv configuration
==========				=================

check OPERATIONAL bit
Set the TX_CTL_AMPDU bit in the packet

					clear OPERATIONAL bit
					stop Tx AGG
Pass Tx packet to the driver.

In that case the driver would get a packet with TX_CTL_AMPDU set
although it has already been notified that the BA session has been
torn down.

To fix this, we need to synchronize all the Qdisc activity after we
cleared the OPERATIONAL bit. After that step, all the following
packets will be buffered until the driver reports it is ready to get
new packets for this RA / TID. This buffering allows not to run into
another race that would send packets with TX_CTL_AMPDU unset while
the driver hasn't been requested to tear down the BA session yet.

This race occurs in practice and iwlwifi complains with a WARN_ON
when it happens.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@...el.com>
Reviewed-by: Johannes Berg <johannes@...solutions.net>
Signed-off-by: John W. Linville <linville@...driver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>

---
 net/mac80211/agg-tx.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -194,6 +194,20 @@ int ___ieee80211_stop_tx_ba_session(stru
 	 */
 	clear_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state);

+	/*
+	 * There might be a few packets being processed right now (on
+	 * another CPU) that have already gotten past the aggregation
+	 * check when it was still OPERATIONAL and consequently have
+	 * IEEE80211_TX_CTL_AMPDU set. In that case, this code might
+	 * call into the driver at the same time or even before the
+	 * TX paths calls into it, which could confuse the driver.
+	 *
+	 * Wait for all currently running TX paths to finish before
+	 * telling the driver. New packets will not go through since
+	 * the aggregation session is no longer OPERATIONAL.
+	 */
+	synchronize_net();
+
 	tid_tx->stop_initiator = initiator;
 	tid_tx->tx_stop = tx;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/