From 704da534af1e366214f790b381fed73ba6c5d37b Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Wed, 14 Jul 2010 09:34:50 -0700 Subject: [PATCH] iwlagn: fix firmware loading TLV error path MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit gcc complains about the firmware loading: iwl-agn.c: In function ‘iwlagn_load_firmware’: iwl-agn.c:1860: warning: ‘tlv_len’ may be used uninitialized in this function iwl-agn.c:1861: warning: ‘tlv_type’ may be used uninitialized in this function iwl-agn.c:1862: warning: ‘tlv_data’ may be used uninitialized in this function This is almost correct but we do do break out of the TLV parsing loop when setting ret. However, the code is hard to follow, and clearly even the compiler is having issues with it too. Additionally, however, the current code is wrong. If there is a TLV length check error, the code will report invalid TLV after parsing: ... because "len" will still be non-zero as we broke out of the loop. So to remove the warning and fix that issue, make the code easier to read by doing length checking with an error label. As a result, we can completely remove the "ret" variable. Also, while at it, remove the "fixed_tlv_size" variable since each TLV type has its own specified length, it just happens that we have only variable length, flags (0 length) and u32 TLVs right now. It should still be checked with more explicit length checks to make it easier to understand. Signed-off-by: Johannes Berg Signed-off-by: Wey-Yi Guy --- drivers/net/wireless/iwlwifi/iwl-agn.c | 79 ++++++++++++-------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c index db86f70d1a3f..573a81b494ea 100644 --- a/drivers/net/wireless/iwlwifi/iwl-agn.c +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c @@ -1828,7 +1828,6 @@ static int iwlagn_load_firmware(struct iwl_priv *priv, u32 tlv_len; enum iwl_ucode_tlv_type tlv_type; const u8 *tlv_data; - int ret = 0; if (len < sizeof(*ucode)) { IWL_ERR(priv, "uCode has invalid length: %zd\n", len); @@ -1864,9 +1863,8 @@ static int iwlagn_load_firmware(struct iwl_priv *priv, len -= sizeof(*ucode); - while (len >= sizeof(*tlv) && !ret) { + while (len >= sizeof(*tlv)) { u16 tlv_alt; - u32 fixed_tlv_size = 4; len -= sizeof(*tlv); tlv = (void *)data; @@ -1914,65 +1912,56 @@ static int iwlagn_load_firmware(struct iwl_priv *priv, pieces->boot_size = tlv_len; break; case IWL_UCODE_TLV_PROBE_MAX_LEN: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - capa->max_probe_length = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + capa->max_probe_length = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_INIT_EVTLOG_PTR: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->init_evtlog_ptr = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->init_evtlog_ptr = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_INIT_EVTLOG_SIZE: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->init_evtlog_size = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->init_evtlog_size = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_INIT_ERRLOG_PTR: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->init_errlog_ptr = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->init_errlog_ptr = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_RUNT_EVTLOG_PTR: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->inst_evtlog_ptr = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->inst_evtlog_ptr = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_RUNT_EVTLOG_SIZE: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->inst_evtlog_size = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->inst_evtlog_size = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_RUNT_ERRLOG_PTR: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - pieces->inst_errlog_ptr = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + pieces->inst_errlog_ptr = le32_to_cpup((__le32 *)tlv_data); break; case IWL_UCODE_TLV_ENHANCE_SENS_TBL: if (tlv_len) - ret = -EINVAL; - else - priv->enhance_sensitivity_table = true; + goto invalid_tlv_len; + priv->enhance_sensitivity_table = true; break; case IWL_UCODE_TLV_PHY_CALIBRATION_SIZE: - if (tlv_len != fixed_tlv_size) - ret = -EINVAL; - else - capa->standard_phy_calibration_size = + if (tlv_len != sizeof(u32)) + goto invalid_tlv_len; + capa->standard_phy_calibration_size = le32_to_cpup((__le32 *)tlv_data); break; default: @@ -1984,14 +1973,16 @@ static int iwlagn_load_firmware(struct iwl_priv *priv, if (len) { IWL_ERR(priv, "invalid TLV after parsing: %zd\n", len); iwl_print_hex_dump(priv, IWL_DL_FW, (u8 *)data, len); - ret = -EINVAL; - } else if (ret) { - IWL_ERR(priv, "TLV %d has invalid size: %u\n", - tlv_type, tlv_len); - iwl_print_hex_dump(priv, IWL_DL_FW, (u8 *)tlv_data, tlv_len); + return -EINVAL; } - return ret; + return 0; + + invalid_tlv_len: + IWL_ERR(priv, "TLV %d has invalid size: %u\n", tlv_type, tlv_len); + iwl_print_hex_dump(priv, IWL_DL_FW, tlv_data, tlv_len); + + return -EINVAL; } /** -- 2.30.2