Commit 2438d430 authored by Johannes Berg's avatar Johannes Berg Committed by Luca Coelho

iwlwifi: mvm: fix delBA vs. NSSN queue sync race

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: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarLuca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20211204083237.44abbbc50f40.I5492600dfe513356555abe2d7df0e2835846e3d8@changeidSigned-off-by: default avatarLuca Coelho <luciano.coelho@intel.com>
parent 00d667fc
...@@ -766,8 +766,11 @@ static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm, ...@@ -766,8 +766,11 @@ static void iwl_mvm_release_frames_from_notif(struct iwl_mvm *mvm,
rcu_read_lock(); rcu_read_lock();
ba_data = rcu_dereference(mvm->baid_map[baid]); 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; goto out;
}
sta = rcu_dereference(mvm->fw_id_to_mac_id[ba_data->sta_id]); sta = rcu_dereference(mvm->fw_id_to_mac_id[ba_data->sta_id]);
if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta))) if (WARN_ON_ONCE(IS_ERR_OR_NULL(sta)))
......
...@@ -2684,6 +2684,16 @@ int iwl_mvm_sta_rx_agg(struct iwl_mvm *mvm, struct ieee80211_sta *sta, ...@@ -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); RCU_INIT_POINTER(mvm->baid_map[baid], NULL);
kfree_rcu(baid_data, rcu_head); kfree_rcu(baid_data, rcu_head);
IWL_DEBUG_HT(mvm, "BAID %d is free\n", baid); 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; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment