mac80211: fix tx aggregation locking issue
Signed-off-by: Felix Fietkau <nbd@nbd.name>
This commit is contained in:
		| @@ -0,0 +1,79 @@ | |||||||
|  | From: Johannes Berg <johannes.berg@intel.com> | ||||||
|  | Date: Mon, 29 Nov 2021 15:32:47 +0200 | ||||||
|  | Subject: [PATCH] mac80211: agg-tx: don't schedule_and_wake_txq() under | ||||||
|  |  sta->lock | ||||||
|  |  | ||||||
|  | When we call ieee80211_agg_start_txq(), that will in turn call | ||||||
|  | schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb() | ||||||
|  | this is done under sta->lock, which leads to certain circular | ||||||
|  | lock dependencies, as reported by Chris Murphy: | ||||||
|  | https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com | ||||||
|  |  | ||||||
|  | In general, ieee80211_agg_start_txq() is usually not called | ||||||
|  | with sta->lock held, only in this one place. But it's always | ||||||
|  | called with sta->ampdu_mlme.mtx held, and that's therefore | ||||||
|  | clearly sufficient. | ||||||
|  |  | ||||||
|  | Change ieee80211_stop_tx_ba_cb() to also call it without the | ||||||
|  | sta->lock held, by factoring it out of ieee80211_remove_tid_tx() | ||||||
|  | (which is only called in this one place). | ||||||
|  |  | ||||||
|  | This breaks the locking chain and makes it less likely that | ||||||
|  | we'll have similar locking chain problems in the future. | ||||||
|  |  | ||||||
|  | Reported-by: Chris Murphy <lists@colorremedies.com> | ||||||
|  | Signed-off-by: Johannes Berg <johannes.berg@intel.com> | ||||||
|  | Signed-off-by: Luca Coelho <luciano.coelho@intel.com> | ||||||
|  | --- | ||||||
|  |  | ||||||
|  | --- a/net/mac80211/agg-tx.c | ||||||
|  | +++ b/net/mac80211/agg-tx.c | ||||||
|  | @@ -9,7 +9,7 @@ | ||||||
|  |   * Copyright 2007, Michael Wu <flamingice@sourmilk.net> | ||||||
|  |   * Copyright 2007-2010, Intel Corporation | ||||||
|  |   * Copyright(c) 2015-2017 Intel Deutschland GmbH | ||||||
|  | - * Copyright (C) 2018 - 2020 Intel Corporation | ||||||
|  | + * Copyright (C) 2018 - 2021 Intel Corporation | ||||||
|  |   */ | ||||||
|  |   | ||||||
|  |  #include <linux/ieee80211.h> | ||||||
|  | @@ -213,6 +213,8 @@ ieee80211_agg_start_txq(struct sta_info | ||||||
|  |  	struct ieee80211_txq *txq = sta->sta.txq[tid]; | ||||||
|  |  	struct txq_info *txqi; | ||||||
|  |   | ||||||
|  | +	lockdep_assert_held(&sta->ampdu_mlme.mtx); | ||||||
|  | + | ||||||
|  |  	if (!txq) | ||||||
|  |  		return; | ||||||
|  |   | ||||||
|  | @@ -290,7 +292,6 @@ static void ieee80211_remove_tid_tx(stru | ||||||
|  |  	ieee80211_assign_tid_tx(sta, tid, NULL); | ||||||
|  |   | ||||||
|  |  	ieee80211_agg_splice_finish(sta->sdata, tid); | ||||||
|  | -	ieee80211_agg_start_txq(sta, tid, false); | ||||||
|  |   | ||||||
|  |  	kfree_rcu(tid_tx, rcu_head); | ||||||
|  |  } | ||||||
|  | @@ -889,6 +890,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_ | ||||||
|  |  { | ||||||
|  |  	struct ieee80211_sub_if_data *sdata = sta->sdata; | ||||||
|  |  	bool send_delba = false; | ||||||
|  | +	bool start_txq = false; | ||||||
|  |   | ||||||
|  |  	ht_dbg(sdata, "Stopping Tx BA session for %pM tid %d\n", | ||||||
|  |  	       sta->sta.addr, tid); | ||||||
|  | @@ -906,10 +908,14 @@ void ieee80211_stop_tx_ba_cb(struct sta_ | ||||||
|  |  		send_delba = true; | ||||||
|  |   | ||||||
|  |  	ieee80211_remove_tid_tx(sta, tid); | ||||||
|  | +	start_txq = true; | ||||||
|  |   | ||||||
|  |   unlock_sta: | ||||||
|  |  	spin_unlock_bh(&sta->lock); | ||||||
|  |   | ||||||
|  | +	if (start_txq) | ||||||
|  | +		ieee80211_agg_start_txq(sta, tid, false); | ||||||
|  | + | ||||||
|  |  	if (send_delba) | ||||||
|  |  		ieee80211_send_delba(sdata, sta->sta.addr, tid, | ||||||
|  |  			WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE); | ||||||
		Reference in New Issue
	
	Block a user
	 Felix Fietkau
					Felix Fietkau