iwlwifi: dbg_ini: verify debug TLVs at allocation phase
authorShahar S Matityahu <shahar.s.matityahu@intel.com>
Sun, 30 Jun 2019 07:23:26 +0000 (10:23 +0300)
committerLuca Coelho <luciano.coelho@intel.com>
Fri, 6 Sep 2019 12:31:21 +0000 (15:31 +0300)
Reimplement debug TLV allocation flow. The driver will check the
validity of the debug TLVs prior allocating space for them.
Any malformed or unsupported TLV will be skipped.
The TLV specific checks will be added in later patches.

Signed-off-by: Shahar S Matityahu <shahar.s.matityahu@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
drivers/net/wireless/intel/iwlwifi/fw/dbg.c
drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c
drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.h
drivers/net/wireless/intel/iwlwifi/iwl-drv.c
drivers/net/wireless/intel/iwlwifi/iwl-trans.h

index 3679ba89ea9da23645c2fb7066151972a0a23972..6e5a3289e04a9217573bb1547095f320136e88be 100644 (file)
@@ -1739,7 +1739,7 @@ static void iwl_dump_ini_info(struct iwl_fw_runtime *fwrt,
        dump->version = cpu_to_le32(IWL_INI_DUMP_VER);
        dump->trigger_id = trigger->trigger_id;
        dump->is_external_cfg =
-               cpu_to_le32(fwrt->trans->dbg.external_ini_loaded);
+               cpu_to_le32(fwrt->trans->dbg.external_ini_cfg);
 
        dump->ver_type = cpu_to_le32(fwrt->dump.fw_ver.type);
        dump->ver_subtype = cpu_to_le32(fwrt->dump.fw_ver.subtype);
@@ -2855,17 +2855,22 @@ static void iwl_fw_dbg_ini_reset_cfg(struct iwl_fw_runtime *fwrt)
 void iwl_fw_dbg_apply_point(struct iwl_fw_runtime *fwrt,
                            enum iwl_fw_ini_apply_point apply_point)
 {
-       void *data = &fwrt->trans->dbg.apply_points[apply_point];
+       void *data;
 
        IWL_DEBUG_FW(fwrt, "WRT: enabling apply point %d\n", apply_point);
 
        if (apply_point == IWL_FW_INI_APPLY_EARLY)
                iwl_fw_dbg_ini_reset_cfg(fwrt);
 
-       _iwl_fw_dbg_apply_point(fwrt, data, apply_point, false);
+       if (fwrt->trans->dbg.internal_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED) {
+               data = &fwrt->trans->dbg.apply_points[apply_point];
+               _iwl_fw_dbg_apply_point(fwrt, data, apply_point, false);
+       }
 
-       data = &fwrt->trans->dbg.apply_points_ext[apply_point];
-       _iwl_fw_dbg_apply_point(fwrt, data, apply_point, true);
+       if (fwrt->trans->dbg.external_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED) {
+               data = &fwrt->trans->dbg.apply_points_ext[apply_point];
+               _iwl_fw_dbg_apply_point(fwrt, data, apply_point, true);
+       }
 }
 IWL_EXPORT_SYMBOL(iwl_fw_dbg_apply_point);
 
index 7c1e76ee7edea81476efa5e45e2e045a9969e9dd..5b1644a70aceea1cebc38bd327fed15e5d06c3e3 100644 (file)
 #include "iwl-trans.h"
 #include "iwl-dbg-tlv.h"
 
-void iwl_dbg_tlv_copy(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
-                     bool ext)
+/**
+ * enum iwl_dbg_tlv_type - debug TLV types
+ * @IWL_DBG_TLV_TYPE_DEBUG_INFO: debug info TLV
+ * @IWL_DBG_TLV_TYPE_BUF_ALLOC: buffer allocation TLV
+ * @IWL_DBG_TLV_TYPE_HCMD: host command TLV
+ * @IWL_DBG_TLV_TYPE_REGION: region TLV
+ * @IWL_DBG_TLV_TYPE_TRIGGER: trigger TLV
+ * @IWL_DBG_TLV_TYPE_NUM: number of debug TLVs
+ */
+enum iwl_dbg_tlv_type {
+       IWL_DBG_TLV_TYPE_DEBUG_INFO =
+               IWL_UCODE_TLV_TYPE_DEBUG_INFO - IWL_UCODE_TLV_DEBUG_BASE,
+       IWL_DBG_TLV_TYPE_BUF_ALLOC,
+       IWL_DBG_TLV_TYPE_HCMD,
+       IWL_DBG_TLV_TYPE_REGION,
+       IWL_DBG_TLV_TYPE_TRIGGER,
+       IWL_DBG_TLV_TYPE_NUM,
+};
+
+/**
+ * struct iwl_dbg_tlv_ver_data -  debug TLV version struct
+ * @min_ver: min version supported
+ * @max_ver: max version supported
+ */
+struct iwl_dbg_tlv_ver_data {
+       int min_ver;
+       int max_ver;
+};
+
+static const struct iwl_dbg_tlv_ver_data
+dbg_ver_table[IWL_DBG_TLV_TYPE_NUM] = {
+       [IWL_DBG_TLV_TYPE_DEBUG_INFO]   = {.min_ver = 1, .max_ver = 1,},
+       [IWL_DBG_TLV_TYPE_BUF_ALLOC]    = {.min_ver = 1, .max_ver = 1,},
+       [IWL_DBG_TLV_TYPE_HCMD]         = {.min_ver = 1, .max_ver = 1,},
+       [IWL_DBG_TLV_TYPE_REGION]       = {.min_ver = 1, .max_ver = 1,},
+       [IWL_DBG_TLV_TYPE_TRIGGER]      = {.min_ver = 1, .max_ver = 1,},
+};
+
+static int iwl_dbg_tlv_copy(struct iwl_ucode_tlv *tlv, struct list_head *list)
 {
-       struct iwl_apply_point_data *dbg_cfg, *tlv_copy;
-       struct iwl_fw_ini_header *header = (void *)&tlv->data[0];
-       u32 tlv_type = le32_to_cpu(tlv->type);
+       struct iwl_apply_point_data *tlv_copy;
        u32 len = le32_to_cpu(tlv->length);
-       u32 apply_point = le32_to_cpu(header->apply_point);
 
-       if (le32_to_cpu(header->tlv_version) != 1)
-               return;
+       tlv_copy = kzalloc(sizeof(*tlv_copy) + len, GFP_KERNEL);
+       if (!tlv_copy)
+               return -ENOMEM;
 
-       if (WARN_ONCE(apply_point >= IWL_FW_INI_APPLY_NUM,
-                     "Invalid apply point %d\n", apply_point))
-               return;
+       INIT_LIST_HEAD(&tlv_copy->list);
+       memcpy(&tlv_copy->tlv, tlv, sizeof(*tlv) + len);
+
+       if (!list->next)
+               INIT_LIST_HEAD(list);
+
+       list_add_tail(&tlv_copy->list, list);
+
+       return 0;
+}
+
+static bool iwl_dbg_tlv_ver_support(struct iwl_ucode_tlv *tlv)
+{
+       struct iwl_fw_ini_header *hdr = (void *)&tlv->data[0];
+       u32 type = le32_to_cpu(tlv->type);
+       u32 tlv_idx = type - IWL_UCODE_TLV_DEBUG_BASE;
+       u32 ver = le32_to_cpu(hdr->tlv_version);
+
+       if (ver < dbg_ver_table[tlv_idx].min_ver ||
+           ver > dbg_ver_table[tlv_idx].max_ver)
+               return false;
+
+       return true;
+}
+
+void iwl_dbg_tlv_alloc(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
+                      bool ext)
+{
+       struct iwl_fw_ini_header *hdr = (void *)&tlv->data[0];
+       u32 type = le32_to_cpu(tlv->type);
+       u32 pnt = le32_to_cpu(hdr->apply_point);
+       u32 tlv_idx = type - IWL_UCODE_TLV_DEBUG_BASE;
+       enum iwl_ini_cfg_state *cfg_state = ext ?
+               &trans->dbg.external_ini_cfg : &trans->dbg.internal_ini_cfg;
+       struct list_head *dbg_cfg_list = ext ?
+               &trans->dbg.apply_points_ext[pnt].list :
+               &trans->dbg.apply_points[pnt].list;
 
        IWL_DEBUG_FW(trans, "WRT: read TLV 0x%x, apply point %d\n",
-                    tlv_type, apply_point);
+                    type, pnt);
 
-       if (ext)
-               dbg_cfg = &trans->dbg.apply_points_ext[apply_point];
-       else
-               dbg_cfg = &trans->dbg.apply_points[apply_point];
+       if (tlv_idx >= IWL_DBG_TLV_TYPE_NUM) {
+               IWL_ERR(trans, "WRT: Unsupported TLV 0x%x\n", type);
+               goto out_err;
+       }
 
-       tlv_copy = kzalloc(sizeof(*tlv_copy) + len, GFP_KERNEL);
-       if (!tlv_copy) {
-               IWL_ERR(trans, "WRT: No memory for TLV 0x%x, apply point %d\n",
-                       tlv_type, apply_point);
-               return;
+       if (pnt >= IWL_FW_INI_APPLY_NUM) {
+               IWL_ERR(trans, "WRT: Invalid apply point %d\n", pnt);
+               goto out_err;
        }
 
-       INIT_LIST_HEAD(&tlv_copy->list);
-       memcpy(&tlv_copy->tlv, tlv, sizeof(*tlv) + len);
+       if (!iwl_dbg_tlv_ver_support(tlv)) {
+               IWL_ERR(trans, "WRT: Unsupported TLV 0x%x version %u\n", type,
+                       le32_to_cpu(hdr->tlv_version));
+               goto out_err;
+       }
 
-       if (!dbg_cfg->list.next)
-               INIT_LIST_HEAD(&dbg_cfg->list);
+       if (iwl_dbg_tlv_copy(tlv, dbg_cfg_list)) {
+               IWL_ERR(trans,
+                       "WRT: Failed to allocate TLV 0x%x, apply point %d\n",
+                       type, pnt);
+               goto out_err;
+       }
+
+       if (*cfg_state == IWL_INI_CFG_STATE_NOT_LOADED)
+               *cfg_state = IWL_INI_CFG_STATE_LOADED;
 
-       list_add_tail(&tlv_copy->list, &dbg_cfg->list);
+       return;
 
-       trans->dbg.ini_valid = true;
+out_err:
+       *cfg_state = IWL_INI_CFG_STATE_CORRUPTED;
 }
 
 static void iwl_dbg_tlv_free_list(struct list_head *list)
@@ -138,7 +216,6 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
                                 size_t len)
 {
        struct iwl_ucode_tlv *tlv;
-       enum iwl_ucode_tlv_type tlv_type;
        u32 tlv_len;
 
        while (len >= sizeof(*tlv)) {
@@ -146,7 +223,6 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
                tlv = (void *)data;
 
                tlv_len = le32_to_cpu(tlv->length);
-               tlv_type = le32_to_cpu(tlv->type);
 
                if (len < tlv_len) {
                        IWL_ERR(trans, "invalid TLV len: %zd/%u\n",
@@ -156,19 +232,7 @@ static int iwl_dbg_tlv_parse_bin(struct iwl_trans *trans, const u8 *data,
                len -= ALIGN(tlv_len, 4);
                data += sizeof(*tlv) + ALIGN(tlv_len, 4);
 
-               switch (tlv_type) {
-               case IWL_UCODE_TLV_TYPE_DEBUG_INFO:
-               case IWL_UCODE_TLV_TYPE_BUFFER_ALLOCATION:
-               case IWL_UCODE_TLV_TYPE_HCMD:
-               case IWL_UCODE_TLV_TYPE_REGIONS:
-               case IWL_UCODE_TLV_TYPE_TRIGGERS:
-               case IWL_UCODE_TLV_TYPE_DEBUG_FLOW:
-                       iwl_dbg_tlv_copy(trans, tlv, true);
-                       break;
-               default:
-                       WARN_ONCE(1, "Invalid TLV %x\n", tlv_type);
-                       break;
-               }
+               iwl_dbg_tlv_alloc(trans, tlv, true);
        }
 
        return 0;
@@ -179,7 +243,7 @@ void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans)
        const struct firmware *fw;
        int res;
 
-       if (trans->dbg.external_ini_loaded || !iwlwifi_mod_params.enable_ini)
+       if (!iwlwifi_mod_params.enable_ini)
                return;
 
        res = request_firmware(&fw, "iwl-dbg-tlv.ini", dev);
@@ -188,6 +252,5 @@ void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans)
 
        iwl_dbg_tlv_parse_bin(trans, fw->data, fw->size);
 
-       trans->dbg.external_ini_loaded = true;
        release_firmware(fw);
 }
index 72552de801d42be3933b414e90907b611e3563ed..3a60590d274de35d91d557a6ae23858cbcfaac7d 100644 (file)
@@ -77,7 +77,7 @@ struct iwl_apply_point_data {
 struct iwl_trans;
 void iwl_dbg_tlv_load_bin(struct device *dev, struct iwl_trans *trans);
 void iwl_dbg_tlv_free(struct iwl_trans *trans);
-void iwl_dbg_tlv_copy(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
-                     bool ext);
+void iwl_dbg_tlv_alloc(struct iwl_trans *trans, struct iwl_ucode_tlv *tlv,
+                      bool ext);
 
 #endif /* __iwl_dbg_tlv_h__*/
index 3792b421746e1d6fbb6cd145da56a503b99bd765..1351f7fe5ae2fcf987c65d662ab72d3c521176fe 100644 (file)
@@ -1153,7 +1153,7 @@ static int iwl_parse_tlv_firmware(struct iwl_drv *drv,
                case IWL_UCODE_TLV_TYPE_TRIGGERS:
                case IWL_UCODE_TLV_TYPE_DEBUG_FLOW:
                        if (iwlwifi_mod_params.enable_ini)
-                               iwl_dbg_tlv_copy(drv->trans, tlv, false);
+                               iwl_dbg_tlv_alloc(drv->trans, tlv, false);
                        break;
                case IWL_UCODE_TLV_CMD_VERSIONS:
                        if (tlv_len % sizeof(struct iwl_fw_cmd_version)) {
index 731ffb4a33af7b16a275729238b0b29342cc002a..09ed0dd163ebdc83dc96a9d287df5fcb9a6a3e45 100644 (file)
@@ -650,6 +650,19 @@ enum iwl_plat_pm_mode {
        IWL_PLAT_PM_MODE_D3,
 };
 
+/**
+ * enum iwl_ini_cfg_state
+ * @IWL_INI_CFG_STATE_NOT_LOADED: no debug cfg was given
+ * @IWL_INI_CFG_STATE_LOADED: debug cfg was found and loaded
+ * @IWL_INI_CFG_STATE_CORRUPTED: debug cfg was found and some of the TLVs
+ *     are corrupted. The rest of the debug TLVs will still be used
+ */
+enum iwl_ini_cfg_state {
+       IWL_INI_CFG_STATE_NOT_LOADED,
+       IWL_INI_CFG_STATE_LOADED,
+       IWL_INI_CFG_STATE_CORRUPTED,
+};
+
 /* Max time to wait for nmi interrupt */
 #define IWL_TRANS_NMI_TIMEOUT (HZ / 4)
 
@@ -691,8 +704,8 @@ struct iwl_self_init_dram {
  * @umac_error_event_table: addr of umac error table
  * @error_event_table_tlv_status: bitmap that indicates what error table
  *     pointers was recevied via TLV. uses enum &iwl_error_event_table_status
- * @external_ini_loaded: indicates if an external ini cfg was given
- * @ini_valid: indicates if debug ini mode is on
+ * @internal_ini_cfg: internal debug cfg state. Uses &enum iwl_ini_cfg_state
+ * @external_ini_cfg: external debug cfg state. Uses &enum iwl_ini_cfg_state
  * @num_blocks: number of blocks in fw_mon
  * @fw_mon: address of the buffers for firmware monitor
  * @is_alloc: bit i is set if buffer i was allocated
@@ -711,8 +724,8 @@ struct iwl_trans_debug {
        u32 umac_error_event_table;
        unsigned int error_event_table_tlv_status;
 
-       bool external_ini_loaded;
-       bool ini_valid;
+       enum iwl_ini_cfg_state internal_ini_cfg;
+       enum iwl_ini_cfg_state external_ini_cfg;
 
        struct iwl_apply_point_data apply_points[IWL_FW_INI_APPLY_NUM];
        struct iwl_apply_point_data apply_points_ext[IWL_FW_INI_APPLY_NUM];
@@ -1218,7 +1231,8 @@ static inline void iwl_trans_sync_nmi(struct iwl_trans *trans)
 
 static inline bool iwl_trans_dbg_ini_valid(struct iwl_trans *trans)
 {
-       return trans->dbg.ini_valid;
+       return trans->dbg.internal_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED ||
+               trans->dbg.external_ini_cfg != IWL_INI_CFG_STATE_NOT_LOADED;
 }
 
 /*****************************************************