iwlwifi: tighten RX MPDU bounds checks
authorJohannes Berg <johannes.berg@intel.com>
Wed, 9 Dec 2020 21:16:46 +0000 (23:16 +0200)
committerLuca Coelho <luciano.coelho@intel.com>
Wed, 9 Dec 2020 22:16:05 +0000 (00:16 +0200)
Previously, we added checks that the contained MPDU size is long
enough, but really we should also check that the notification
itself fits into the data. Add some checks for that.

Also add unlikely() annotations on the previously added checks.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20201209231352.51cc04cf1e3e.I7bfd6809f8f5feb75f79397646e6656e95688a0e@changeid
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
drivers/net/wireless/intel/iwlwifi/dvm/rx.c
drivers/net/wireless/intel/iwlwifi/mvm/rx.c
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c

index d06278558b33a4f742f2910be83998578be823b2..ecbf8d3cddae9d61de3e8cc68cf708f02dec2b99 100644 (file)
@@ -794,6 +794,12 @@ static void iwlagn_rx_reply_rx(struct iwl_priv *priv,
                IWL_ERR(priv, "MPDU frame without cached PHY data\n");
                return;
        }
+
+       if (unlikely(pkt_len < sizeof(*amsdu))) {
+               IWL_DEBUG_DROP(priv, "Bad REPLY_RX_MPDU_CMD size\n");
+               return;
+       }
+
        phy_res = &priv->last_phy_res;
        amsdu = (struct iwl_rx_mpdu_res_start *)pkt->data;
        header = (struct ieee80211_hdr *)(pkt->data + sizeof(*amsdu));
index 0d2f7b069a527da7aa8bbbe6e06d639dde23eb78..79d1778e6bc0e6d6b38d4d6a383e140a8a087aee 100644 (file)
@@ -298,6 +298,11 @@ void iwl_mvm_rx_rx_mpdu(struct iwl_mvm *mvm, struct napi_struct *napi,
        u32 rx_pkt_status;
        u8 crypt_len = 0;
 
+       if (unlikely(pkt_len < sizeof(*rx_res))) {
+               IWL_DEBUG_DROP(mvm, "Bad REPLY_RX_MPDU_CMD size\n");
+               return;
+       }
+
        phy_info = &mvm->last_phy_info;
        rx_res = (struct iwl_rx_mpdu_res_start *)pkt->data;
        hdr = (struct ieee80211_hdr *)(pkt->data + sizeof(*rx_res));
index e4f73b6cd94c705cffa1a1d082aa31f6f78a3304..8d33739c29efbdeaf1f63e8095331cafe4344239 100644 (file)
@@ -1574,16 +1574,15 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
        struct iwl_rx_packet *pkt = rxb_addr(rxb);
        struct iwl_rx_mpdu_desc *desc = (void *)pkt->data;
        struct ieee80211_hdr *hdr;
-       u32 len = le16_to_cpu(desc->mpdu_len);
+       u32 len;
        u32 pkt_len = iwl_rx_packet_payload_len(pkt);
        u32 rate_n_flags, gp2_on_air_rise;
-       u16 phy_info = le16_to_cpu(desc->phy_info);
+       u16 phy_info;
        struct ieee80211_sta *sta = NULL;
        struct sk_buff *skb;
        u8 crypt_len = 0, channel, energy_a, energy_b;
        size_t desc_size;
        struct iwl_mvm_rx_phy_data phy_data = {
-               .d4 = desc->phy_data4,
                .info_type = IWL_RX_PHY_INFO_TYPE_NONE,
        };
        bool csi = false;
@@ -1591,13 +1590,22 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
        if (unlikely(test_bit(IWL_MVM_STATUS_IN_HW_RESTART, &mvm->status)))
                return;
 
+       if (mvm->trans->trans_cfg->device_family >= IWL_DEVICE_FAMILY_AX210)
+               desc_size = sizeof(*desc);
+       else
+               desc_size = IWL_RX_DESC_SIZE_V1;
+
+       if (unlikely(pkt_len < desc_size)) {
+               IWL_DEBUG_DROP(mvm, "Bad REPLY_RX_MPDU_CMD size\n");
+               return;
+       }
+
        if (mvm->trans->trans_cfg->device_family >= IWL_DEVICE_FAMILY_AX210) {
                rate_n_flags = le32_to_cpu(desc->v3.rate_n_flags);
                channel = desc->v3.channel;
                gp2_on_air_rise = le32_to_cpu(desc->v3.gp2_on_air_rise);
                energy_a = desc->v3.energy_a;
                energy_b = desc->v3.energy_b;
-               desc_size = sizeof(*desc);
 
                phy_data.d0 = desc->v3.phy_data0;
                phy_data.d1 = desc->v3.phy_data1;
@@ -1609,7 +1617,6 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
                gp2_on_air_rise = le32_to_cpu(desc->v1.gp2_on_air_rise);
                energy_a = desc->v1.energy_a;
                energy_b = desc->v1.energy_b;
-               desc_size = IWL_RX_DESC_SIZE_V1;
 
                phy_data.d0 = desc->v1.phy_data0;
                phy_data.d1 = desc->v1.phy_data1;
@@ -1617,16 +1624,21 @@ void iwl_mvm_rx_mpdu_mq(struct iwl_mvm *mvm, struct napi_struct *napi,
                phy_data.d3 = desc->v1.phy_data3;
        }
 
-       if (phy_info & IWL_RX_MPDU_PHY_TSF_OVERLOAD)
-               phy_data.info_type =
-                       le32_get_bits(phy_data.d1,
-                                     IWL_RX_PHY_DATA1_INFO_TYPE_MASK);
+       len = le16_to_cpu(desc->mpdu_len);
 
-       if (len + desc_size > pkt_len) {
+       if (unlikely(len + desc_size > pkt_len)) {
                IWL_DEBUG_DROP(mvm, "FW lied about packet len\n");
                return;
        }
 
+       phy_info = le16_to_cpu(desc->phy_info);
+       phy_data.d4 = desc->phy_data4;
+
+       if (phy_info & IWL_RX_MPDU_PHY_TSF_OVERLOAD)
+               phy_data.info_type =
+                       le32_get_bits(phy_data.d1,
+                                     IWL_RX_PHY_DATA1_INFO_TYPE_MASK);
+
        hdr = (void *)(pkt->data + desc_size);
        /* Dont use dev_alloc_skb(), we'll have enough headroom once
         * ieee80211_hdr pulled.