From 28916a165aa7e0ceddedabf722808881ba6a3b03 Mon Sep 17 00:00:00 2001 From: Emmanuel Grumbach Date: Mon, 3 Dec 2018 20:13:27 +0200 Subject: [PATCH] iwlwifi: mvm: fix AP mode in WEP Recently we started to send the WEP keys to the firmware so that we could use hardware Tx encryption also on newer devices that require the keys to be installed in the firmware for encryption (as opposed to older devices that can get the key in the Tx command for each Tx). When we implemented that, we forgot to remove the key when we remove a station leading to a situation where a station that connects and disconnects a lot of times exhausts the key database inside the firmware. A fix was made for that, but we always removed the same key: mvmvif->ap_wep_key which means that we removed the same key entry in the firmware. This can make sense since in WEP, the key is the same for all the stations, but the internal implementation of iwl_mvm_set_sta_key and iwl_mvm_remove_sta_key assumes that each station uses a different key in the firmware's key database. So now we got to the situation where we have a single ieee80211_key_conf instance that means, a single ieee80211_key_conf.hw_key_idx index for several stations and hence for several keys. ieee80211_key_conf.hw_key_idx is set to 0 when the first station associates, and then it is overwritten to 1 when the second station associates which is a buggy of course. This led to the following message upon the removal of the second station: iwlwifi 0000:00:03.0: offset 1 not used in fw key table. WARNING: CPU: 2 PID: 27883 at net/mac80211/sta_info.c:1122 __sta_info_destroy_part2+0x16b/0x180 [mac80211] RIP: 0010:__sta_info_destroy_part2+0x16b/0x180 [mac80211] Call Trace: __sta_info_destroy+0x2a/0x40 [mac80211] sta_info_destroy_addr_bss+0x38/0x60 [mac80211] ieee80211_del_station+0x1d/0x30 [mac80211] nl80211_del_station+0xe0/0x1f0 [cfg80211] Fix this by copying the ieee80211_key_conf structure for each and every station. This is the easiest way to properly remove the keys with the right index. Another solution would have been to allow several stations to use the same key offset in the firmware. That would require to change the way we track keys in iwlmvm and not really worth it. Also, maintain correctly fw_key_table when we add a key for the multicast station. Remove the key when we remove the multicast station. Signed-off-by: Emmanuel Grumbach Fixes: 337bfc9881a2 ("iwlwifi: mvm: set wep key for all stations in soft ap mode") Signed-off-by: Luca Coelho --- drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 30 +++-- drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 132 +++++++++++++--------- drivers/net/wireless/intel/iwlwifi/mvm/sta.h | 3 + 3 files changed, 102 insertions(+), 63 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c index 4e676a6..92605aa 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c @@ -3078,6 +3078,24 @@ static int iwl_mvm_mac_sta_state(struct ieee80211_hw *hw, ret = iwl_mvm_update_sta(mvm, vif, sta); } else if (old_state == IEEE80211_STA_ASSOC && new_state == IEEE80211_STA_AUTHORIZED) { + /* if wep is used, need to set the key for the station now */ + if (vif->type == NL80211_IFTYPE_AP && mvmvif->ap_wep_key) { + mvm_sta->wep_key = + kmemdup(mvmvif->ap_wep_key, + sizeof(*mvmvif->ap_wep_key) + + mvmvif->ap_wep_key->keylen, + GFP_KERNEL); + if (!mvm_sta->wep_key) { + ret = -ENOMEM; + goto out_unlock; + } + + ret = iwl_mvm_set_sta_key(mvm, vif, sta, + mvm_sta->wep_key, + STA_KEY_IDX_INVALID); + } else { + ret = 0; + } /* we don't support TDLS during DCM */ if (iwl_mvm_phy_ctx_count(mvm) > 1) @@ -3092,14 +3110,6 @@ static int iwl_mvm_mac_sta_state(struct ieee80211_hw *hw, iwl_mvm_rs_rate_init(mvm, sta, mvmvif->phy_ctxt->channel->band, true); - - /* if wep is used, need to set the key for the station now */ - if (vif->type == NL80211_IFTYPE_AP && mvmvif->ap_wep_key) - ret = iwl_mvm_set_sta_key(mvm, vif, sta, - mvmvif->ap_wep_key, - STA_KEY_IDX_INVALID); - else - ret = 0; } else if (old_state == IEEE80211_STA_AUTHORIZED && new_state == IEEE80211_STA_ASSOC) { /* disable beacon filtering */ @@ -3127,10 +3137,12 @@ static int iwl_mvm_mac_sta_state(struct ieee80211_hw *hw, /* Remove STA key if this is an AP using WEP */ if (vif->type == NL80211_IFTYPE_AP && mvmvif->ap_wep_key) { int rm_ret = iwl_mvm_remove_sta_key(mvm, vif, sta, - mvmvif->ap_wep_key); + mvm_sta->wep_key); if (!ret) ret = rm_ret; + kfree(mvm_sta->wep_key); + mvm_sta->wep_key = NULL; } } else { diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c index a102a16..3ea39bb 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c @@ -2337,11 +2337,13 @@ int iwl_mvm_add_mcast_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif) if (mvmvif->ap_wep_key) { u8 key_offset = iwl_mvm_set_fw_key_idx(mvm); + __set_bit(key_offset, mvm->fw_key_table); + if (key_offset == STA_KEY_IDX_INVALID) return -ENOSPC; ret = iwl_mvm_send_sta_key(mvm, mvmvif->mcast_sta.sta_id, - mvmvif->ap_wep_key, 1, 0, NULL, 0, + mvmvif->ap_wep_key, true, 0, NULL, 0, key_offset, 0); if (ret) return ret; @@ -2350,6 +2352,59 @@ int iwl_mvm_add_mcast_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif) return 0; } +static int __iwl_mvm_remove_sta_key(struct iwl_mvm *mvm, u8 sta_id, + struct ieee80211_key_conf *keyconf, + bool mcast) +{ + union { + struct iwl_mvm_add_sta_key_cmd_v1 cmd_v1; + struct iwl_mvm_add_sta_key_cmd cmd; + } u = {}; + bool new_api = fw_has_api(&mvm->fw->ucode_capa, + IWL_UCODE_TLV_API_TKIP_MIC_KEYS); + __le16 key_flags; + int ret, size; + u32 status; + + /* This is a valid situation for GTK removal */ + if (sta_id == IWL_MVM_INVALID_STA) + return 0; + + key_flags = cpu_to_le16((keyconf->keyidx << STA_KEY_FLG_KEYID_POS) & + STA_KEY_FLG_KEYID_MSK); + key_flags |= cpu_to_le16(STA_KEY_FLG_NO_ENC | STA_KEY_FLG_WEP_KEY_MAP); + key_flags |= cpu_to_le16(STA_KEY_NOT_VALID); + + if (mcast) + key_flags |= cpu_to_le16(STA_KEY_MULTICAST); + + /* + * The fields assigned here are in the same location at the start + * of the command, so we can do this union trick. + */ + u.cmd.common.key_flags = key_flags; + u.cmd.common.key_offset = keyconf->hw_key_idx; + u.cmd.common.sta_id = sta_id; + + size = new_api ? sizeof(u.cmd) : sizeof(u.cmd_v1); + + status = ADD_STA_SUCCESS; + ret = iwl_mvm_send_cmd_pdu_status(mvm, ADD_STA_KEY, size, &u.cmd, + &status); + + switch (status) { + case ADD_STA_SUCCESS: + IWL_DEBUG_WEP(mvm, "MODIFY_STA: remove sta key passed\n"); + break; + default: + ret = -EIO; + IWL_ERR(mvm, "MODIFY_STA: remove sta key failed\n"); + break; + } + + return ret; +} + /* * Send the FW a request to remove the station from it's internal data * structures, and in addition remove it from the local data structure. @@ -2365,6 +2420,28 @@ int iwl_mvm_rm_mcast_sta(struct iwl_mvm *mvm, struct ieee80211_vif *vif) iwl_mvm_disable_txq(mvm, NULL, mvmvif->cab_queue, 0, 0); + if (mvmvif->ap_wep_key) { + int i; + + if (!__test_and_clear_bit(mvmvif->ap_wep_key->hw_key_idx, + mvm->fw_key_table)) { + IWL_ERR(mvm, "offset %d not used in fw key table.\n", + mvmvif->ap_wep_key->hw_key_idx); + return -ENOENT; + } + + /* track which key was deleted last */ + for (i = 0; i < STA_KEY_MAX_NUM; i++) { + if (mvm->fw_key_deleted[i] < U8_MAX) + mvm->fw_key_deleted[i]++; + } + mvm->fw_key_deleted[mvmvif->ap_wep_key->hw_key_idx] = 0; + ret = __iwl_mvm_remove_sta_key(mvm, mvmvif->mcast_sta.sta_id, + mvmvif->ap_wep_key, true); + if (ret) + return ret; + } + ret = iwl_mvm_rm_sta_common(mvm, mvmvif->mcast_sta.sta_id); if (ret) IWL_WARN(mvm, "Failed sending remove station\n"); @@ -3396,59 +3473,6 @@ static int __iwl_mvm_set_sta_key(struct iwl_mvm *mvm, return ret; } -static int __iwl_mvm_remove_sta_key(struct iwl_mvm *mvm, u8 sta_id, - struct ieee80211_key_conf *keyconf, - bool mcast) -{ - union { - struct iwl_mvm_add_sta_key_cmd_v1 cmd_v1; - struct iwl_mvm_add_sta_key_cmd cmd; - } u = {}; - bool new_api = fw_has_api(&mvm->fw->ucode_capa, - IWL_UCODE_TLV_API_TKIP_MIC_KEYS); - __le16 key_flags; - int ret, size; - u32 status; - - /* This is a valid situation for GTK removal */ - if (sta_id == IWL_MVM_INVALID_STA) - return 0; - - key_flags = cpu_to_le16((keyconf->keyidx << STA_KEY_FLG_KEYID_POS) & - STA_KEY_FLG_KEYID_MSK); - key_flags |= cpu_to_le16(STA_KEY_FLG_NO_ENC | STA_KEY_FLG_WEP_KEY_MAP); - key_flags |= cpu_to_le16(STA_KEY_NOT_VALID); - - if (mcast) - key_flags |= cpu_to_le16(STA_KEY_MULTICAST); - - /* - * The fields assigned here are in the same location at the start - * of the command, so we can do this union trick. - */ - u.cmd.common.key_flags = key_flags; - u.cmd.common.key_offset = keyconf->hw_key_idx; - u.cmd.common.sta_id = sta_id; - - size = new_api ? sizeof(u.cmd) : sizeof(u.cmd_v1); - - status = ADD_STA_SUCCESS; - ret = iwl_mvm_send_cmd_pdu_status(mvm, ADD_STA_KEY, size, &u.cmd, - &status); - - switch (status) { - case ADD_STA_SUCCESS: - IWL_DEBUG_WEP(mvm, "MODIFY_STA: remove sta key passed\n"); - break; - default: - ret = -EIO; - IWL_ERR(mvm, "MODIFY_STA: remove sta key failed\n"); - break; - } - - return ret; -} - int iwl_mvm_set_sta_key(struct iwl_mvm *mvm, struct ieee80211_vif *vif, struct ieee80211_sta *sta, diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h index 0614296..79700c7 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.h +++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.h @@ -394,6 +394,7 @@ struct iwl_mvm_rxq_dup_data { * the BA window. To be used for UAPSD only. * @ptk_pn: per-queue PTK PN data structures * @dup_data: per queue duplicate packet detection data + * @wep_key: used in AP mode. Is a duplicate of the WEP key. * @deferred_traffic_tid_map: indication bitmap of deferred traffic per-TID * @tx_ant: the index of the antenna to use for data tx to this station. Only * used during connection establishment (e.g. for the 4 way handshake @@ -425,6 +426,8 @@ struct iwl_mvm_sta { struct iwl_mvm_key_pn __rcu *ptk_pn[4]; struct iwl_mvm_rxq_dup_data *dup_data; + struct ieee80211_key_conf *wep_key; + u8 reserved_queue; /* Temporary, until the new TLC will control the Tx protection */ -- 2.7.4