From 46ba20c1a416d47d975cb96f6a7fef273bcbc63b Mon Sep 17 00:00:00 2001 From: Varun Reddy Yeturu Date: Thu, 17 Aug 2017 15:03:27 -0700 Subject: [PATCH] qcacld-3.0: Add sanity check to avoid len overflow issue in WMI event data In WMI/WMA, data from event buffer from FW is used without sanity checks for upper limit in multiple places. This might lead to a potential integer overflow further leading to buffer corruption Add upper bound checks for max limit of event buffer (1536) in all affected places to prevent the potential integer overflow Change-Id: I30826bb69939bcf02ac850bd2d22ada4795b3c98 CRs-Fixed: 2091584 --- core/wma/src/wma_features.c | 6 ++ core/wma/src/wma_ocb.c | 14 +++++ core/wma/src/wma_scan_roam.c | 96 ++++++++++++++++++++++++++++++++ core/wma/src/wma_utils.c | 104 ++++++++++++++++++++++++++++++++++- 4 files changed, 218 insertions(+), 2 deletions(-) diff --git a/core/wma/src/wma_features.c b/core/wma/src/wma_features.c index 575f2535a056..1ea2ce88ce97 100644 --- a/core/wma/src/wma_features.c +++ b/core/wma/src/wma_features.c @@ -1198,6 +1198,12 @@ int wma_nan_rsp_event_handler(void *handle, uint8_t *event_buf, buf_ptr = (uint8_t *) nan_rsp_event_hdr; alloc_len = sizeof(tSirNanEvent); alloc_len += nan_rsp_event_hdr->data_len; + if (nan_rsp_event_hdr->data_len > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*nan_rsp_event_hdr)) / sizeof(uint8_t))) { + WMA_LOGE("excess data length:%d", nan_rsp_event_hdr->data_len); + QDF_ASSERT(0); + return -EINVAL; + } nan_rsp_event = (tSirNanEvent *) qdf_mem_malloc(alloc_len); if (NULL == nan_rsp_event) { WMA_LOGE("%s: Memory allocation failure", __func__); diff --git a/core/wma/src/wma_ocb.c b/core/wma/src/wma_ocb.c index 40e65dec69ab..71bedfc1a6ef 100644 --- a/core/wma/src/wma_ocb.c +++ b/core/wma/src/wma_ocb.c @@ -511,6 +511,13 @@ static int wma_dcc_get_stats_resp_event_handler(void *handle, fix_param = param_tlvs->fixed_param; /* Allocate and populate the response */ + if (fix_param->num_channels > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*fix_param)) / sizeof(wmi_dcc_ndl_stats_per_channel))) { + WMA_LOGE("%s: too many channels:%d", __func__, + fix_param->num_channels); + QDF_ASSERT(0); + return -EINVAL; + } response = qdf_mem_malloc(sizeof(*response) + fix_param->num_channels * sizeof(wmi_dcc_ndl_stats_per_channel)); if (response == NULL) @@ -651,6 +658,13 @@ static int wma_dcc_stats_event_handler(void *handle, uint8_t *event_buf, param_tlvs = (WMI_DCC_STATS_EVENTID_param_tlvs *)event_buf; fix_param = param_tlvs->fixed_param; /* Allocate and populate the response */ + if (fix_param->num_channels > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*fix_param)) / sizeof(wmi_dcc_ndl_stats_per_channel))) { + WMA_LOGE("%s: too many channels:%d", __func__, + fix_param->num_channels); + QDF_ASSERT(0); + return -EINVAL; + } response = qdf_mem_malloc(sizeof(*response) + fix_param->num_channels * sizeof(wmi_dcc_ndl_stats_per_channel)); if (response == NULL) diff --git a/core/wma/src/wma_scan_roam.c b/core/wma/src/wma_scan_roam.c index fc4ba5115c0e..fcadcbc3f8d7 100644 --- a/core/wma/src/wma_scan_roam.c +++ b/core/wma/src/wma_scan_roam.c @@ -2384,6 +2384,23 @@ int wma_roam_synch_event_handler(void *handle, uint8_t *event, } WMA_LOGI("LFR3: Received WMA_ROAM_OFFLOAD_SYNCH_IND"); + /* + * All below length fields are unsigned and hence positive numbers. + * Maximum number during the addition would be (3 * MAX_LIMIT(UINT32) + + * few fixed fields). + */ + if (sizeof(*synch_event) + synch_event->bcn_probe_rsp_len + + synch_event->reassoc_rsp_len + + synch_event->reassoc_req_len + + sizeof(wmi_channel) + sizeof(wmi_key_material) + + sizeof(uint32_t) > WMI_SVC_MSG_MAX_SIZE) { + WMA_LOGE("excess synch payload: LEN bcn:%d, req:%d, rsp:%d", + synch_event->bcn_probe_rsp_len, + synch_event->reassoc_req_len, + synch_event->reassoc_rsp_len); + goto cleanup_label; + } + cds_host_diag_log_work(&wma->roam_ho_wl, WMA_ROAM_HO_WAKE_LOCK_DURATION, WIFI_POWER_EVENT_WAKELOCK_WOW); @@ -4005,6 +4022,8 @@ int wma_extscan_cached_results_event_handler(void *handle, wmi_extscan_rssi_info *src_rssi; int numap, i, moredata, scan_ids_cnt, buf_len; tpAniSirGlobal pMac = cds_get_context(QDF_MODULE_ID_PE); + uint32_t total_len; + bool excess_data = false; if (!pMac) { WMA_LOGE("%s: Invalid pMac", __func__); @@ -4050,6 +4069,44 @@ int wma_extscan_cached_results_event_handler(void *handle, WMA_LOGI("%s: scan_ids_cnt %d", __func__, scan_ids_cnt); dest_cachelist->num_scan_ids = scan_ids_cnt; + if (event->num_entries_in_page > + (WMI_SVC_MSG_MAX_SIZE - sizeof(*event))/sizeof(*src_hotlist)) { + WMA_LOGE("%s:excess num_entries_in_page %d in WMI event", + __func__, event->num_entries_in_page); + qdf_mem_free(dest_cachelist); + QDF_ASSERT(0); + return -EINVAL; + } else { + total_len = sizeof(*event) + + (event->num_entries_in_page * sizeof(*src_hotlist)); + } + for (i = 0; i < event->num_entries_in_page; i++) { + if (src_hotlist[i].ie_length > WMI_SVC_MSG_MAX_SIZE - + total_len) { + excess_data = true; + break; + } else { + total_len += src_hotlist[i].ie_length; + WMA_LOGD("total len IE: %d", total_len); + } + + if (src_hotlist[i].number_rssi_samples > + (WMI_SVC_MSG_MAX_SIZE - total_len)/sizeof(*src_rssi)) { + excess_data = true; + break; + } else { + total_len += (src_hotlist[i].number_rssi_samples * + sizeof(*src_rssi)); + WMA_LOGD("total len RSSI samples: %d", total_len); + } + } + if (excess_data) { + WMA_LOGE("%s:excess data in WMI event", + __func__); + qdf_mem_free(dest_cachelist); + QDF_ASSERT(0); + return -EINVAL; + } buf_len = sizeof(*dest_result) * scan_ids_cnt; dest_cachelist->result = qdf_mem_malloc(buf_len); if (!dest_cachelist->result) { @@ -4113,6 +4170,8 @@ int wma_extscan_change_results_event_handler(void *handle, int moredata; int rssi_num = 0; tpAniSirGlobal pMac = cds_get_context(QDF_MODULE_ID_PE); + uint32_t buf_len; + bool excess_data = false; if (!pMac) { WMA_LOGE("%s: Invalid pMac", __func__); @@ -4146,6 +4205,31 @@ int wma_extscan_change_results_event_handler(void *handle, } else { moredata = 0; } + + do { + if (event->num_entries_in_page > + (WMI_SVC_MSG_MAX_SIZE - sizeof(*event))/ + sizeof(*src_chglist)) { + excess_data = true; + break; + } else { + buf_len = + sizeof(*event) + (event->num_entries_in_page * + sizeof(*src_chglist)); + } + if (rssi_num > + (WMI_SVC_MSG_MAX_SIZE - buf_len)/sizeof(int32_t)) { + excess_data = true; + break; + } + } while (0); + + if (excess_data) { + WMA_LOGE("buffer len exceeds WMI payload,numap:%d, rssi_num:%d", + numap, rssi_num); + QDF_ASSERT(0); + return -EINVAL; + } dest_chglist = qdf_mem_malloc(sizeof(*dest_chglist) + sizeof(*dest_ap) * numap + sizeof(int32_t) * rssi_num); @@ -4220,6 +4304,18 @@ int wma_passpoint_match_event_handler(void *handle, event = param_buf->fixed_param; buf_ptr = (uint8_t *)param_buf->fixed_param; + /* + * All the below lengths are UINT32 and summing up and checking + * against a constant should not be an issue. + */ + if ((sizeof(*event) + event->ie_length + event->anqp_length) > + WMI_SVC_MSG_MAX_SIZE) { + WMA_LOGE("IE Length: %d or ANQP Length: %d is huge", + event->ie_length, event->anqp_length); + QDF_ASSERT(0); + return -EINVAL; + } + dest_match = qdf_mem_malloc(sizeof(*dest_match) + event->ie_length + event->anqp_length); if (!dest_match) { diff --git a/core/wma/src/wma_utils.c b/core/wma/src/wma_utils.c index 9dff6a6d9260..fb62c723d057 100644 --- a/core/wma/src/wma_utils.c +++ b/core/wma/src/wma_utils.c @@ -453,6 +453,12 @@ int wma_stats_ext_event_handler(void *handle, uint8_t *event_buf, alloc_len = sizeof(tSirStatsExtEvent); alloc_len += stats_ext_info->data_len; + if (stats_ext_info->data_len > (WMI_SVC_MSG_MAX_SIZE - + sizeof(*stats_ext_info))) { + WMA_LOGE("Excess data_len:%d", stats_ext_info->data_len); + QDF_ASSERT(0); + return -EINVAL; + } stats_ext_event = (tSirStatsExtEvent *) qdf_mem_malloc(alloc_len); if (NULL == stats_ext_event) { WMA_LOGE("%s: Memory allocation failure", __func__); @@ -1145,6 +1151,8 @@ static int wma_unified_link_peer_stats_event_handler(void *handle, uint32_t next_res_offset, next_peer_offset, next_rate_offset; size_t peer_info_size, peer_stats_size, rate_stats_size; size_t link_stats_results_size; + bool excess_data = false; + uint32_t buf_len; tpAniSirGlobal pMac = cds_get_context(QDF_MODULE_ID_PE); @@ -1180,6 +1188,33 @@ static int wma_unified_link_peer_stats_event_handler(void *handle, return -EINVAL; } + do { + if (peer_stats->num_rates > + WMI_SVC_MSG_MAX_SIZE/sizeof(wmi_rate_stats)) { + excess_data = true; + break; + } else { + buf_len = + peer_stats->num_rates * sizeof(wmi_rate_stats); + } + if (fixed_param->num_peers > + WMI_SVC_MSG_MAX_SIZE/sizeof(wmi_peer_link_stats)) { + excess_data = true; + break; + } else { + buf_len += fixed_param->num_peers * + sizeof(wmi_peer_link_stats); + } + } while (0); + + if (excess_data || + (sizeof(*fixed_param) > WMI_SVC_MSG_MAX_SIZE - buf_len)) { + WMA_LOGE("excess wmi buffer: rates:%d, peers:%d", + peer_stats->num_rates, fixed_param->num_peers); + QDF_ASSERT(0); + return -EINVAL; + } + /* * num_rates - sum of the rates of all the peers */ @@ -1348,6 +1383,13 @@ static int wma_unified_radio_tx_power_level_stats_event_handler(void *handle, fixed_param->power_level_offset, fixed_param->radio_id); + if (fixed_param->num_tx_power_levels > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*fixed_param)) / sizeof(uint32_t))) { + WMA_LOGE("%s: excess tx_power buffers:%d", __func__, + fixed_param->num_tx_power_levels); + QDF_ASSERT(0); + return -EINVAL; + } rs_results = (tSirWifiRadioStat *) &link_stats_results->results[0] + fixed_param->radio_id; tx_power_level_values = (uint8_t *) param_tlvs->tx_time_per_power_level; @@ -2646,7 +2688,7 @@ int wma_link_status_event_handler(void *handle, uint8_t *cmd_param_info, wmi_vdev_rate_ht_info *ht_info; struct wma_txrx_node *intr = wma->interfaces; uint8_t link_status = LINK_STATUS_LEGACY; - int i; + uint32_t i; param_buf = (WMI_UPDATE_VDEV_RATE_STATS_EVENTID_param_tlvs *) cmd_param_info; @@ -2660,6 +2702,14 @@ int wma_link_status_event_handler(void *handle, uint8_t *cmd_param_info, ht_info = (wmi_vdev_rate_ht_info *) param_buf->ht_info; WMA_LOGD("num_vdev_stats: %d", event->num_vdev_stats); + + if (event->num_vdev_stats > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*event)) / sizeof(*ht_info))) { + WMA_LOGE("%s: excess vdev_stats buffers:%d", __func__, + event->num_vdev_stats); + QDF_ASSERT(0); + return -EINVAL; + } for (i = 0; (i < event->num_vdev_stats) && ht_info; i++) { WMA_LOGD("%s vdevId:%d tx_nss:%d rx_nss:%d tx_preamble:%d rx_preamble:%d", __func__, ht_info->vdevid, ht_info->tx_nss, @@ -2850,7 +2900,10 @@ int wma_stats_event_handler(void *handle, uint8_t *cmd_param_info, wmi_rssi_stats *rssi_stats; wmi_per_chain_rssi_stats *rssi_event; struct wma_txrx_node *node; - uint8_t i, *temp; + uint8_t *temp; + uint32_t i; + uint32_t buf_len = 0; + bool excess_data = false; wmi_congestion_stats *congestion_stats; tpAniSirGlobal mac; @@ -2862,6 +2915,53 @@ int wma_stats_event_handler(void *handle, uint8_t *cmd_param_info, event = param_buf->fixed_param; temp = (uint8_t *) param_buf->data; + do { + if (event->num_pdev_stats > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*event)) / sizeof(*pdev_stats))) { + excess_data = true; + break; + } else { + buf_len += event->num_pdev_stats * sizeof(*pdev_stats); + } + + if (event->num_vdev_stats > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*event)) / sizeof(*vdev_stats))) { + excess_data = true; + break; + } else { + buf_len += event->num_vdev_stats * sizeof(*vdev_stats); + } + + if (event->num_peer_stats > ((WMI_SVC_MSG_MAX_SIZE - + sizeof(*event)) / sizeof(*peer_stats))) { + excess_data = true; + break; + } else { + buf_len += event->num_peer_stats * sizeof(*peer_stats); + } + + rssi_event = + (wmi_per_chain_rssi_stats *) param_buf->chain_stats; + if (rssi_event && (rssi_event->num_per_chain_rssi_stats > + ((WMI_SVC_MSG_MAX_SIZE - sizeof(*event)) / + sizeof(*peer_stats)))) { + excess_data = true; + break; + } else { + buf_len += rssi_event->num_per_chain_rssi_stats * + sizeof(*peer_stats); + } + } while (0); + + if (excess_data || + (sizeof(*event) > WMI_SVC_MSG_MAX_SIZE - buf_len)) { + WMA_LOGE("excess wmi buffer: stats pdev %d vdev %d peer %d", + event->num_pdev_stats, event->num_vdev_stats, + event->num_peer_stats); + QDF_ASSERT(0); + return -EINVAL; + } + if (event->num_pdev_stats > 0) { for (i = 0; i < event->num_pdev_stats; i++) { pdev_stats = (wmi_pdev_stats *) temp;