iwlwifi: yoyo: don't access TLV before verifying len

If we access the TLV memory with shorter len than the struct
we access garbage data that was not given by the user.

On the way rewrite the checker in a cleaner way.

Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Fixes: a9248de42464 ("iwlwifi: dbg_ini: add TLV allocation new API support")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20200424182644.54418c829390.I15d6b462a0e69a280b6c6cfbcb6bcb05bb5f79ee@changeid

diff --git a/drivers/net/wireless/intel/iwlwifi/fw/api/dbg-tlv.h b/drivers/net/wireless/intel/iwlwifi/fw/api/dbg-tlv.h
index b9d7ed93311c0f78f086f8d641a9c5cd04bf0672..74ac65bd545aa0da18da134ac93bf7fa81e174ff 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/fw/api/dbg-tlv.h
+++ b/drivers/net/wireless/intel/iwlwifi/fw/api/dbg-tlv.h
@@ -5,7 +5,7 @@
  *
  * GPL LICENSE SUMMARY
  *
- * Copyright (C) 2018 - 2019 Intel Corporation
+ * Copyright (C) 2018 - 2020 Intel Corporation
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of version 2 of the GNU General Public License as
@@ -25,7 +25,7 @@
  *
  * BSD LICENSE
  *
- * Copyright (C) 2018 - 2019 Intel Corporation
+ * Copyright (C) 2018 - 2020 Intel Corporation
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -304,6 +304,7 @@ enum iwl_fw_ini_buffer_location {
        IWL_FW_INI_LOCATION_SRAM_PATH,
        IWL_FW_INI_LOCATION_DRAM_PATH,
        IWL_FW_INI_LOCATION_NPK_PATH,
+       IWL_FW_INI_LOCATION_NUM,
 }; /* FW_DEBUG_TLV_BUFFER_LOCATION_E_VER_1 */
 
 /**

diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
index 9eb8fbfaa2a2a311981d58da35b40c8c2326e66c..7987a288917bac0aab3ccb3164fdb7634255e473 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
@@ -165,38 +165,36 @@ static int iwl_dbg_tlv_alloc_buf_alloc(struct iwl_trans *trans,
                                       struct iwl_ucode_tlv *tlv)
 {
        struct iwl_fw_ini_allocation_tlv *alloc = (void *)tlv->data;
-       u32 buf_location = le32_to_cpu(alloc->buf_location);
-       u32 alloc_id = le32_to_cpu(alloc->alloc_id);
+       u32 buf_location;
+       u32 alloc_id;
 
-       if (le32_to_cpu(tlv->length) != sizeof(*alloc) ||
-           (buf_location != IWL_FW_INI_LOCATION_SRAM_PATH &&
-            buf_location != IWL_FW_INI_LOCATION_DRAM_PATH &&
-            buf_location != IWL_FW_INI_LOCATION_NPK_PATH)) {
-               IWL_ERR(trans,
-                       "WRT: Invalid allocation TLV\n");
+       if (le32_to_cpu(tlv->length) != sizeof(*alloc))
                return -EINVAL;
-       }
 
-       if ((buf_location == IWL_FW_INI_LOCATION_SRAM_PATH ||
-            buf_location == IWL_FW_INI_LOCATION_NPK_PATH) &&
-            alloc_id != IWL_FW_INI_ALLOCATION_ID_DBGC1) {
-               IWL_ERR(trans,
-                       "WRT: Allocation TLV for SMEM/NPK path must have id %u (current: %u)\n",
-                       IWL_FW_INI_ALLOCATION_ID_DBGC1, alloc_id);
-               return -EINVAL;
-       }
+       buf_location = le32_to_cpu(alloc->buf_location);
+       alloc_id = le32_to_cpu(alloc->alloc_id);
+
+       if (buf_location == IWL_FW_INI_LOCATION_INVALID ||
+           buf_location >= IWL_FW_INI_LOCATION_NUM)
+               goto err;
 
        if (alloc_id == IWL_FW_INI_ALLOCATION_INVALID ||
-           alloc_id >= IWL_FW_INI_ALLOCATION_NUM) {
-               IWL_ERR(trans,
-                       "WRT: Invalid allocation id %u for allocation TLV\n",
-                       alloc_id);
-               return -EINVAL;
-       }
+           alloc_id >= IWL_FW_INI_ALLOCATION_NUM)
+               goto err;
+
+       if ((buf_location == IWL_FW_INI_LOCATION_SRAM_PATH ||
+            buf_location == IWL_FW_INI_LOCATION_NPK_PATH) &&
+            alloc_id != IWL_FW_INI_ALLOCATION_ID_DBGC1)
+               goto err;
 
        trans->dbg.fw_mon_cfg[alloc_id] = *alloc;
 
        return 0;
+err:
+       IWL_ERR(trans,
+               "WRT: Invalid allocation id %u and/or location id %u for allocation TLV\n",
+               alloc_id, buf_location);
+       return -EINVAL;
 }
 
 static int iwl_dbg_tlv_alloc_hcmd(struct iwl_trans *trans,