From f0e1c2d49989faee432b5fdeaa5c82e01092508a Mon Sep 17 00:00:00 2001 From: Sarada Prasanna Garnayak Date: Fri, 1 Dec 2017 15:29:54 +0530 Subject: [PATCH] wcnss: fix the potential buffer overflow in wlan ctrl data process Validate the userspace wlan control cmd data, info and length before copy into wcnss driver buffer. Avoid unnecessary string manipulation and use kernel defined format specifier to print wlan MAC address. CRs-Fixed: 2149331 Change-Id: Ib59fdcc0e6b84cdd73972dcb62b2c05e4741f5f7 Signed-off-by: Yuanyuan Liu Signed-off-by: Sarada Prasanna Garnayak --- drivers/net/wireless/wcnss/wcnss_wlan.c | 91 +++++++++++-------------- 1 file changed, 41 insertions(+), 50 deletions(-) diff --git a/drivers/net/wireless/wcnss/wcnss_wlan.c b/drivers/net/wireless/wcnss/wcnss_wlan.c index b604f61e1a05..04d6b2e6fec1 100644 --- a/drivers/net/wireless/wcnss/wcnss_wlan.c +++ b/drivers/net/wireless/wcnss/wcnss_wlan.c @@ -203,6 +203,7 @@ static DEFINE_SPINLOCK(reg_spinlock); #define WCNSS_MAX_BUILD_VER_LEN 256 #define WCNSS_MAX_CMD_LEN (128) #define WCNSS_MIN_CMD_LEN (3) +#define WCNSS_CMD_INFO_LEN 2 /* control messages from userspace */ #define WCNSS_USR_CTRL_MSG_START 0x00000000 @@ -210,7 +211,6 @@ static DEFINE_SPINLOCK(reg_spinlock); #define WCNSS_USR_WLAN_MAC_ADDR (WCNSS_USR_CTRL_MSG_START + 3) #define MAC_ADDRESS_STR "%02x:%02x:%02x:%02x:%02x:%02x" -#define SHOW_MAC_ADDRESS_STR "%02x:%02x:%02x:%02x:%02x:%02x\n" #define WCNSS_USER_MAC_ADDR_LENGTH 18 /* message types */ @@ -473,11 +473,7 @@ static ssize_t wcnss_wlan_macaddr_store(struct device *dev, (char *)&macAddr[index], sizeof(char)); } - pr_info("%s: Write MAC Addr:" MAC_ADDRESS_STR "\n", __func__, - penv->wlan_nv_macAddr[0], penv->wlan_nv_macAddr[1], - penv->wlan_nv_macAddr[2], penv->wlan_nv_macAddr[3], - penv->wlan_nv_macAddr[4], penv->wlan_nv_macAddr[5]); - + pr_info("%s: Write MAC Addr: %pM\n", __func__, penv->wlan_nv_macAddr); return count; } @@ -487,10 +483,7 @@ static ssize_t wcnss_wlan_macaddr_show(struct device *dev, if (!penv) return -ENODEV; - return scnprintf(buf, PAGE_SIZE, SHOW_MAC_ADDRESS_STR, - penv->wlan_nv_macAddr[0], penv->wlan_nv_macAddr[1], - penv->wlan_nv_macAddr[2], penv->wlan_nv_macAddr[3], - penv->wlan_nv_macAddr[4], penv->wlan_nv_macAddr[5]); + return scnprintf(buf, PAGE_SIZE, "%pM\n", penv->wlan_nv_macAddr); } static DEVICE_ATTR(wcnss_mac_addr, S_IRUSR | S_IWUSR, @@ -1653,10 +1646,8 @@ int wcnss_get_wlan_mac_address(char mac_addr[WLAN_MAC_ADDR_SIZE]) return -ENODEV; memcpy(mac_addr, penv->wlan_nv_macAddr, WLAN_MAC_ADDR_SIZE); - pr_debug("%s: Get MAC Addr:" MAC_ADDRESS_STR "\n", __func__, - penv->wlan_nv_macAddr[0], penv->wlan_nv_macAddr[1], - penv->wlan_nv_macAddr[2], penv->wlan_nv_macAddr[3], - penv->wlan_nv_macAddr[4], penv->wlan_nv_macAddr[5]); + pr_debug("%s: Get MAC Addr: %pM\n", __func__, penv->wlan_nv_macAddr); + return 0; } EXPORT_SYMBOL(wcnss_get_wlan_mac_address); @@ -2625,57 +2616,57 @@ static int wcnss_ctrl_open(struct inode *inode, struct file *file) return rc; } - -void process_usr_ctrl_cmd(u8 *buf, size_t len) -{ - u16 cmd = buf[0] << 8 | buf[1]; - - switch (cmd) { - - case WCNSS_USR_HAS_CAL_DATA: - if (1 < buf[2]) - pr_err("%s: Invalid data for cal %d\n", __func__, - buf[2]); - has_calibrated_data = buf[2]; - break; - - case WCNSS_USR_WLAN_MAC_ADDR: - memcpy(&penv->wlan_nv_macAddr, &buf[2], - sizeof(penv->wlan_nv_macAddr)); - - pr_debug("%s: MAC Addr:" MAC_ADDRESS_STR "\n", __func__, - penv->wlan_nv_macAddr[0], penv->wlan_nv_macAddr[1], - penv->wlan_nv_macAddr[2], penv->wlan_nv_macAddr[3], - penv->wlan_nv_macAddr[4], penv->wlan_nv_macAddr[5]); - break; - - default: - pr_err("%s: Invalid command %d\n", __func__, cmd); - break; - } -} - static ssize_t wcnss_ctrl_write(struct file *fp, const char __user *user_buffer, size_t count, loff_t *position) { int rc = 0; + u16 cmd; u8 buf[WCNSS_MAX_CMD_LEN]; - if (!penv || !penv->ctrl_device_opened || WCNSS_MAX_CMD_LEN < count - || WCNSS_MIN_CMD_LEN > count) + if (!penv || !penv->ctrl_device_opened || + WCNSS_MAX_CMD_LEN < count || WCNSS_MIN_CMD_LEN > count) return -EFAULT; mutex_lock(&penv->ctrl_lock); rc = copy_from_user(buf, user_buffer, count); - if (0 == rc) - process_usr_ctrl_cmd(buf, count); + if (rc) { + pr_err("%s: Failed to copy ctrl data\n", __func__); + goto exit; + } + cmd = buf[0] << 8 | buf[1]; + switch (cmd) { + case WCNSS_USR_HAS_CAL_DATA: + if (buf[2] > 1) { + pr_err("%s: Invalid cal data %d\n", __func__, buf[2]); + rc = -EINVAL; + goto exit; + } + has_calibrated_data = buf[2]; + break; + + case WCNSS_USR_WLAN_MAC_ADDR: + if ((count - WCNSS_CMD_INFO_LEN) != WLAN_MAC_ADDR_SIZE) { + pr_err("%s: Invalid Mac addr %d\n", __func__, buf[2]); + rc = -EINVAL; + goto exit; + } + + memcpy(&penv->wlan_nv_macAddr, &buf[2], + sizeof(penv->wlan_nv_macAddr)); + pr_debug("%s:MAC Addr: %pM\n", __func__, penv->wlan_nv_macAddr); + break; + default: + pr_err("%s: Invalid command %d\n", __func__, cmd); + rc = -EINVAL; + break; + } + +exit: mutex_unlock(&penv->ctrl_lock); - return rc; } - static const struct file_operations wcnss_ctrl_fops = { .owner = THIS_MODULE, .open = wcnss_ctrl_open,