[PATCH v1 1/4] lib/net_utils: Relax NULL-termination requirement on input string

[PATCH v1 1/4] lib/net_utils: Relax NULL-termination requirement on input string

[Andy Shevchenko]

We have not been caring about garbage at the end of input string and this
change doesn't affect it. What we prevent here is possibility to go over
boundaries when input string is not NULL terminated at all.

Allow mac_pton() work on non-NULL terminated strings.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/net_utils.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/net_utils.c b/lib/net_utils.c
index af525353395d..d32c6961fe0f 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -4,12 +4,14 @@
 #include <linux/ctype.h>
 #include <linux/kernel.h>
 
+#define MAC_PTON_MINLEN		(3 * ETH_ALEN - 1)
+
 bool mac_pton(const char *s, u8 *mac)
 {
 	int i;
 
 	/* XX:XX:XX:XX:XX:XX */
-	if (strlen(s) < 3 * ETH_ALEN - 1)
+	if (strnlen(s, MAC_PTON_MINLEN) < MAC_PTON_MINLEN)
 		return false;
 
 	/* Don't dirty result unless string is valid MAC. */
-- 
2.15.1

[PATCH v1 2/4] lib/net_utils: Introduce mac_pton_from_user()

[Andy Shevchenko]

Some drivers are getting MAC from user space. Make a helper for them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/kernel.h |  1 +
 lib/net_utils.c        | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ce51455e2adf..e203b313608d 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -587,6 +587,7 @@ extern int __must_check hex2bin(u8 *dst, const char *src, size_t count);
 extern char *bin2hex(char *dst, const void *src, size_t count);
 
 bool mac_pton(const char *s, u8 *mac);
+int __must_check mac_pton_from_user(const char __user *s, size_t count, u8 *mac);
 
 /*
  * General tracing related utility functions - trace_printk(),
diff --git a/lib/net_utils.c b/lib/net_utils.c
index d32c6961fe0f..7be3483aece6 100644
--- a/lib/net_utils.c
+++ b/lib/net_utils.c
@@ -3,6 +3,7 @@
 #include <linux/if_ether.h>
 #include <linux/ctype.h>
 #include <linux/kernel.h>
+#include <linux/uaccess.h>
 
 #define MAC_PTON_MINLEN		(3 * ETH_ALEN - 1)
 
@@ -27,3 +28,14 @@ bool mac_pton(const char *s, u8 *mac)
 	return true;
 }
 EXPORT_SYMBOL(mac_pton);
+
+int mac_pton_from_user(const char __user *s, size_t count, u8 *mac)
+{
+	char buf[MAC_PTON_MINLEN];
+
+	count = min(count, sizeof(buf));
+	if (copy_from_user(buf, s, count))
+		return -EFAULT;
+	return mac_pton(buf, mac) ? 0 : -EINVAL;
+}
+EXPORT_SYMBOL(mac_pton_from_user);
-- 
2.15.1

Re: [PATCH v1 2/4] lib/net_utils: Introduce mac_pton_from_user()

[Greg Kroah-Hartman]

On Tue, Dec 19, 2017 at 09:14:10PM +0200, Andy Shevchenko wrote:
> Some drivers are getting MAC from user space. Make a helper for them.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/kernel.h |  1 +
>  lib/net_utils.c        | 12 ++++++++++++
>  2 files changed, 13 insertions(+)

Don't do this just for some horrid staging drivers.  They can just drop
that functionality entirely and use the "normal" way of doing this if
they really want it.

thanks,

greg k-h

Re: [PATCH v1 2/4] lib/net_utils: Introduce mac_pton_from_user()

[David Miller]

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Date: Wed, 20 Dec 2017 08:13:55 +0100

> On Tue, Dec 19, 2017 at 09:14:10PM +0200, Andy Shevchenko wrote:
>> Some drivers are getting MAC from user space. Make a helper for them.
>> 
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  include/linux/kernel.h |  1 +
>>  lib/net_utils.c        | 12 ++++++++++++
>>  2 files changed, 13 insertions(+)
> 
> Don't do this just for some horrid staging drivers.  They can just drop
> that functionality entirely and use the "normal" way of doing this if
> they really want it.

Agreed.

[PATCH v1 3/4] staging: rtl8723bs: Replace mac address parsing

[Andy Shevchenko]

Replace copy_from_user() + sscanf() with mac_pton_from_user().

While here, replace memcpy(..., ETH_ALEN) with ether_addr_copy().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/staging/rtl8723bs/core/rtw_debug.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c
index b5dd244fee8f..79ac91a6c3ba 100644
--- a/drivers/staging/rtl8723bs/core/rtw_debug.c
+++ b/drivers/staging/rtl8723bs/core/rtw_debug.c
@@ -380,21 +380,15 @@ ssize_t proc_set_roam_tgt_addr(struct file *file, const char __user *buffer, siz
 {
 	struct net_device *dev = data;
 	struct adapter *adapter = (struct adapter *)rtw_netdev_priv(dev);
-
-	char tmp[32];
 	u8 addr[ETH_ALEN];
+	int ret;
 
-	if (count < 1)
-		return -EFAULT;
-
-	if (buffer && !copy_from_user(tmp, buffer, sizeof(tmp))) {
-
-		int num = sscanf(tmp, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx", addr, addr+1, addr+2, addr+3, addr+4, addr+5);
-		if (num == 6)
-			memcpy(adapter->mlmepriv.roam_tgt_addr, addr, ETH_ALEN);
+	ret = mac_pton_from_user(buffer, count, addr);
+	if (ret)
+		return ret;
 
-		DBG_871X("set roam_tgt_addr to "MAC_FMT"\n", MAC_ARG(adapter->mlmepriv.roam_tgt_addr));
-	}
+	ether_addr_copy(adapter->mlmepriv.roam_tgt_addr, addr);
+	DBG_871X("set roam_tgt_addr to "MAC_FMT"\n", MAC_ARG(adapter->mlmepriv.roam_tgt_addr));
 
 	return count;
 }
-- 
2.15.1

[PATCH v1 4/4] staging: rtl8712: Replace mac address parsing

[Andy Shevchenko]

Replace copy_from_user() + mac_pton() with mac_pton_from_user().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index e30a5be5f318..3f82c39eb071 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1953,7 +1953,7 @@ static int r871x_get_ap_info(struct net_device *dev,
 	struct list_head *plist, *phead;
 	unsigned char *pbuf;
 	u8 bssid[ETH_ALEN];
-	char data[33];
+	int ret;
 
 	if (padapter->bDriverStopped || (pdata == NULL))
 		return -EINVAL;
@@ -1967,9 +1967,11 @@ static int r871x_get_ap_info(struct net_device *dev,
 	pdata->flags = 0;
 	if (pdata->length < 32)
 		return -EINVAL;
-	if (copy_from_user(data, pdata->pointer, 32))
-		return -EINVAL;
-	data[32] = 0;
+	ret = mac_pton_from_user(pdata->pointer, 32, bssid);
+	if (ret)
+		return ret;
+
+	netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
 
 	spin_lock_irqsave(&(pmlmepriv->scanned_queue.lock), irqL);
 	phead = &queue->queue;
@@ -1978,14 +1980,6 @@ static int r871x_get_ap_info(struct net_device *dev,
 		if (end_of_queue_search(phead, plist))
 			break;
 		pnetwork = container_of(plist, struct wlan_network, list);
-		if (!mac_pton(data, bssid)) {
-			netdev_info(dev, "r8712u: Invalid BSSID '%s'.\n",
-				    (u8 *)data);
-			spin_unlock_irqrestore(&(pmlmepriv->scanned_queue.lock),
-					       irqL);
-			return -EINVAL;
-		}
-		netdev_info(dev, "r8712u: BSSID:%pM\n", bssid);
 		if (ether_addr_equal(bssid, pnetwork->network.MacAddress)) {
 			/* BSSID match, then check if supporting wpa/wpa2 */
 			pbuf = r8712_get_wpa_ie(&pnetwork->network.IEs[12],
-- 
2.15.1