[PATCH] rt61pci: Work around a firmware bug with shared keys

* [PATCH] rt61pci: Work around a firmware bug with shared keys
@ 2019-01-15 14:01 Bernd Edlinger
  2019-01-16 11:21 ` Stanislaw Gruszka
  2019-02-01 12:16 ` [PATCH] rt2x00: " Kalle Valo
  0 siblings, 2 replies; 4+ messages in thread
From: Bernd Edlinger @ 2019-01-15 14:01 UTC (permalink / raw)
  To: Stanislaw Gruszka, Helmut Schaa, Kalle Valo, David S. Miller,
	linux-wireless, netdev, linux-kernel

Apparently the rt2x61 firmware fails temporarily to decode
broadcast packets if the shared keys are not assigned
in the "correct" sequence. At the same time unicast
packets work fine, since they are encrypted with the
pairwise key.

At least with WPA2 CCMP mode the shared keys are
set in the following sequence: keyidx=1, 2, 1, 2.
After a while only keyidx 2 gets decrypted, and
keyidx 1 is ignored, probably because there is never
a keyidx 3.

Symptoms are arping -b works for 10 minutes, since
keyidx=2 is used for broadcast, and then it stops
working for 10 minutes, because keyidx=1 is used.
That failure mode repeats forever.

Note, the firmware does not even know which keyidx
corresponds to which hw_key_idx so the firmware is
trying to be smarter than the driver, which is bound
to fail.

As workaround the function rt61pci_config_shared_key
requests software decryption of the shared keys,
by returning EOPNOTSUPP. However, pairwise keys are
still handled by hardware which works just fine.

Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
 drivers/net/wireless/ralink/rt2x00/rt61pci.c | 93 ++--------------------------
 1 file changed, 4 insertions(+), 89 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt61pci.c b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
index 4c5de8f..52b9fc4 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt61pci.c
@@ -321,97 +321,12 @@ static int rt61pci_config_shared_key(struct rt2x00_dev *rt2x00dev,
 				     struct rt2x00lib_crypto *crypto,
 				     struct ieee80211_key_conf *key)
 {
-	struct hw_key_entry key_entry;
-	struct rt2x00_field32 field;
-	u32 mask;
-	u32 reg;
-
-	if (crypto->cmd == SET_KEY) {
-		/*
-		 * rt2x00lib can't determine the correct free
-		 * key_idx for shared keys. We have 1 register
-		 * with key valid bits. The goal is simple, read
-		 * the register, if that is full we have no slots
-		 * left.
-		 * Note that each BSS is allowed to have up to 4
-		 * shared keys, so put a mask over the allowed
-		 * entries.
-		 */
-		mask = (0xf << crypto->bssidx);
-
-		reg = rt2x00mmio_register_read(rt2x00dev, SEC_CSR0);
-		reg &= mask;
-
-		if (reg && reg == mask)
-			return -ENOSPC;
-
-		key->hw_key_idx += reg ? ffz(reg) : 0;
-
-		/*
-		 * Upload key to hardware
-		 */
-		memcpy(key_entry.key, crypto->key,
-		       sizeof(key_entry.key));
-		memcpy(key_entry.tx_mic, crypto->tx_mic,
-		       sizeof(key_entry.tx_mic));
-		memcpy(key_entry.rx_mic, crypto->rx_mic,
-		       sizeof(key_entry.rx_mic));
-
-		reg = SHARED_KEY_ENTRY(key->hw_key_idx);
-		rt2x00mmio_register_multiwrite(rt2x00dev, reg,
-					       &key_entry, sizeof(key_entry));
-
-		/*
-		 * The cipher types are stored over 2 registers.
-		 * bssidx 0 and 1 keys are stored in SEC_CSR1 and
-		 * bssidx 1 and 2 keys are stored in SEC_CSR5.
-		 * Using the correct defines correctly will cause overhead,
-		 * so just calculate the correct offset.
-		 */
-		if (key->hw_key_idx < 8) {
-			field.bit_offset = (3 * key->hw_key_idx);
-			field.bit_mask = 0x7 << field.bit_offset;
-
-			reg = rt2x00mmio_register_read(rt2x00dev, SEC_CSR1);
-			rt2x00_set_field32(&reg, field, crypto->cipher);
-			rt2x00mmio_register_write(rt2x00dev, SEC_CSR1, reg);
-		} else {
-			field.bit_offset = (3 * (key->hw_key_idx - 8));
-			field.bit_mask = 0x7 << field.bit_offset;
-
-			reg = rt2x00mmio_register_read(rt2x00dev, SEC_CSR5);
-			rt2x00_set_field32(&reg, field, crypto->cipher);
-			rt2x00mmio_register_write(rt2x00dev, SEC_CSR5, reg);
-		}
-
-		/*
-		 * The driver does not support the IV/EIV generation
-		 * in hardware. However it doesn't support the IV/EIV
-		 * inside the ieee80211 frame either, but requires it
-		 * to be provided separately for the descriptor.
-		 * rt2x00lib will cut the IV/EIV data out of all frames
-		 * given to us by mac80211, but we must tell mac80211
-		 * to generate the IV/EIV data.
-		 */
-		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
-	}
-
 	/*
-	 * SEC_CSR0 contains only single-bit fields to indicate
-	 * a particular key is valid. Because using the FIELD32()
-	 * defines directly will cause a lot of overhead, we use
-	 * a calculation to determine the correct bit directly.
+	 * Let the software handle the shared keys,
+	 * since the hardware decryption does not work reliably,
+	 * because the firmware does not know the key's keyidx.
 	 */
-	mask = 1 << key->hw_key_idx;
-
-	reg = rt2x00mmio_register_read(rt2x00dev, SEC_CSR0);
-	if (crypto->cmd == SET_KEY)
-		reg |= mask;
-	else if (crypto->cmd == DISABLE_KEY)
-		reg &= ~mask;
-	rt2x00mmio_register_write(rt2x00dev, SEC_CSR0, reg);
-
-	return 0;
+	return -EOPNOTSUPP;
 }
 
 static int rt61pci_config_pairwise_key(struct rt2x00_dev *rt2x00dev,
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread