diff mbox

[v2] ath10k: fix PMG by using AES-CMAC/IGTK software crypto

Message ID 1425994339-15864-1-git-send-email-bartosz.markowski@tieto.com (mailing list archive)
State Not Applicable
Delegated to: Kalle Valo
Headers show

Commit Message

Bartosz Markowski March 10, 2015, 1:32 p.m. UTC
While testing with older supplicant, .drv_set_key() was failing due to
higher than ath10k firmware could handle key_index (WMI_MAX_KEY_INDEX == 3).

--
wpa_driver_nl80211_set_key: ifindex=15 alg=4 addr=0x7f02b129fbe3 key_idx=4 set_tx=0 seq_len=6 key_len=16
    broadcast key
nl80211: set_key failed; err=-22 Invalid argument)
wlan0: WPA: Failed to configure IGTK to the driver
wlan0: RSN: Failed to configure IGTK
--

In order to fix this case (PMF: AES-CMAC/IGTK) force the AES_CMAC cipher to
be handled by software.

Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
---
 drivers/net/wireless/ath/ath10k/mac.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Ben Greear March 10, 2015, 4:02 p.m. UTC | #1
On 03/10/2015 06:32 AM, Bartosz Markowski wrote:
> While testing with older supplicant, .drv_set_key() was failing due to
> higher than ath10k firmware could handle key_index (WMI_MAX_KEY_INDEX == 3).
> 
> --
> wpa_driver_nl80211_set_key: ifindex=15 alg=4 addr=0x7f02b129fbe3 key_idx=4 set_tx=0 seq_len=6 key_len=16
>     broadcast key
> nl80211: set_key failed; err=-22 Invalid argument)
> wlan0: WPA: Failed to configure IGTK to the driver
> wlan0: RSN: Failed to configure IGTK
> --
> 
> In order to fix this case (PMF: AES-CMAC/IGTK) force the AES_CMAC cipher to
> be handled by software.

How did you get firmware to allow the host to do the encryption?

Every time I've tried such a thing I end up with either nothing or garbage on
the air.

Or this this particular type of encryption treated differently by the
firmware?

Thanks,
Ben

> 
> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
> ---
>  drivers/net/wireless/ath/ath10k/mac.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 1138f4f8923f..5a3129010951 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -66,8 +66,8 @@ static int ath10k_send_key(struct ath10k_vif *arvif,
>  		arg.key_cipher = WMI_CIPHER_WEP;
>  		break;
>  	case WLAN_CIPHER_SUITE_AES_CMAC:
> -		/* this one needs to be done in software */
> -		return 1;
> +		WARN_ON(1);
> +		return -EINVAL;
>  	default:
>  		ath10k_warn(ar, "cipher %d is not supported\n", key->cipher);
>  		return -EOPNOTSUPP;
> @@ -3995,6 +3995,10 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  	int ret = 0;
>  	u32 flags = 0;
>  
> +	/* this one needs to be done in software */
> +	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC)
> +		return 1;
> +
>  	if (key->keyidx > WMI_MAX_KEY_INDEX)
>  		return -ENOSPC;
>  
>
Bartosz Markowski March 11, 2015, 9:17 a.m. UTC | #2
On 10 March 2015 at 17:02, Ben Greear <greearb@candelatech.com> wrote:
> On 03/10/2015 06:32 AM, Bartosz Markowski wrote:
>> While testing with older supplicant, .drv_set_key() was failing due to
>> higher than ath10k firmware could handle key_index (WMI_MAX_KEY_INDEX == 3).
>>
>> --
>> wpa_driver_nl80211_set_key: ifindex=15 alg=4 addr=0x7f02b129fbe3 key_idx=4 set_tx=0 seq_len=6 key_len=16
>>     broadcast key
>> nl80211: set_key failed; err=-22 Invalid argument)
>> wlan0: WPA: Failed to configure IGTK to the driver
>> wlan0: RSN: Failed to configure IGTK
>> --
>>
>> In order to fix this case (PMF: AES-CMAC/IGTK) force the AES_CMAC cipher to
>> be handled by software.
>
> How did you get firmware to allow the host to do the encryption?
:
> Every time I've tried such a thing I end up with either nothing or garbage on
> the air.
>
> Or this this particular type of encryption treated differently by the
> firmware?

I would suspect so, but it's just my gut feeling. I did not check this
on a firmware level, as the PMF (AES_CMAC) encryption was done this
way from the very beggining in ath10k.
This patch only address the key_idx exceeded case, where
ath10k_set_key() was returning -ENOSPC, so we could inform mac80211
explicitly (by returning '1') it shall do the encryption in software.

-Bartosz
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Greear March 11, 2015, 4 p.m. UTC | #3
On 03/11/2015 02:17 AM, Bartosz Markowski wrote:
> On 10 March 2015 at 17:02, Ben Greear <greearb@candelatech.com> wrote:
>> On 03/10/2015 06:32 AM, Bartosz Markowski wrote:
>>> While testing with older supplicant, .drv_set_key() was failing due to
>>> higher than ath10k firmware could handle key_index (WMI_MAX_KEY_INDEX == 3).
>>>
>>> --
>>> wpa_driver_nl80211_set_key: ifindex=15 alg=4 addr=0x7f02b129fbe3 key_idx=4 set_tx=0 seq_len=6 key_len=16
>>>     broadcast key
>>> nl80211: set_key failed; err=-22 Invalid argument)
>>> wlan0: WPA: Failed to configure IGTK to the driver
>>> wlan0: RSN: Failed to configure IGTK
>>> --
>>>
>>> In order to fix this case (PMF: AES-CMAC/IGTK) force the AES_CMAC cipher to
>>> be handled by software.
>>
>> How did you get firmware to allow the host to do the encryption?
> :
>> Every time I've tried such a thing I end up with either nothing or garbage on
>> the air.
>>
>> Or this this particular type of encryption treated differently by the
>> firmware?
> 
> I would suspect so, but it's just my gut feeling. I did not check this
> on a firmware level, as the PMF (AES_CMAC) encryption was done this
> way from the very beggining in ath10k.
> This patch only address the key_idx exceeded case, where
> ath10k_set_key() was returning -ENOSPC, so we could inform mac80211
> explicitly (by returning '1') it shall do the encryption in software.

One of my very early patches when trying to use ath10k was to ask
the firmware for one extra key slot.  Maybe I hit the same problem
you did.  I'll have to try reverting my patch and try yours instead
some day.

Thanks,
Ben

> 
> -Bartosz
>
Kalle Valo March 12, 2015, 12:59 p.m. UTC | #4
Bartosz Markowski <bartosz.markowski@tieto.com> writes:

> While testing with older supplicant, .drv_set_key() was failing due to
> higher than ath10k firmware could handle key_index (WMI_MAX_KEY_INDEX == 3).
>
> --
> wpa_driver_nl80211_set_key: ifindex=15 alg=4 addr=0x7f02b129fbe3 key_idx=4 set_tx=0 seq_len=6 key_len=16
>     broadcast key
> nl80211: set_key failed; err=-22 Invalid argument)
> wlan0: WPA: Failed to configure IGTK to the driver
> wlan0: RSN: Failed to configure IGTK
> --
>
> In order to fix this case (PMF: AES-CMAC/IGTK) force the AES_CMAC cipher to
> be handled by software.
>
> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>

Thanks, applied. I just did s/PMG/PMF/.
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 1138f4f8923f..5a3129010951 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -66,8 +66,8 @@  static int ath10k_send_key(struct ath10k_vif *arvif,
 		arg.key_cipher = WMI_CIPHER_WEP;
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
-		/* this one needs to be done in software */
-		return 1;
+		WARN_ON(1);
+		return -EINVAL;
 	default:
 		ath10k_warn(ar, "cipher %d is not supported\n", key->cipher);
 		return -EOPNOTSUPP;
@@ -3995,6 +3995,10 @@  static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	int ret = 0;
 	u32 flags = 0;
 
+	/* this one needs to be done in software */
+	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC)
+		return 1;
+
 	if (key->keyidx > WMI_MAX_KEY_INDEX)
 		return -ENOSPC;