From c2f9a396f1236a5a5e7bd1c90e32fbcf2ef35367 Mon Sep 17 00:00:00 2001 From: Sudhir Kohalli Date: Mon, 31 Oct 2016 14:49:11 -0700 Subject: [PATCH] net: wireless: bcmdhd: Fix up the BRCM wifi DHD code Security Vulnerability fix for memory overflow wifi driver function wl_cfgvendor_rtt_set_config. In the current fix added check to validate if the target_cnt is valid or not if it is not valid then parse error. Since target_cnt can be controlled by user netlink input which needs to validated at the DHD level. Signed-off-by: Sudhir Kohalli Bug: 32219255 Change-Id: I5cf771c60a6ae8019e5e36571197e2849c572b40 --- drivers/net/wireless/bcmdhd/wl_cfgvendor.c | 62 ++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/drivers/net/wireless/bcmdhd/wl_cfgvendor.c b/drivers/net/wireless/bcmdhd/wl_cfgvendor.c index b536de31010a9..eb83c8339e471 100644 --- a/drivers/net/wireless/bcmdhd/wl_cfgvendor.c +++ b/drivers/net/wireless/bcmdhd/wl_cfgvendor.c @@ -1537,13 +1537,18 @@ wl_cfgvendor_rtt_set_config(struct wiphy *wiphy, struct wireless_dev *wdev, } memset(&rtt_param, 0, sizeof(rtt_param)); + if (len <= 0) { + err = BCME_ERROR; + goto exit; + } nla_for_each_attr(iter, data, len, rem) { type = nla_type(iter); switch (type) { case RTT_ATTRIBUTE_TARGET_CNT: target_cnt = nla_get_u8(iter); - if (rtt_param.rtt_target_cnt > RTT_MAX_TARGET_CNT) { - WL_ERR(("exceed max target count : %d\n", + if ((target_cnt <= 0) || + (target_cnt > RTT_MAX_TARGET_CNT)) { + WL_ERR(("target_cnt is not valid: %d", target_cnt)); err = BCME_RANGE; goto exit; @@ -1557,6 +1562,13 @@ wl_cfgvendor_rtt_set_config(struct wiphy *wiphy, struct wireless_dev *wdev, } break; case RTT_ATTRIBUTE_TARGET_INFO: + /* Added this variable for safe check to avoid crash + * incase the caller did not respect the order + */ + if (!rtt_param.target_info) { + err = BCME_NOMEM; + goto exit; + } rtt_target = rtt_param.target_info; nla_for_each_nested(iter1, iter, rem1) { nla_for_each_nested(iter2, iter1, rem2) { @@ -1677,6 +1689,7 @@ wl_cfgvendor_rtt_set_config(struct wiphy *wiphy, struct wireless_dev *wdev, exit: /* free the target info list */ kfree(rtt_param.target_info); + rtt_param.target_info = NULL; return err; } @@ -1685,46 +1698,63 @@ wl_cfgvendor_rtt_cancel_config(struct wiphy *wiphy, struct wireless_dev *wdev, const void *data, int len) { int err = 0, rem, type, target_cnt = 0; - int target_cnt_chk = 0; + int target_idx = 0; const struct nlattr *iter; - struct ether_addr *mac_list = NULL, *mac_addr = NULL; + struct ether_addr *mac_list = NULL; struct bcm_cfg80211 *cfg = wiphy_priv(wiphy); + if (len <= 0) { + err = -EINVAL; + goto exit; + } nla_for_each_attr(iter, data, len, rem) { type = nla_type(iter); switch (type) { case RTT_ATTRIBUTE_TARGET_CNT: if (mac_list != NULL) { WL_ERR(("mac_list is not NULL\n")); + err = -EINVAL; goto exit; } target_cnt = nla_get_u8(iter); - if (target_cnt > 0) { + if ((target_cnt > 0) && + (target_cnt < RTT_MAX_TARGET_CNT)) { mac_list = (struct ether_addr *)kzalloc(target_cnt * ETHER_ADDR_LEN, GFP_KERNEL); if (mac_list == NULL) { WL_ERR(("failed to allocate mem for mac list\n")); + err = -EINVAL; goto exit; } - mac_addr = &mac_list[0]; } else { /* cancel the current whole RTT process */ goto cancel; } break; case RTT_ATTRIBUTE_TARGET_MAC: - if (mac_addr) { - memcpy(mac_addr++, nla_data(iter), ETHER_ADDR_LEN); - target_cnt_chk++; - if (target_cnt_chk > target_cnt) { - WL_ERR(("over target count\n")); - goto exit; - } - break; - } else { - WL_ERR(("mac_list is NULL\n")); + if (!mac_list) { + err = -EINVAL; goto exit; } + + if (target_idx >= target_cnt) { + err = -EINVAL; + goto exit; + } + + if (nla_len(iter) != ETHER_ADDR_LEN) { + err = -EINVAL; + goto exit; + } + + memcpy(&mac_list[target_idx], nla_data(iter), + ETHER_ADDR_LEN); + target_idx++; + break; + + default: + err = -EINVAL; + goto exit; } } cancel: