linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luca Coelho <luca@coelho.fi>
To: kvalo@codeaurora.org
Cc: luca@coelho.fi, linux-wireless@vger.kernel.org
Subject: [PATCH 01/12] iwlwifi: mvm: fix delBA vs. NSSN queue sync race
Date: Sat,  4 Dec 2021 08:35:44 +0200	[thread overview]
Message-ID: <iwlwifi.20211204083237.44abbbc50f40.I5492600dfe513356555abe2d7df0e2835846e3d8@changeid> (raw)
In-Reply-To: <20211204063555.769822-1-luca@coelho.fi>

From: Johannes Berg <johannes.berg@intel.com>

If we happen to decide an NSSN queue sync (IWL_MVM_RXQ_NSSN_SYNC)
for some remaining packets that are still on the queue, but just
after we've decided to do a delBA (which causes its own queues
sync with IWL_MVM_RXQ_NOTIF_DEL_BA) we can end up with a sequence
of events like this:

 CPU 1                              CPU 2

remove BA session with baid N
send IWL_MVM_RXQ_NOTIF_DEL_BA
send IWL_MVM_RXQ_NSSN_SYNC
get IWL_MVM_RXQ_NOTIF_DEL_BA
                                    get IWL_MVM_RXQ_NOTIF_DEL_BA
get IWL_MVM_RXQ_NSSN_SYNC
complete IWL_MVM_RXQ_NOTIF_DEL_BA
remove N from baid_map[]
                                    get IWL_MVM_RXQ_NSSN_SYNC
                                    WARN_ON(!baid_map[N])

Thus, there's a race that leads in hitting the WARN_ON, but more
importantly, it's a race that potentially even results in a new
aggregation session getting assigned to baid N.

To fix this, remove the WARN_ON() in the NSSN_SYNC case, we can't
completely protect against hitting this case, so we shouldn't be
warning. However, guard ourselves against BAID reuse by doing yet
another round of queue synchronization after the entry is removed
from the baid_map, so that it cannot be reused with any in-flight
IWL_MVM_RXQ_NSSN_SYNC messages.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c |  5 ++++-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c  | 10 ++++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
index e0601f802628..03e90087576c 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c
@@ -766,8 +766,11 @@ static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
 	rcu_read_lock();
 
 	ba_data = rcu_dereference(mvm->baid_map[baid]);
-	if (WARN_ON_ONCE(!ba_data))
+	if (!ba_data) {
+		WARN(!(flags & IWL_MVM_RELEASE_FROM_RSS_SYNC),
+		     "BAID %d not found in map\n", baid);
 		goto out;
+	}
 
 	sta = rcu_dereference(mvm->fw_id_to_mac_id[ba_data->sta_id]);
 	if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta)))
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index a64874c05ced..feab0bfcd7a2 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -2684,6 +2684,16 @@ int iwl_mvm_sta_rx_agg(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
 		RCU_INIT_POINTER(mvm->baid_map[baid], NULL);
 		kfree_rcu(baid_data, rcu_head);
 		IWL_DEBUG_HT(mvm, "BAID %d is free\n", baid);
+
+		/*
+		 * After we've deleted it, do another queue sync
+		 * so if an IWL_MVM_RXQ_NSSN_SYNC was concurrently
+		 * running it won't find a new session in the old
+		 * BAID. It can find the NULL pointer for the BAID,
+		 * but we must not have it find a different session.
+		 */
+		iwl_mvm_sync_rx_queues_internal(mvm, IWL_MVM_RXQ_EMPTY,
+						true, NULL, 0);
 	}
 	return 0;
 
-- 
2.33.1


  reply	other threads:[~2021-12-04  6:36 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-04  6:35 [PATCH 00/12] iwlwifi: updates intended for v5.16 2021-12-04 part 1 Luca Coelho
2021-12-04  6:35 ` Luca Coelho [this message]
2021-12-04  6:35 ` [PATCH 02/12] iwlwifi: mvm: synchronize with FW after multicast commands Luca Coelho
2021-12-04  6:35 ` [PATCH 03/12] iwlwifi: properly support 4-bit MAC step Luca Coelho
2021-12-07 13:44   ` Kalle Valo
2021-12-07 13:49     ` Luca Coelho
2021-12-07 13:58       ` Kalle Valo
2021-12-07 14:05   ` [PATCH v2 03/12] iwlwifi: support 4-bits in MAC step value Luca Coelho
2021-12-04  6:35 ` [PATCH 04/12] iwlwifi: add support for Bz-Z HW Luca Coelho
2021-12-04  6:35 ` [PATCH 05/12] iwlwifi: mvm: d3: move GTK rekeys condition Luca Coelho
2021-12-04  6:35 ` [PATCH 06/12] iwlwifi: pcie: support Bz suspend/resume trigger Luca Coelho
2021-12-04  6:35 ` [PATCH 07/12] iwlwifi: mvm: parse firmware alive message version 6 Luca Coelho
2021-12-04  6:35 ` [PATCH 08/12] iwlwifi: mvm: d3: support v12 wowlan status Luca Coelho
2021-12-04  6:35 ` [PATCH 09/12] iwlwifi: mvm: support RLC configuration command Luca Coelho
2021-12-04  6:35 ` [PATCH 10/12] iwlwifi: fw: api: add link to PHY context command struct v1 Luca Coelho
2021-12-04  6:35 ` [PATCH 11/12] iwlwifi: mvm: add support for PHY context command v4 Luca Coelho
2021-12-04  6:35 ` [PATCH 12/12] iwlwifi: remove unused iwlax210_2ax_cfg_so_hr_a0 structure Luca Coelho
2021-12-07 13:49 ` [PATCH 00/12] iwlwifi: updates intended for v5.16 2021-12-04 part 1 Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=iwlwifi.20211204083237.44abbbc50f40.I5492600dfe513356555abe2d7df0e2835846e3d8@changeid \
    --to=luca@coelho.fi \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).