2015-03-27 08:24:43

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 1/2] ath10k: fix arvif->wep_keys clearing

The variable should be cleared regardless of
whether there's a peer associated with the key or
not.

This fixes case when user first associates with 2
WEP keys and then disconnects and connects with 1
WEP key. This resulted in WEP key count being 2 in
the driver leading to default keyidx fixup
failure.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9b8313dcb888..9d873900257d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4101,6 +4101,13 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,

key->hw_key_idx = key->keyidx;

+ if (is_wep) {
+ if (cmd == SET_KEY)
+ arvif->wep_keys[key->keyidx] = key;
+ else
+ arvif->wep_keys[key->keyidx] = NULL;
+ }
+
/* the peer should not disappear in mid-way (unless FW goes awry) since
* we already hold conf_mutex. we just make sure its there now. */
spin_lock_bh(&ar->data_lock);
@@ -4126,11 +4133,6 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
flags |= WMI_KEY_GROUP;

if (is_wep) {
- if (cmd == SET_KEY)
- arvif->wep_keys[key->keyidx] = key;
- else
- arvif->wep_keys[key->keyidx] = NULL;
-
if (cmd == DISABLE_KEY)
ath10k_clear_vdev_key(arvif, key);

--
2.1.4



2015-03-27 08:49:18

by Bartosz Markowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath10k: fix arvif->wep_keys clearing

On 27 March 2015 at 09:21, Michal Kazior <[email protected]> wrote:
> The variable should be cleared regardless of
> whether there's a peer associated with the key or
> not.
>
> This fixes case when user first associates with 2
> WEP keys and then disconnects and connects with 1
> WEP key. This resulted in WEP key count being 2 in
> the driver leading to default keyidx fixup
> failure.
>
> Signed-off-by: Michal Kazior <[email protected]>

Tested-by: Bartosz Markowski <[email protected]>

> ---
> drivers/net/wireless/ath/ath10k/mac.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 9b8313dcb888..9d873900257d 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4101,6 +4101,13 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>
> key->hw_key_idx = key->keyidx;
>
> + if (is_wep) {
> + if (cmd == SET_KEY)
> + arvif->wep_keys[key->keyidx] = key;
> + else
> + arvif->wep_keys[key->keyidx] = NULL;
> + }
> +
> /* the peer should not disappear in mid-way (unless FW goes awry) since
> * we already hold conf_mutex. we just make sure its there now. */
> spin_lock_bh(&ar->data_lock);
> @@ -4126,11 +4133,6 @@ static int ath10k_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> flags |= WMI_KEY_GROUP;
>
> if (is_wep) {
> - if (cmd == SET_KEY)
> - arvif->wep_keys[key->keyidx] = key;
> - else
> - arvif->wep_keys[key->keyidx] = NULL;
> -
> if (cmd == DISABLE_KEY)
> ath10k_clear_vdev_key(arvif, key);
>
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Bartosz

2015-03-27 08:49:40

by Bartosz Markowski

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath10k: fix static wep with more than 1 key in client mode

On 27 March 2015 at 09:21, Michal Kazior <[email protected]> wrote:
> The default keyidx callback may be called after
> more than 1 key is installed. This led to only 1
> WEP key being reinstalled only. This caused Rxed
> traffic encrypted with other WEP keys to be
> dropped in client mode.
>
> Signed-off-by: Michal Kazior <[email protected]>

Tested-by: Bartosz Markowski <[email protected]>

> ---
> drivers/net/wireless/ath/ath10k/mac.c | 45 +++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 9d873900257d..3baea2b671dd 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -259,41 +259,44 @@ static int ath10k_clear_vdev_key(struct ath10k_vif *arvif,
> return first_errno;
> }
>
> -static int ath10k_mac_vif_sta_fix_wep_key(struct ath10k_vif *arvif)
> +static int ath10k_mac_vif_sta_fix_wep_key(struct ath10k_vif *arvif, int keyidx)
> {
> struct ath10k *ar = arvif->ar;
> enum nl80211_iftype iftype = arvif->vif->type;
> struct ieee80211_key_conf *key;
> - u32 flags = 0;
> - int num = 0;
> - int i;
> + u32 flags;
> int ret;
> + int i;
>
> lockdep_assert_held(&ar->conf_mutex);
>
> if (iftype != NL80211_IFTYPE_STATION)
> return 0;
>
> + if (keyidx < 0)
> + return 0;
> +
> for (i = 0; i < ARRAY_SIZE(arvif->wep_keys); i++) {
> - if (arvif->wep_keys[i]) {
> - key = arvif->wep_keys[i];
> - ++num;
> + if (!arvif->wep_keys[i])
> + continue;
> +
> + key = arvif->wep_keys[i];
> +
> + flags = 0;
> + flags |= WMI_KEY_PAIRWISE;
> +
> + if (key->keyidx == keyidx)
> + flags |= WMI_KEY_TX_USAGE;
> +
> + ret = ath10k_install_key(arvif, key, SET_KEY, arvif->bssid,
> + flags);
> + if (ret) {
> + ath10k_warn(ar, "failed to install key %i on vdev %i: %d\n",
> + key->keyidx, arvif->vdev_id, ret);
> + return ret;
> }
> }
>
> - if (num != 1)
> - return 0;
> -
> - flags |= WMI_KEY_PAIRWISE;
> - flags |= WMI_KEY_TX_USAGE;
> -
> - ret = ath10k_install_key(arvif, key, SET_KEY, arvif->bssid, flags);
> - if (ret) {
> - ath10k_warn(ar, "failed to install key %i on vdev %i: %d\n",
> - key->keyidx, arvif->vdev_id, ret);
> - return ret;
> - }
> -
> return 0;
> }
>
> @@ -4227,7 +4230,7 @@ static void ath10k_set_default_unicast_key(struct ieee80211_hw *hw,
>
> arvif->def_wep_key_idx = keyidx;
>
> - ret = ath10k_mac_vif_sta_fix_wep_key(arvif);
> + ret = ath10k_mac_vif_sta_fix_wep_key(arvif, keyidx);
> if (ret) {
> ath10k_warn(ar, "failed to fix sta wep key on vdev %i: %d\n",
> arvif->vdev_id, ret);
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Bartosz

2015-03-27 08:24:45

by Michal Kazior

[permalink] [raw]
Subject: [PATCH 2/2] ath10k: fix static wep with more than 1 key in client mode

The default keyidx callback may be called after
more than 1 key is installed. This led to only 1
WEP key being reinstalled only. This caused Rxed
traffic encrypted with other WEP keys to be
dropped in client mode.

Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 45 +++++++++++++++++++----------------
1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 9d873900257d..3baea2b671dd 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -259,41 +259,44 @@ static int ath10k_clear_vdev_key(struct ath10k_vif *arvif,
return first_errno;
}

-static int ath10k_mac_vif_sta_fix_wep_key(struct ath10k_vif *arvif)
+static int ath10k_mac_vif_sta_fix_wep_key(struct ath10k_vif *arvif, int keyidx)
{
struct ath10k *ar = arvif->ar;
enum nl80211_iftype iftype = arvif->vif->type;
struct ieee80211_key_conf *key;
- u32 flags = 0;
- int num = 0;
- int i;
+ u32 flags;
int ret;
+ int i;

lockdep_assert_held(&ar->conf_mutex);

if (iftype != NL80211_IFTYPE_STATION)
return 0;

+ if (keyidx < 0)
+ return 0;
+
for (i = 0; i < ARRAY_SIZE(arvif->wep_keys); i++) {
- if (arvif->wep_keys[i]) {
- key = arvif->wep_keys[i];
- ++num;
+ if (!arvif->wep_keys[i])
+ continue;
+
+ key = arvif->wep_keys[i];
+
+ flags = 0;
+ flags |= WMI_KEY_PAIRWISE;
+
+ if (key->keyidx == keyidx)
+ flags |= WMI_KEY_TX_USAGE;
+
+ ret = ath10k_install_key(arvif, key, SET_KEY, arvif->bssid,
+ flags);
+ if (ret) {
+ ath10k_warn(ar, "failed to install key %i on vdev %i: %d\n",
+ key->keyidx, arvif->vdev_id, ret);
+ return ret;
}
}

- if (num != 1)
- return 0;
-
- flags |= WMI_KEY_PAIRWISE;
- flags |= WMI_KEY_TX_USAGE;
-
- ret = ath10k_install_key(arvif, key, SET_KEY, arvif->bssid, flags);
- if (ret) {
- ath10k_warn(ar, "failed to install key %i on vdev %i: %d\n",
- key->keyidx, arvif->vdev_id, ret);
- return ret;
- }
-
return 0;
}

@@ -4227,7 +4230,7 @@ static void ath10k_set_default_unicast_key(struct ieee80211_hw *hw,

arvif->def_wep_key_idx = keyidx;

- ret = ath10k_mac_vif_sta_fix_wep_key(arvif);
+ ret = ath10k_mac_vif_sta_fix_wep_key(arvif, keyidx);
if (ret) {
ath10k_warn(ar, "failed to fix sta wep key on vdev %i: %d\n",
arvif->vdev_id, ret);
--
2.1.4


2015-04-09 12:05:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 1/2] ath10k: fix arvif->wep_keys clearing

Michal Kazior <[email protected]> writes:

> The variable should be cleared regardless of
> whether there's a peer associated with the key or
> not.
>
> This fixes case when user first associates with 2
> WEP keys and then disconnects and connects with 1
> WEP key. This resulted in WEP key count being 2 in
> the driver leading to default keyidx fixup
> failure.
>
> Signed-off-by: Michal Kazior <[email protected]>

Thanks, both patches applied.

--
Kalle Valo