mac80211, mt76: add fixes for recently discovered security issues
authorFelix Fietkau <nbd@nbd.name>
Wed, 29 Mar 2023 15:54:19 +0000 (17:54 +0200)
committerFelix Fietkau <nbd@nbd.name>
Thu, 30 Mar 2023 10:14:47 +0000 (12:14 +0200)
Fixes CVE-2022-47522

Signed-off-by: Felix Fietkau <nbd@nbd.name>
(cherry picked from commit d54c91bd9ab3c54ee06923eafbd67047816a37e4)

package/kernel/mac80211/patches/subsys/347-wifi-ieee80211-correctly-mark-FTM-frames-non-buffera.patch [new file with mode: 0644]
package/kernel/mac80211/patches/subsys/348-wifi-mac80211-flush-queues-on-STA-removal.patch [new file with mode: 0644]
package/kernel/mac80211/patches/subsys/349-wifi-iwlwifi-mvm-support-flush-on-AP-interfaces.patch [new file with mode: 0644]
package/kernel/mac80211/patches/subsys/350-wifi-mac80211-add-flush_sta-method.patch [new file with mode: 0644]
package/kernel/mac80211/patches/subsys/351-wifi-iwlwifi-mvm-support-new-flush_sta-method.patch [new file with mode: 0644]
package/kernel/mt76/patches/110-api_update.patch [new file with mode: 0644]
package/kernel/mt76/patches/120-wifi-mt76-ignore-key-disable-commands.patch [new file with mode: 0644]

diff --git a/package/kernel/mac80211/patches/subsys/347-wifi-ieee80211-correctly-mark-FTM-frames-non-buffera.patch b/package/kernel/mac80211/patches/subsys/347-wifi-ieee80211-correctly-mark-FTM-frames-non-buffera.patch
new file mode 100644 (file)
index 0000000..ac707de
--- /dev/null
@@ -0,0 +1,134 @@
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Wed, 29 Mar 2023 16:46:26 +0200
+Subject: [PATCH] wifi: ieee80211: correctly mark FTM frames non-bufferable
+
+The checks of whether or not a frame is bufferable were not
+taking into account that some action frames aren't, such as
+FTM. Check this, which requires some changes to the function
+ieee80211_is_bufferable_mmpdu() since we need the whole skb
+for the checks now.
+
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Reviewed-by: Greenman, Gregory <gregory.greenman@intel.com>
+Reviewed-by: Peer, Ilan <ilan.peer@intel.com>
+---
+
+--- a/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
++++ b/drivers/net/wireless/intel/iwlwifi/mvm/tx.c
+@@ -551,8 +551,9 @@ static void iwl_mvm_skb_prepare_status(s
+ static int iwl_mvm_get_ctrl_vif_queue(struct iwl_mvm *mvm,
+                                     struct ieee80211_tx_info *info,
+-                                    struct ieee80211_hdr *hdr)
++                                    struct sk_buff *skb)
+ {
++      struct ieee80211_hdr *hdr = (void *)skb->data;
+       struct iwl_mvm_vif *mvmvif =
+               iwl_mvm_vif_from_mac80211(info->control.vif);
+       __le16 fc = hdr->frame_control;
+@@ -571,7 +572,7 @@ static int iwl_mvm_get_ctrl_vif_queue(st
+                * reason 7 ("Class 3 frame received from nonassociated STA").
+                */
+               if (ieee80211_is_mgmt(fc) &&
+-                  (!ieee80211_is_bufferable_mmpdu(fc) ||
++                  (!ieee80211_is_bufferable_mmpdu(skb) ||
+                    ieee80211_is_deauth(fc) || ieee80211_is_disassoc(fc)))
+                       return mvm->probe_queue;
+@@ -689,7 +690,7 @@ int iwl_mvm_tx_skb_non_sta(struct iwl_mv
+                       else
+                               sta_id = mvmvif->mcast_sta.sta_id;
+-                      queue = iwl_mvm_get_ctrl_vif_queue(mvm, &info, hdr);
++                      queue = iwl_mvm_get_ctrl_vif_queue(mvm, &info, skb);
+               } else if (info.control.vif->type == NL80211_IFTYPE_MONITOR) {
+                       queue = mvm->snif_queue;
+                       sta_id = mvm->snif_sta.sta_id;
+--- a/include/linux/ieee80211.h
++++ b/include/linux/ieee80211.h
+@@ -738,20 +738,6 @@ static inline bool ieee80211_is_any_null
+ }
+ /**
+- * ieee80211_is_bufferable_mmpdu - check if frame is bufferable MMPDU
+- * @fc: frame control field in little-endian byteorder
+- */
+-static inline bool ieee80211_is_bufferable_mmpdu(__le16 fc)
+-{
+-      /* IEEE 802.11-2012, definition of "bufferable management frame";
+-       * note that this ignores the IBSS special case. */
+-      return ieee80211_is_mgmt(fc) &&
+-             (ieee80211_is_action(fc) ||
+-              ieee80211_is_disassoc(fc) ||
+-              ieee80211_is_deauth(fc));
+-}
+-
+-/**
+  * ieee80211_is_first_frag - check if IEEE80211_SCTL_FRAG is not set
+  * @seq_ctrl: frame sequence control bytes in little-endian byteorder
+  */
+@@ -3672,6 +3658,44 @@ static inline u8 *ieee80211_get_DA(struc
+ }
+ /**
++ * ieee80211_is_bufferable_mmpdu - check if frame is bufferable MMPDU
++ * @skb: the skb to check, starting with the 802.11 header
++ */
++static inline bool ieee80211_is_bufferable_mmpdu(struct sk_buff *skb)
++{
++      struct ieee80211_mgmt *mgmt = (void *)skb->data;
++      __le16 fc = mgmt->frame_control;
++
++      /*
++       * IEEE 802.11 REVme D2.0 definition of bufferable MMPDU;
++       * note that this ignores the IBSS special case.
++       */
++      if (!ieee80211_is_mgmt(fc))
++              return false;
++
++      if (ieee80211_is_disassoc(fc) || ieee80211_is_deauth(fc))
++              return true;
++
++      if (!ieee80211_is_action(fc))
++              return false;
++
++      if (skb->len < offsetofend(typeof(*mgmt), u.action.u.ftm.action_code))
++              return true;
++
++      /* action frame - additionally check for non-bufferable FTM */
++
++      if (mgmt->u.action.category != WLAN_CATEGORY_PUBLIC &&
++          mgmt->u.action.category != WLAN_CATEGORY_PROTECTED_DUAL_OF_ACTION)
++              return true;
++
++      if (mgmt->u.action.u.ftm.action_code == WLAN_PUB_ACTION_FTM_REQUEST ||
++          mgmt->u.action.u.ftm.action_code == WLAN_PUB_ACTION_FTM)
++              return false;
++
++      return true;
++}
++
++/**
+  * _ieee80211_is_robust_mgmt_frame - check if frame is a robust management frame
+  * @hdr: the frame (buffer must include at least the first octet of payload)
+  */
+--- a/net/mac80211/tx.c
++++ b/net/mac80211/tx.c
+@@ -487,7 +487,7 @@ ieee80211_tx_h_unicast_ps_buf(struct iee
+               int ac = skb_get_queue_mapping(tx->skb);
+               if (ieee80211_is_mgmt(hdr->frame_control) &&
+-                  !ieee80211_is_bufferable_mmpdu(hdr->frame_control)) {
++                  !ieee80211_is_bufferable_mmpdu(tx->skb)) {
+                       info->flags |= IEEE80211_TX_CTL_NO_PS_BUFFER;
+                       return TX_CONTINUE;
+               }
+@@ -1282,7 +1282,7 @@ static struct txq_info *ieee80211_get_tx
+       if (!(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
+           unlikely(!ieee80211_is_data_present(hdr->frame_control))) {
+               if ((!ieee80211_is_mgmt(hdr->frame_control) ||
+-                   ieee80211_is_bufferable_mmpdu(hdr->frame_control) ||
++                   ieee80211_is_bufferable_mmpdu(skb) ||
+                    vif->type == NL80211_IFTYPE_STATION) &&
+                   sta && sta->uploaded) {
+                       /*
diff --git a/package/kernel/mac80211/patches/subsys/348-wifi-mac80211-flush-queues-on-STA-removal.patch b/package/kernel/mac80211/patches/subsys/348-wifi-mac80211-flush-queues-on-STA-removal.patch
new file mode 100644 (file)
index 0000000..3e148a9
--- /dev/null
@@ -0,0 +1,36 @@
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Mon, 13 Mar 2023 11:42:12 +0100
+Subject: [PATCH] wifi: mac80211: flush queues on STA removal
+
+When we remove a station, we first make it unreachable,
+then we (must) remove its keys, and then remove the
+station itself. Depending on the hardware design, if
+we have hardware crypto at all, frames still sitting
+on hardware queues may then be transmitted without a
+valid key, possibly unencrypted or with a fixed key.
+
+Fix this by flushing the queues when removing stations
+so this cannot happen.
+
+Cc: stable@vger.kernel.org
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Reviewed-by: Greenman, Gregory <gregory.greenman@intel.com>
+---
+
+--- a/net/mac80211/sta_info.c
++++ b/net/mac80211/sta_info.c
+@@ -1070,6 +1070,14 @@ static void __sta_info_destroy_part2(str
+               WARN_ON_ONCE(ret);
+       }
++      /* Flush queues before removing keys, as that might remove them
++       * from hardware, and then depending on the offload method, any
++       * frames sitting on hardware queues might be sent out without
++       * any encryption at all.
++       */
++      if (local->ops->set_key)
++              ieee80211_flush_queues(local, sta->sdata, false);
++
+       /* now keys can no longer be reached */
+       ieee80211_free_sta_keys(local, sta);
diff --git a/package/kernel/mac80211/patches/subsys/349-wifi-iwlwifi-mvm-support-flush-on-AP-interfaces.patch b/package/kernel/mac80211/patches/subsys/349-wifi-iwlwifi-mvm-support-flush-on-AP-interfaces.patch
new file mode 100644 (file)
index 0000000..0b070b1
--- /dev/null
@@ -0,0 +1,34 @@
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Mon, 13 Mar 2023 12:02:58 +0100
+Subject: [PATCH] wifi: iwlwifi: mvm: support flush on AP interfaces
+
+Support TX flush on AP interfaces so that we will do a
+proper flush for frames on the queue before keys are
+removed.
+
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Reviewed-by: Greenman, Gregory <gregory.greenman@intel.com>
+---
+
+--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
++++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+@@ -4817,9 +4817,6 @@ static void iwl_mvm_mac_flush(struct iee
+               return;
+       }
+-      if (vif->type != NL80211_IFTYPE_STATION)
+-              return;
+-
+       /* Make sure we're done with the deferred traffic before flushing */
+       flush_work(&mvm->add_stream_wk);
+@@ -4837,9 +4834,6 @@ static void iwl_mvm_mac_flush(struct iee
+               if (mvmsta->vif != vif)
+                       continue;
+-              /* make sure only TDLS peers or the AP are flushed */
+-              WARN_ON(i != mvmvif->ap_sta_id && !sta->tdls);
+-
+               if (drop) {
+                       if (iwl_mvm_flush_sta(mvm, mvmsta, false))
+                               IWL_ERR(mvm, "flush request fail\n");
diff --git a/package/kernel/mac80211/patches/subsys/350-wifi-mac80211-add-flush_sta-method.patch b/package/kernel/mac80211/patches/subsys/350-wifi-mac80211-add-flush_sta-method.patch
new file mode 100644 (file)
index 0000000..ae2ef83
--- /dev/null
@@ -0,0 +1,91 @@
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Mon, 13 Mar 2023 11:53:51 +0100
+Subject: [PATCH] wifi: mac80211: add flush_sta method
+
+Some drivers like iwlwifi might have per-STA queues, so we
+may want to flush/drop just those queues rather than all
+when removing a station. Add a separate method for that.
+
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Reviewed-by: Greenman, Gregory <gregory.greenman@intel.com>
+---
+
+--- a/include/net/mac80211.h
++++ b/include/net/mac80211.h
+@@ -3688,6 +3688,10 @@ struct ieee80211_prep_tx_info {
+  *    Note that vif can be NULL.
+  *    The callback can sleep.
+  *
++ * @flush_sta: Flush or drop all pending frames from the hardware queue(s) for
++ *    the given station, as it's about to be removed.
++ *    The callback can sleep.
++ *
+  * @channel_switch: Drivers that need (or want) to offload the channel
+  *    switch operation for CSAs received from the AP may implement this
+  *    callback. They must then call ieee80211_chswitch_done() to indicate
+@@ -4116,6 +4120,8 @@ struct ieee80211_ops {
+ #endif
+       void (*flush)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+                     u32 queues, bool drop);
++      void (*flush_sta)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
++                        struct ieee80211_sta *sta);
+       void (*channel_switch)(struct ieee80211_hw *hw,
+                              struct ieee80211_vif *vif,
+                              struct ieee80211_channel_switch *ch_switch);
+--- a/net/mac80211/driver-ops.h
++++ b/net/mac80211/driver-ops.h
+@@ -639,6 +639,21 @@ static inline void drv_flush(struct ieee
+       trace_drv_return_void(local);
+ }
++static inline void drv_flush_sta(struct ieee80211_local *local,
++                               struct ieee80211_sub_if_data *sdata,
++                               struct sta_info *sta)
++{
++      might_sleep();
++
++      if (sdata && !check_sdata_in_driver(sdata))
++              return;
++
++      trace_drv_flush_sta(local, sdata, &sta->sta);
++      if (local->ops->flush_sta)
++              local->ops->flush_sta(&local->hw, &sdata->vif, &sta->sta);
++      trace_drv_return_void(local);
++}
++
+ static inline void drv_channel_switch(struct ieee80211_local *local,
+                                     struct ieee80211_sub_if_data *sdata,
+                                     struct ieee80211_channel_switch *ch_switch)
+--- a/net/mac80211/sta_info.c
++++ b/net/mac80211/sta_info.c
+@@ -1075,8 +1075,12 @@ static void __sta_info_destroy_part2(str
+        * frames sitting on hardware queues might be sent out without
+        * any encryption at all.
+        */
+-      if (local->ops->set_key)
+-              ieee80211_flush_queues(local, sta->sdata, false);
++      if (local->ops->set_key) {
++              if (local->ops->flush_sta)
++                      drv_flush_sta(local, sta->sdata, sta);
++              else
++                      ieee80211_flush_queues(local, sta->sdata, false);
++      }
+       /* now keys can no longer be reached */
+       ieee80211_free_sta_keys(local, sta);
+--- a/net/mac80211/trace.h
++++ b/net/mac80211/trace.h
+@@ -1140,6 +1140,13 @@ TRACE_EVENT(drv_flush,
+       )
+ );
++DEFINE_EVENT(sta_event, drv_flush_sta,
++      TP_PROTO(struct ieee80211_local *local,
++               struct ieee80211_sub_if_data *sdata,
++               struct ieee80211_sta *sta),
++      TP_ARGS(local, sdata, sta)
++);
++
+ TRACE_EVENT(drv_channel_switch,
+       TP_PROTO(struct ieee80211_local *local,
+                struct ieee80211_sub_if_data *sdata,
diff --git a/package/kernel/mac80211/patches/subsys/351-wifi-iwlwifi-mvm-support-new-flush_sta-method.patch b/package/kernel/mac80211/patches/subsys/351-wifi-iwlwifi-mvm-support-new-flush_sta-method.patch
new file mode 100644 (file)
index 0000000..31f60ce
--- /dev/null
@@ -0,0 +1,53 @@
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Mon, 13 Mar 2023 12:05:35 +0100
+Subject: [PATCH] wifi: iwlwifi: mvm: support new flush_sta method
+
+For iwlwifi this is simple to implement, and on newer hardware
+it's an improvement since we have per-station queues.
+
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Reviewed-by: Greenman, Gregory <gregory.greenman@intel.com>
+---
+
+--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
++++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+@@ -4853,6 +4853,31 @@ static void iwl_mvm_mac_flush(struct iee
+               iwl_trans_wait_tx_queues_empty(mvm->trans, msk);
+ }
++static void iwl_mvm_mac_flush_sta(struct ieee80211_hw *hw,
++                                struct ieee80211_vif *vif,
++                                struct ieee80211_sta *sta)
++{
++      struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
++      int i;
++
++      mutex_lock(&mvm->mutex);
++      for (i = 0; i < mvm->fw->ucode_capa.num_stations; i++) {
++              struct iwl_mvm_sta *mvmsta;
++              struct ieee80211_sta *tmp;
++
++              tmp = rcu_dereference_protected(mvm->fw_id_to_mac_id[i],
++                                              lockdep_is_held(&mvm->mutex));
++              if (tmp != sta)
++                      continue;
++
++              mvmsta = iwl_mvm_sta_from_mac80211(sta);
++
++              if (iwl_mvm_flush_sta(mvm, mvmsta, false))
++                      IWL_ERR(mvm, "flush request fail\n");
++      }
++      mutex_unlock(&mvm->mutex);
++}
++
+ static int iwl_mvm_mac_get_survey(struct ieee80211_hw *hw, int idx,
+                                 struct survey_info *survey)
+ {
+@@ -5366,6 +5391,7 @@ const struct ieee80211_ops iwl_mvm_hw_op
+       .mgd_prepare_tx = iwl_mvm_mac_mgd_prepare_tx,
+       .mgd_protect_tdls_discover = iwl_mvm_mac_mgd_protect_tdls_discover,
+       .flush = iwl_mvm_mac_flush,
++      .flush_sta = iwl_mvm_mac_flush_sta,
+       .sched_scan_start = iwl_mvm_mac_sched_scan_start,
+       .sched_scan_stop = iwl_mvm_mac_sched_scan_stop,
+       .set_key = iwl_mvm_mac_set_key,
diff --git a/package/kernel/mt76/patches/110-api_update.patch b/package/kernel/mt76/patches/110-api_update.patch
new file mode 100644 (file)
index 0000000..27bd628
--- /dev/null
@@ -0,0 +1,11 @@
+--- a/tx.c
++++ b/tx.c
+@@ -325,7 +325,7 @@ mt76_tx(struct mt76_phy *phy, struct iee
+       if ((dev->drv->drv_flags & MT_DRV_HW_MGMT_TXQ) &&
+           !(info->flags & IEEE80211_TX_CTL_HW_80211_ENCAP) &&
+           !ieee80211_is_data(hdr->frame_control) &&
+-          !ieee80211_is_bufferable_mmpdu(hdr->frame_control)) {
++          !ieee80211_is_bufferable_mmpdu(skb)) {
+               qid = MT_TXQ_PSD;
+       }
diff --git a/package/kernel/mt76/patches/120-wifi-mt76-ignore-key-disable-commands.patch b/package/kernel/mt76/patches/120-wifi-mt76-ignore-key-disable-commands.patch
new file mode 100644 (file)
index 0000000..3ac6ceb
--- /dev/null
@@ -0,0 +1,301 @@
+From: Felix Fietkau <nbd@nbd.name>
+Date: Wed, 22 Mar 2023 10:17:49 +0100
+Subject: [PATCH] wifi: mt76: ignore key disable commands
+
+This helps avoid cleartext leakage of already queued or powersave buffered
+packets, when a reassoc triggers the key deletion.
+
+Cc: stable@vger.kernel.org
+Signed-off-by: Felix Fietkau <nbd@nbd.name>
+---
+
+--- a/mt7603/main.c
++++ b/mt7603/main.c
+@@ -512,15 +512,15 @@ mt7603_set_key(struct ieee80211_hw *hw,
+           !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE))
+               return -EOPNOTSUPP;
+-      if (cmd == SET_KEY) {
+-              key->hw_key_idx = wcid->idx;
+-              wcid->hw_key_idx = idx;
+-      } else {
++      if (cmd != SET_KEY) {
+               if (idx == wcid->hw_key_idx)
+                       wcid->hw_key_idx = -1;
+-              key = NULL;
++              return 0;
+       }
++
++      key->hw_key_idx = wcid->idx;
++      wcid->hw_key_idx = idx;
+       mt76_wcid_key_setup(&dev->mt76, wcid, key);
+       return mt7603_wtbl_set_key(dev, wcid->idx, key);
+--- a/mt7615/mac.c
++++ b/mt7615/mac.c
+@@ -1178,8 +1178,7 @@ EXPORT_SYMBOL_GPL(mt7615_mac_set_rates);
+ static int
+ mt7615_mac_wtbl_update_key(struct mt7615_dev *dev, struct mt76_wcid *wcid,
+                          struct ieee80211_key_conf *key,
+-                         enum mt76_cipher_type cipher, u16 cipher_mask,
+-                         enum set_key_cmd cmd)
++                         enum mt76_cipher_type cipher, u16 cipher_mask)
+ {
+       u32 addr = mt7615_mac_wtbl_addr(dev, wcid->idx) + 30 * 4;
+       u8 data[32] = {};
+@@ -1188,27 +1187,18 @@ mt7615_mac_wtbl_update_key(struct mt7615
+               return -EINVAL;
+       mt76_rr_copy(dev, addr, data, sizeof(data));
+-      if (cmd == SET_KEY) {
+-              if (cipher == MT_CIPHER_TKIP) {
+-                      /* Rx/Tx MIC keys are swapped */
+-                      memcpy(data, key->key, 16);
+-                      memcpy(data + 16, key->key + 24, 8);
+-                      memcpy(data + 24, key->key + 16, 8);
+-              } else {
+-                      if (cipher_mask == BIT(cipher))
+-                              memcpy(data, key->key, key->keylen);
+-                      else if (cipher != MT_CIPHER_BIP_CMAC_128)
+-                              memcpy(data, key->key, 16);
+-                      if (cipher == MT_CIPHER_BIP_CMAC_128)
+-                              memcpy(data + 16, key->key, 16);
+-              }
++      if (cipher == MT_CIPHER_TKIP) {
++              /* Rx/Tx MIC keys are swapped */
++              memcpy(data, key->key, 16);
++              memcpy(data + 16, key->key + 24, 8);
++              memcpy(data + 24, key->key + 16, 8);
+       } else {
++              if (cipher_mask == BIT(cipher))
++                      memcpy(data, key->key, key->keylen);
++              else if (cipher != MT_CIPHER_BIP_CMAC_128)
++                      memcpy(data, key->key, 16);
+               if (cipher == MT_CIPHER_BIP_CMAC_128)
+-                      memset(data + 16, 0, 16);
+-              else if (cipher_mask)
+-                      memset(data, 0, 16);
+-              if (!cipher_mask)
+-                      memset(data, 0, sizeof(data));
++                      memcpy(data + 16, key->key, 16);
+       }
+       mt76_wr_copy(dev, addr, data, sizeof(data));
+@@ -1219,7 +1209,7 @@ mt7615_mac_wtbl_update_key(struct mt7615
+ static int
+ mt7615_mac_wtbl_update_pk(struct mt7615_dev *dev, struct mt76_wcid *wcid,
+                         enum mt76_cipher_type cipher, u16 cipher_mask,
+-                        int keyidx, enum set_key_cmd cmd)
++                        int keyidx)
+ {
+       u32 addr = mt7615_mac_wtbl_addr(dev, wcid->idx), w0, w1;
+@@ -1238,9 +1228,7 @@ mt7615_mac_wtbl_update_pk(struct mt7615_
+       else
+               w0 &= ~MT_WTBL_W0_RX_IK_VALID;
+-      if (cmd == SET_KEY &&
+-          (cipher != MT_CIPHER_BIP_CMAC_128 ||
+-           cipher_mask == BIT(cipher))) {
++      if (cipher != MT_CIPHER_BIP_CMAC_128 || cipher_mask == BIT(cipher)) {
+               w0 &= ~MT_WTBL_W0_KEY_IDX;
+               w0 |= FIELD_PREP(MT_WTBL_W0_KEY_IDX, keyidx);
+       }
+@@ -1257,19 +1245,10 @@ mt7615_mac_wtbl_update_pk(struct mt7615_
+ static void
+ mt7615_mac_wtbl_update_cipher(struct mt7615_dev *dev, struct mt76_wcid *wcid,
+-                            enum mt76_cipher_type cipher, u16 cipher_mask,
+-                            enum set_key_cmd cmd)
++                            enum mt76_cipher_type cipher, u16 cipher_mask)
+ {
+       u32 addr = mt7615_mac_wtbl_addr(dev, wcid->idx);
+-      if (!cipher_mask) {
+-              mt76_clear(dev, addr + 2 * 4, MT_WTBL_W2_KEY_TYPE);
+-              return;
+-      }
+-
+-      if (cmd != SET_KEY)
+-              return;
+-
+       if (cipher == MT_CIPHER_BIP_CMAC_128 &&
+           cipher_mask & ~BIT(MT_CIPHER_BIP_CMAC_128))
+               return;
+@@ -1280,8 +1259,7 @@ mt7615_mac_wtbl_update_cipher(struct mt7
+ int __mt7615_mac_wtbl_set_key(struct mt7615_dev *dev,
+                             struct mt76_wcid *wcid,
+-                            struct ieee80211_key_conf *key,
+-                            enum set_key_cmd cmd)
++                            struct ieee80211_key_conf *key)
+ {
+       enum mt76_cipher_type cipher;
+       u16 cipher_mask = wcid->cipher;
+@@ -1291,19 +1269,14 @@ int __mt7615_mac_wtbl_set_key(struct mt7
+       if (cipher == MT_CIPHER_NONE)
+               return -EOPNOTSUPP;
+-      if (cmd == SET_KEY)
+-              cipher_mask |= BIT(cipher);
+-      else
+-              cipher_mask &= ~BIT(cipher);
+-
+-      mt7615_mac_wtbl_update_cipher(dev, wcid, cipher, cipher_mask, cmd);
+-      err = mt7615_mac_wtbl_update_key(dev, wcid, key, cipher, cipher_mask,
+-                                       cmd);
++      cipher_mask |= BIT(cipher);
++      mt7615_mac_wtbl_update_cipher(dev, wcid, cipher, cipher_mask);
++      err = mt7615_mac_wtbl_update_key(dev, wcid, key, cipher, cipher_mask);
+       if (err < 0)
+               return err;
+       err = mt7615_mac_wtbl_update_pk(dev, wcid, cipher, cipher_mask,
+-                                      key->keyidx, cmd);
++                                      key->keyidx);
+       if (err < 0)
+               return err;
+@@ -1314,13 +1287,12 @@ int __mt7615_mac_wtbl_set_key(struct mt7
+ int mt7615_mac_wtbl_set_key(struct mt7615_dev *dev,
+                           struct mt76_wcid *wcid,
+-                          struct ieee80211_key_conf *key,
+-                          enum set_key_cmd cmd)
++                          struct ieee80211_key_conf *key)
+ {
+       int err;
+       spin_lock_bh(&dev->mt76.lock);
+-      err = __mt7615_mac_wtbl_set_key(dev, wcid, key, cmd);
++      err = __mt7615_mac_wtbl_set_key(dev, wcid, key);
+       spin_unlock_bh(&dev->mt76.lock);
+       return err;
+--- a/mt7615/main.c
++++ b/mt7615/main.c
+@@ -391,18 +391,17 @@ static int mt7615_set_key(struct ieee802
+       if (cmd == SET_KEY)
+               *wcid_keyidx = idx;
+-      else if (idx == *wcid_keyidx)
+-              *wcid_keyidx = -1;
+-      else
++      else {
++              if (idx == *wcid_keyidx)
++                      *wcid_keyidx = -1;
+               goto out;
++      }
+-      mt76_wcid_key_setup(&dev->mt76, wcid,
+-                          cmd == SET_KEY ? key : NULL);
+-
++      mt76_wcid_key_setup(&dev->mt76, wcid, key);
+       if (mt76_is_mmio(&dev->mt76))
+-              err = mt7615_mac_wtbl_set_key(dev, wcid, key, cmd);
++              err = mt7615_mac_wtbl_set_key(dev, wcid, key);
+       else
+-              err = __mt7615_mac_wtbl_set_key(dev, wcid, key, cmd);
++              err = __mt7615_mac_wtbl_set_key(dev, wcid, key);
+ out:
+       mt7615_mutex_release(dev);
+--- a/mt7615/mt7615.h
++++ b/mt7615/mt7615.h
+@@ -482,11 +482,9 @@ int mt7615_mac_write_txwi(struct mt7615_
+ void mt7615_mac_set_timing(struct mt7615_phy *phy);
+ int __mt7615_mac_wtbl_set_key(struct mt7615_dev *dev,
+                             struct mt76_wcid *wcid,
+-                            struct ieee80211_key_conf *key,
+-                            enum set_key_cmd cmd);
++                            struct ieee80211_key_conf *key);
+ int mt7615_mac_wtbl_set_key(struct mt7615_dev *dev, struct mt76_wcid *wcid,
+-                          struct ieee80211_key_conf *key,
+-                          enum set_key_cmd cmd);
++                          struct ieee80211_key_conf *key);
+ void mt7615_mac_reset_work(struct work_struct *work);
+ u32 mt7615_mac_get_sta_tid_sn(struct mt7615_dev *dev, int wcid, u8 tid);
+--- a/mt76x02_util.c
++++ b/mt76x02_util.c
+@@ -455,20 +455,20 @@ int mt76x02_set_key(struct ieee80211_hw
+       msta = sta ? (struct mt76x02_sta *)sta->drv_priv : NULL;
+       wcid = msta ? &msta->wcid : &mvif->group_wcid;
+-      if (cmd == SET_KEY) {
+-              key->hw_key_idx = wcid->idx;
+-              wcid->hw_key_idx = idx;
+-              if (key->flags & IEEE80211_KEY_FLAG_RX_MGMT) {
+-                      key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
+-                      wcid->sw_iv = true;
+-              }
+-      } else {
++      if (cmd != SET_KEY) {
+               if (idx == wcid->hw_key_idx) {
+                       wcid->hw_key_idx = -1;
+                       wcid->sw_iv = false;
+               }
+-              key = NULL;
++              return 0;
++      }
++
++      key->hw_key_idx = wcid->idx;
++      wcid->hw_key_idx = idx;
++      if (key->flags & IEEE80211_KEY_FLAG_RX_MGMT) {
++              key->flags |= IEEE80211_KEY_FLAG_SW_MGMT_TX;
++              wcid->sw_iv = true;
+       }
+       mt76_wcid_key_setup(&dev->mt76, wcid, key);
+--- a/mt7915/main.c
++++ b/mt7915/main.c
+@@ -387,16 +387,15 @@ static int mt7915_set_key(struct ieee802
+               mt7915_mcu_add_bss_info(phy, vif, true);
+       }
+-      if (cmd == SET_KEY)
++      if (cmd == SET_KEY) {
+               *wcid_keyidx = idx;
+-      else if (idx == *wcid_keyidx)
+-              *wcid_keyidx = -1;
+-      else
++      } else {
++              if (idx == *wcid_keyidx)
++                      *wcid_keyidx = -1;
+               goto out;
++      }
+-      mt76_wcid_key_setup(&dev->mt76, wcid,
+-                          cmd == SET_KEY ? key : NULL);
+-
++      mt76_wcid_key_setup(&dev->mt76, wcid, key);
+       err = mt76_connac_mcu_add_key(&dev->mt76, vif, &msta->bip,
+                                     key, MCU_EXT_CMD(STA_REC_UPDATE),
+                                     &msta->wcid, cmd);
+--- a/mt7921/main.c
++++ b/mt7921/main.c
+@@ -463,16 +463,15 @@ static int mt7921_set_key(struct ieee802
+       mt7921_mutex_acquire(dev);
+-      if (cmd == SET_KEY)
++      if (cmd == SET_KEY) {
+               *wcid_keyidx = idx;
+-      else if (idx == *wcid_keyidx)
+-              *wcid_keyidx = -1;
+-      else
++      } else {
++              if (idx == *wcid_keyidx)
++                      *wcid_keyidx = -1;
+               goto out;
++      }
+-      mt76_wcid_key_setup(&dev->mt76, wcid,
+-                          cmd == SET_KEY ? key : NULL);
+-
++      mt76_wcid_key_setup(&dev->mt76, wcid, key);
+       err = mt76_connac_mcu_add_key(&dev->mt76, vif, &msta->bip,
+                                     key, MCU_UNI_CMD(STA_REC_UPDATE),
+                                     &msta->wcid, cmd);