[PATCH 0/5] ath9k: Safer key deletion to avoid unexpected behavior

[PATCH 0/5] ath9k: Safer key deletion to avoid unexpected behavior

[Jouni Malinen]

While earlier mac80211 commits like a0761a301746 ("mac80211: drop data
frames without key on encrypted links"), ce2e1ca70307 ("mac80211: Check
port authorization in the ieee80211_tx_dequeue() case"), and
b16798f5b907 ("mac80211: mark station unauthorized before key removal")
addressed the most visible and easiest to trigger cases where pending
frames from TX queues could potentially end up getting transmitted
without proper encryption, some similar cases could still remain in the
hardware TX queue. In such cases, clearing of the hardware key cache
entry could happen while there were remaining frames in the TX queues
with reference to the deleted key. That could end up transmitting such
frames without encryption. These issues are related to CVE-2020-3702.

It is not clear whether this can be triggered in the station mode after
the mac80211 changes, but it has been possible to come up with a test
setup where this could be triggered in the AP mode where the TX queue is
shared with multiple destinations and cannot be flushed when a single
station disconnects. Address such cases by not clearing the hardware key
cache entry immediately, but instead, temporarily leaving the key in
place and disabling RX processing for the key cache entry. Such a
postponed key deletion is completed once there are no remaining frames
referencing the particular key cache entry. This is done before handling
the next key cache operation to avoid having to perform heavy operations
during normal TX operations.

Jouni Malinen (5):
  ath: Use safer key clearing with key cache entries
  ath9k: Clear key cache explicitly on disabling hardware
  ath: Export ath_hw_keysetmac()
  ath: Modify ath_key_delete() to not need full key entry
  ath9k: Postpone key cache entry deletion for TXQ frames reference it

 drivers/net/wireless/ath/ath.h                |  3 +-
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_main.c |  2 +-
 drivers/net/wireless/ath/ath9k/hw.h           |  1 +
 drivers/net/wireless/ath/ath9k/main.c         | 95 ++++++++++++++++++-
 drivers/net/wireless/ath/key.c                | 41 ++++----
 6 files changed, 122 insertions(+), 22 deletions(-)

-- 
2.20.1

[PATCH 1/5] ath: Use safer key clearing with key cache entries

[Jouni Malinen]

It is possible for there to be pending frames in TXQs with a reference
to the key cache entry that is being deleted. If such a key cache entry
is cleared, those pending frame in TXQ might get transmitted without
proper encryption. It is safer to leave the previously used key into the
key cache in such cases. Instead, only clear the MAC address to prevent
RX processing from using this key cache entry.

This is needed in particularly in AP mode where the TXQs cannot be
flushed on station disconnection. This change alone may not be able to
address all cases where the key cache entry might get reused for other
purposes immediately (the key cache entry should be released for reuse
only once the TXQs do not have any remaining references to them), but
this makes it less likely to get unprotected frames and the more
complete changes may end up being significantly more complex.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/key.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 1816b4e7dc26..59618bb41f6c 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -583,7 +583,16 @@ EXPORT_SYMBOL(ath_key_config);
  */
 void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key)
 {
-	ath_hw_keyreset(common, key->hw_key_idx);
+	/* Leave CCMP and TKIP (main key) configured to avoid disabling
+	 * encryption for potentially pending frames already in a TXQ with the
+	 * keyix pointing to this key entry. Instead, only clear the MAC address
+	 * to prevent RX processing from using this key cache entry.
+	 */
+	if (test_bit(key->hw_key_idx, common->ccmp_keymap) ||
+	    test_bit(key->hw_key_idx, common->tkip_keymap))
+		ath_hw_keysetmac(common, key->hw_key_idx, NULL);
+	else
+		ath_hw_keyreset(common, key->hw_key_idx);
 	if (key->hw_key_idx < IEEE80211_WEP_NKID)
 		return;
 
-- 
2.20.1

Re: [PATCH 1/5] ath: Use safer key clearing with key cache entries

[Kalle Valo]

Jouni Malinen <jouni@codeaurora.org> wrote:

> It is possible for there to be pending frames in TXQs with a reference
> to the key cache entry that is being deleted. If such a key cache entry
> is cleared, those pending frame in TXQ might get transmitted without
> proper encryption. It is safer to leave the previously used key into the
> key cache in such cases. Instead, only clear the MAC address to prevent
> RX processing from using this key cache entry.
> 
> This is needed in particularly in AP mode where the TXQs cannot be
> flushed on station disconnection. This change alone may not be able to
> address all cases where the key cache entry might get reused for other
> purposes immediately (the key cache entry should be released for reuse
> only once the TXQs do not have any remaining references to them), but
> this makes it less likely to get unprotected frames and the more
> complete changes may end up being significantly more complex.
> 
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

5 patches applied to ath-next branch of ath.git, thanks.

56c5485c9e44 ath: Use safer key clearing with key cache entries
73488cb2fa3b ath9k: Clear key cache explicitly on disabling hardware
d2d3e36498dd ath: Export ath_hw_keysetmac()
144cd24dbc36 ath: Modify ath_key_delete() to not need full key entry
ca2848022c12 ath9k: Postpone key cache entry deletion for TXQ frames reference it

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20201214172118.18100-2-jouni@codeaurora.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 1/5] ath: Use safer key clearing with key cache entries

[Pali Rohár]

On Thursday 17 December 2020 06:51:48 Kalle Valo wrote:
> Jouni Malinen <jouni@codeaurora.org> wrote:
> 
> > It is possible for there to be pending frames in TXQs with a reference
> > to the key cache entry that is being deleted. If such a key cache entry
> > is cleared, those pending frame in TXQ might get transmitted without
> > proper encryption. It is safer to leave the previously used key into the
> > key cache in such cases. Instead, only clear the MAC address to prevent
> > RX processing from using this key cache entry.
> > 
> > This is needed in particularly in AP mode where the TXQs cannot be
> > flushed on station disconnection. This change alone may not be able to
> > address all cases where the key cache entry might get reused for other
> > purposes immediately (the key cache entry should be released for reuse
> > only once the TXQs do not have any remaining references to them), but
> > this makes it less likely to get unprotected frames and the more
> > complete changes may end up being significantly more complex.
> > 
> > Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> 
> 5 patches applied to ath-next branch of ath.git, thanks.
> 
> 56c5485c9e44 ath: Use safer key clearing with key cache entries
> 73488cb2fa3b ath9k: Clear key cache explicitly on disabling hardware
> d2d3e36498dd ath: Export ath_hw_keysetmac()
> 144cd24dbc36 ath: Modify ath_key_delete() to not need full key entry
> ca2848022c12 ath9k: Postpone key cache entry deletion for TXQ frames reference it

Hello! Should not these patches be suitable for backporting into stable
kernels (via CC: stable@ commit message line) as they are related to
security issue CVE-2020-3702?

Re: [PATCH 1/5] ath: Use safer key clearing with key cache entries

[Kalle Valo]

Pali Rohár <pali@kernel.org> writes:

> On Thursday 17 December 2020 06:51:48 Kalle Valo wrote:
>> Jouni Malinen <jouni@codeaurora.org> wrote:
>> 
>> > It is possible for there to be pending frames in TXQs with a reference
>> > to the key cache entry that is being deleted. If such a key cache entry
>> > is cleared, those pending frame in TXQ might get transmitted without
>> > proper encryption. It is safer to leave the previously used key into the
>> > key cache in such cases. Instead, only clear the MAC address to prevent
>> > RX processing from using this key cache entry.
>> > 
>> > This is needed in particularly in AP mode where the TXQs cannot be
>> > flushed on station disconnection. This change alone may not be able to
>> > address all cases where the key cache entry might get reused for other
>> > purposes immediately (the key cache entry should be released for reuse
>> > only once the TXQs do not have any remaining references to them), but
>> > this makes it less likely to get unprotected frames and the more
>> > complete changes may end up being significantly more complex.
>> > 
>> > Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
>> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>> 
>> 5 patches applied to ath-next branch of ath.git, thanks.
>> 
>> 56c5485c9e44 ath: Use safer key clearing with key cache entries
>> 73488cb2fa3b ath9k: Clear key cache explicitly on disabling hardware
>> d2d3e36498dd ath: Export ath_hw_keysetmac()
>> 144cd24dbc36 ath: Modify ath_key_delete() to not need full key entry
>> ca2848022c12 ath9k: Postpone key cache entry deletion for TXQ frames reference it
>
> Hello! Should not these patches be suitable for backporting into stable
> kernels (via CC: stable@ commit message line) as they are related to
> security issue CVE-2020-3702?

Yeah, but you were just a little late as I already applied them.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 1/5] ath: Use safer key clearing with key cache entries

[Pali Rohár]

On Thursday 17 December 2020 18:06:27 Kalle Valo wrote:
> Pali Rohár <pali@kernel.org> writes:
> 
> > On Thursday 17 December 2020 06:51:48 Kalle Valo wrote:
> >> Jouni Malinen <jouni@codeaurora.org> wrote:
> >> 
> >> > It is possible for there to be pending frames in TXQs with a reference
> >> > to the key cache entry that is being deleted. If such a key cache entry
> >> > is cleared, those pending frame in TXQ might get transmitted without
> >> > proper encryption. It is safer to leave the previously used key into the
> >> > key cache in such cases. Instead, only clear the MAC address to prevent
> >> > RX processing from using this key cache entry.
> >> > 
> >> > This is needed in particularly in AP mode where the TXQs cannot be
> >> > flushed on station disconnection. This change alone may not be able to
> >> > address all cases where the key cache entry might get reused for other
> >> > purposes immediately (the key cache entry should be released for reuse
> >> > only once the TXQs do not have any remaining references to them), but
> >> > this makes it less likely to get unprotected frames and the more
> >> > complete changes may end up being significantly more complex.
> >> > 
> >> > Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> >> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
> >> 
> >> 5 patches applied to ath-next branch of ath.git, thanks.
> >> 
> >> 56c5485c9e44 ath: Use safer key clearing with key cache entries
> >> 73488cb2fa3b ath9k: Clear key cache explicitly on disabling hardware
> >> d2d3e36498dd ath: Export ath_hw_keysetmac()
> >> 144cd24dbc36 ath: Modify ath_key_delete() to not need full key entry
> >> ca2848022c12 ath9k: Postpone key cache entry deletion for TXQ frames reference it
> >
> > Hello! Should not these patches be suitable for backporting into stable
> > kernels (via CC: stable@ commit message line) as they are related to
> > security issue CVE-2020-3702?
> 
> Yeah, but you were just a little late as I already applied them.

Ok, would you then send these patches to stable manually?

> -- 
> https://patchwork.kernel.org/project/linux-wireless/list/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 1/5] ath: Use safer key clearing with key cache entries

[Kalle Valo]

Pali Rohár <pali@kernel.org> writes:

> On Thursday 17 December 2020 18:06:27 Kalle Valo wrote:
>> Pali Rohár <pali@kernel.org> writes:
>> 
>> > On Thursday 17 December 2020 06:51:48 Kalle Valo wrote:
>> >> Jouni Malinen <jouni@codeaurora.org> wrote:
>> >> 
>> >> > It is possible for there to be pending frames in TXQs with a reference
>> >> > to the key cache entry that is being deleted. If such a key cache entry
>> >> > is cleared, those pending frame in TXQ might get transmitted without
>> >> > proper encryption. It is safer to leave the previously used key into the
>> >> > key cache in such cases. Instead, only clear the MAC address to prevent
>> >> > RX processing from using this key cache entry.
>> >> > 
>> >> > This is needed in particularly in AP mode where the TXQs cannot be
>> >> > flushed on station disconnection. This change alone may not be able to
>> >> > address all cases where the key cache entry might get reused for other
>> >> > purposes immediately (the key cache entry should be released for reuse
>> >> > only once the TXQs do not have any remaining references to them), but
>> >> > this makes it less likely to get unprotected frames and the more
>> >> > complete changes may end up being significantly more complex.
>> >> > 
>> >> > Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
>> >> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
>> >> 
>> >> 5 patches applied to ath-next branch of ath.git, thanks.
>> >> 
>> >> 56c5485c9e44 ath: Use safer key clearing with key cache entries
>> >> 73488cb2fa3b ath9k: Clear key cache explicitly on disabling hardware
>> >> d2d3e36498dd ath: Export ath_hw_keysetmac()
>> >> 144cd24dbc36 ath: Modify ath_key_delete() to not need full key entry
>> >> ca2848022c12 ath9k: Postpone key cache entry deletion for TXQ frames reference it
>> >
>> > Hello! Should not these patches be suitable for backporting into stable
>> > kernels (via CC: stable@ commit message line) as they are related to
>> > security issue CVE-2020-3702?
>> 
>> Yeah, but you were just a little late as I already applied them.
>
> Ok, would you then send these patches to stable manually?

Sorry, I have too many patches in queue to do that. But I don't think I
need to submit them, my understanding is that anyone can submit patches
to stable.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/5] ath9k: Clear key cache explicitly on disabling hardware

[Jouni Malinen]

Now that ath/key.c may not be explicitly clearing keys from the key
cache, clear all key cache entries when disabling hardware to make sure
no keys are left behind beyond this point.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath9k/main.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index caebe3fd6869..10b87aa1d289 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -894,6 +894,11 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 
 	spin_unlock_bh(&sc->sc_pcu_lock);
 
+	/* Clear key cache entries explicitly to get rid of any potentially
+	 * remaining keys.
+	 */
+	ath9k_cmn_init_crypto(sc->sc_ah);
+
 	ath9k_ps_restore(sc);
 
 	sc->ps_idle = prev_idle;
-- 
2.20.1

[PATCH 3/5] ath: Export ath_hw_keysetmac()

[Jouni Malinen]

ath9k is going to use this for safer management of key cache entries.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath.h | 1 +
 drivers/net/wireless/ath/key.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 7a364eca46d6..9d18105c449f 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -203,6 +203,7 @@ int ath_key_config(struct ath_common *common,
 			  struct ieee80211_sta *sta,
 			  struct ieee80211_key_conf *key);
 bool ath_hw_keyreset(struct ath_common *common, u16 entry);
+bool ath_hw_keysetmac(struct ath_common *common, u16 entry, const u8 *mac);
 void ath_hw_cycle_counters_update(struct ath_common *common);
 int32_t ath_hw_get_listen_time(struct ath_common *common);
 
diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index 59618bb41f6c..cb266cf3c77c 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -84,8 +84,7 @@ bool ath_hw_keyreset(struct ath_common *common, u16 entry)
 }
 EXPORT_SYMBOL(ath_hw_keyreset);
 
-static bool ath_hw_keysetmac(struct ath_common *common,
-			     u16 entry, const u8 *mac)
+bool ath_hw_keysetmac(struct ath_common *common, u16 entry, const u8 *mac)
 {
 	u32 macHi, macLo;
 	u32 unicast_flag = AR_KEYTABLE_VALID;
@@ -125,6 +124,7 @@ static bool ath_hw_keysetmac(struct ath_common *common,
 
 	return true;
 }
+EXPORT_SYMBOL(ath_hw_keysetmac);
 
 static bool ath_hw_set_keycache_entry(struct ath_common *common, u16 entry,
 				      const struct ath_keyval *k,
-- 
2.20.1

[PATCH 4/5] ath: Modify ath_key_delete() to not need full key entry

[Jouni Malinen]

tkip_keymap can be used internally to avoid the reference to key->cipher
and with this, only the key index value itself is needed. This allows
ath_key_delete() call to be postponed to be handled after the upper
layer STA and key entry have already been removed. This is needed to
make ath9k key cache management safer.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath.h                |  2 +-
 drivers/net/wireless/ath/ath5k/mac80211-ops.c |  2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_main.c |  2 +-
 drivers/net/wireless/ath/ath9k/main.c         |  5 ++-
 drivers/net/wireless/ath/key.c                | 34 +++++++++----------
 5 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath.h b/drivers/net/wireless/ath/ath.h
index 9d18105c449f..f083fb9038c3 100644
--- a/drivers/net/wireless/ath/ath.h
+++ b/drivers/net/wireless/ath/ath.h
@@ -197,7 +197,7 @@ struct sk_buff *ath_rxbuf_alloc(struct ath_common *common,
 bool ath_is_mybeacon(struct ath_common *common, struct ieee80211_hdr *hdr);
 
 void ath_hw_setbssidmask(struct ath_common *common);
-void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key);
+void ath_key_delete(struct ath_common *common, u8 hw_key_idx);
 int ath_key_config(struct ath_common *common,
 			  struct ieee80211_vif *vif,
 			  struct ieee80211_sta *sta,
diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index 8f2719ff463c..532eeac9e83e 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -522,7 +522,7 @@ ath5k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		}
 		break;
 	case DISABLE_KEY:
-		ath_key_delete(common, key);
+		ath_key_delete(common, key->hw_key_idx);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_main.c b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
index 2b7832b1c800..72ef319feeda 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_main.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_main.c
@@ -1461,7 +1461,7 @@ static int ath9k_htc_set_key(struct ieee80211_hw *hw,
 		}
 		break;
 	case DISABLE_KEY:
-		ath_key_delete(common, key);
+		ath_key_delete(common, key->hw_key_idx);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 10b87aa1d289..bcdf150060f2 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1543,12 +1543,11 @@ static void ath9k_del_ps_key(struct ath_softc *sc,
 {
 	struct ath_common *common = ath9k_hw_common(sc->sc_ah);
 	struct ath_node *an = (struct ath_node *) sta->drv_priv;
-	struct ieee80211_key_conf ps_key = { .hw_key_idx = an->ps_key };
 
 	if (!an->ps_key)
 	    return;
 
-	ath_key_delete(common, &ps_key);
+	ath_key_delete(common, an->ps_key);
 	an->ps_key = 0;
 	an->key_idx[0] = 0;
 }
@@ -1748,7 +1747,7 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 		}
 		break;
 	case DISABLE_KEY:
-		ath_key_delete(common, key);
+		ath_key_delete(common, key->hw_key_idx);
 		if (an) {
 			for (i = 0; i < ARRAY_SIZE(an->key_idx); i++) {
 				if (an->key_idx[i] != key->hw_key_idx)
diff --git a/drivers/net/wireless/ath/key.c b/drivers/net/wireless/ath/key.c
index cb266cf3c77c..61b59a804e30 100644
--- a/drivers/net/wireless/ath/key.c
+++ b/drivers/net/wireless/ath/key.c
@@ -581,38 +581,38 @@ EXPORT_SYMBOL(ath_key_config);
 /*
  * Delete Key.
  */
-void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key)
+void ath_key_delete(struct ath_common *common, u8 hw_key_idx)
 {
 	/* Leave CCMP and TKIP (main key) configured to avoid disabling
 	 * encryption for potentially pending frames already in a TXQ with the
 	 * keyix pointing to this key entry. Instead, only clear the MAC address
 	 * to prevent RX processing from using this key cache entry.
 	 */
-	if (test_bit(key->hw_key_idx, common->ccmp_keymap) ||
-	    test_bit(key->hw_key_idx, common->tkip_keymap))
-		ath_hw_keysetmac(common, key->hw_key_idx, NULL);
+	if (test_bit(hw_key_idx, common->ccmp_keymap) ||
+	    test_bit(hw_key_idx, common->tkip_keymap))
+		ath_hw_keysetmac(common, hw_key_idx, NULL);
 	else
-		ath_hw_keyreset(common, key->hw_key_idx);
-	if (key->hw_key_idx < IEEE80211_WEP_NKID)
+		ath_hw_keyreset(common, hw_key_idx);
+	if (hw_key_idx < IEEE80211_WEP_NKID)
 		return;
 
-	clear_bit(key->hw_key_idx, common->keymap);
-	clear_bit(key->hw_key_idx, common->ccmp_keymap);
-	if (key->cipher != WLAN_CIPHER_SUITE_TKIP)
+	clear_bit(hw_key_idx, common->keymap);
+	clear_bit(hw_key_idx, common->ccmp_keymap);
+	if (!test_bit(hw_key_idx, common->tkip_keymap))
 		return;
 
-	clear_bit(key->hw_key_idx + 64, common->keymap);
+	clear_bit(hw_key_idx + 64, common->keymap);
 
-	clear_bit(key->hw_key_idx, common->tkip_keymap);
-	clear_bit(key->hw_key_idx + 64, common->tkip_keymap);
+	clear_bit(hw_key_idx, common->tkip_keymap);
+	clear_bit(hw_key_idx + 64, common->tkip_keymap);
 
 	if (!(common->crypt_caps & ATH_CRYPT_CAP_MIC_COMBINED)) {
-		ath_hw_keyreset(common, key->hw_key_idx + 32);
-		clear_bit(key->hw_key_idx + 32, common->keymap);
-		clear_bit(key->hw_key_idx + 64 + 32, common->keymap);
+		ath_hw_keyreset(common, hw_key_idx + 32);
+		clear_bit(hw_key_idx + 32, common->keymap);
+		clear_bit(hw_key_idx + 64 + 32, common->keymap);
 
-		clear_bit(key->hw_key_idx + 32, common->tkip_keymap);
-		clear_bit(key->hw_key_idx + 64 + 32, common->tkip_keymap);
+		clear_bit(hw_key_idx + 32, common->tkip_keymap);
+		clear_bit(hw_key_idx + 64 + 32, common->tkip_keymap);
 	}
 }
 EXPORT_SYMBOL(ath_key_delete);
-- 
2.20.1

[PATCH 5/5] ath9k: Postpone key cache entry deletion for TXQ frames reference it

[Jouni Malinen]

Do not delete a key cache entry that is still being referenced by
pending frames in TXQs. This avoids reuse of the key cache entry while a
frame might still be transmitted using it.

To avoid having to do any additional operations during the main TX path
operations, track pending key cache entries in a new bitmap and check
whether any pending entries can be deleted before every new key
add/remove operation. Also clear any remaining entries when stopping the
interface.

Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
---
 drivers/net/wireless/ath/ath9k/hw.h   |  1 +
 drivers/net/wireless/ath/ath9k/main.c | 87 ++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 023599e10dd5..b7b65b1c90e8 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -820,6 +820,7 @@ struct ath_hw {
 	struct ath9k_pacal_info pacal_info;
 	struct ar5416Stats stats;
 	struct ath9k_tx_queue_info txq[ATH9K_NUM_TX_QUEUES];
+	DECLARE_BITMAP(pending_del_keymap, ATH_KEYMAX);
 
 	enum ath9k_int imask;
 	u32 imrs2_reg;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index bcdf150060f2..45f6402478b5 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -821,12 +821,80 @@ static void ath9k_tx(struct ieee80211_hw *hw,
 	ieee80211_free_txskb(hw, skb);
 }
 
+static bool ath9k_txq_list_has_key(struct list_head *txq_list, u32 keyix)
+{
+	struct ath_buf *bf;
+	struct ieee80211_tx_info *txinfo;
+	struct ath_frame_info *fi;
+
+	list_for_each_entry(bf, txq_list, list) {
+		if (bf->bf_state.stale || !bf->bf_mpdu)
+			continue;
+
+		txinfo = IEEE80211_SKB_CB(bf->bf_mpdu);
+		fi = (struct ath_frame_info *)&txinfo->rate_driver_data[0];
+		if (fi->keyix == keyix)
+			return true;
+	}
+
+	return false;
+}
+
+static bool ath9k_txq_has_key(struct ath_softc *sc, u32 keyix)
+{
+	struct ath_hw *ah = sc->sc_ah;
+	int i;
+	struct ath_txq *txq;
+	bool key_in_use = false;
+
+	for (i = 0; !key_in_use && i < ATH9K_NUM_TX_QUEUES; i++) {
+		if (!ATH_TXQ_SETUP(sc, i))
+			continue;
+		txq = &sc->tx.txq[i];
+		if (!txq->axq_depth)
+			continue;
+		if (!ath9k_hw_numtxpending(ah, txq->axq_qnum))
+			continue;
+
+		ath_txq_lock(sc, txq);
+		key_in_use = ath9k_txq_list_has_key(&txq->axq_q, keyix);
+		if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA) {
+			int idx = txq->txq_tailidx;
+
+			while (!key_in_use &&
+			       !list_empty(&txq->txq_fifo[idx])) {
+				key_in_use = ath9k_txq_list_has_key(
+					&txq->txq_fifo[idx], keyix);
+				INCR(idx, ATH_TXFIFO_DEPTH);
+			}
+		}
+		ath_txq_unlock(sc, txq);
+	}
+
+	return key_in_use;
+}
+
+static void ath9k_pending_key_del(struct ath_softc *sc, u8 keyix)
+{
+	struct ath_hw *ah = sc->sc_ah;
+	struct ath_common *common = ath9k_hw_common(ah);
+
+	if (!test_bit(keyix, ah->pending_del_keymap) ||
+	    ath9k_txq_has_key(sc, keyix))
+		return;
+
+	/* No more TXQ frames point to this key cache entry, so delete it. */
+	clear_bit(keyix, ah->pending_del_keymap);
+	ath_key_delete(common, keyix);
+}
+
 static void ath9k_stop(struct ieee80211_hw *hw)
 {
 	struct ath_softc *sc = hw->priv;
 	struct ath_hw *ah = sc->sc_ah;
 	struct ath_common *common = ath9k_hw_common(ah);
 	bool prev_idle;
+	int i;
 
 	ath9k_deinit_channel_context(sc);
 
@@ -894,6 +962,9 @@ static void ath9k_stop(struct ieee80211_hw *hw)
 
 	spin_unlock_bh(&sc->sc_pcu_lock);
 
+	for (i = 0; i < ATH_KEYMAX; i++)
+		ath9k_pending_key_del(sc, i);
+
 	/* Clear key cache entries explicitly to get rid of any potentially
 	 * remaining keys.
 	 */
@@ -1718,6 +1789,12 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 	if (sta)
 		an = (struct ath_node *)sta->drv_priv;
 
+	/* Delete pending key cache entries if no more frames are pointing to
+	 * them in TXQs.
+	 */
+	for (i = 0; i < ATH_KEYMAX; i++)
+		ath9k_pending_key_del(sc, i);
+
 	switch (cmd) {
 	case SET_KEY:
 		if (sta)
@@ -1747,7 +1824,15 @@ static int ath9k_set_key(struct ieee80211_hw *hw,
 		}
 		break;
 	case DISABLE_KEY:
-		ath_key_delete(common, key->hw_key_idx);
+		if (ath9k_txq_has_key(sc, key->hw_key_idx)) {
+			/* Delay key cache entry deletion until there are no
+			 * remaining TXQ frames pointing to this entry.
+			 */
+			set_bit(key->hw_key_idx, sc->sc_ah->pending_del_keymap);
+			ath_hw_keysetmac(common, key->hw_key_idx, NULL);
+		} else {
+			ath_key_delete(common, key->hw_key_idx);
+		}
 		if (an) {
 			for (i = 0; i < ARRAY_SIZE(an->key_idx); i++) {
 				if (an->key_idx[i] != key->hw_key_idx)
-- 
2.20.1