Hi Stefan,
On 04/28/2015 01:51 PM, Stefan Agner wrote:
MAC addresses can be written without leading zeros. A popular
example is libc's ether_ntoa_r function which creates such
MAC addresses.
Example:
00:14:3d:0f:ff:fe can be written as 0:14:3d:f:ff:fe
Additionally, get_ether_addr potentially parsed past the end
of the user provided string. Use the opportunity and fix the
function to never parse beyond the end of the string while
allowing MAC addresses with and without leading zeros.
Signed-off-by: Stefan Agner <stefan@xxxxxxxx>
---
drivers/usb/gadget/function/u_ether.c | 41 +++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index f1fd777..9994645 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -705,23 +705,27 @@ static int eth_stop(struct net_device *net)
static int get_ether_addr(const char *str, u8 *dev_addr)
{
- if (str) {
- unsigned i;
+ int num, i = 0;
+ char c = *str;
- for (i = 0; i < 6; i++) {
- unsigned char num;
-
- if ((*str == '.') || (*str == ':'))
- str++;
- num = hex_to_bin(*str++) << 4;
- num |= hex_to_bin(*str++);
- dev_addr [i] = num;
+ dev_addr[i] = 0;
+ while (c && i < ETH_ALEN) {
+ if (c == '.' || c == ':') {
+ dev_addr[++i] = 0;
+ } else {
+ num = hex_to_bin(c);
+ if (num < 0)
+ break;
+ dev_addr[i] <<= 4;
+ dev_addr[i] |= num;
}
- if (is_valid_ether_addr(dev_addr))
- return 0;
+ c = *++str;
}
- eth_random_addr(dev_addr);
- return 1;
+
+ if ((i + 1) != ETH_ALEN || !is_valid_ether_addr(dev_addr))
+ return -EINVAL;
+
+ return 0;
}
First of all thank you for your remark about reading past the buffer in
my version of this patch. I tried to make my change as little as
possible and didn't catch that this function is broken from beginning.
This one is definitely better in this case but I'm afraid that it won't
be able to correctly interpret MAC address written without separators
while original version works fine for it.
I'm also not sure if it is a good idea to remove random address
generation from this function. Please see my remark below.
static int get_ether_addr_str(u8 dev_addr[ETH_ALEN], char *str, int len)
@@ -786,12 +790,17 @@ struct eth_dev *gether_setup_name(struct usb_gadget *g,
dev->qmult = qmult;
snprintf(net->name, sizeof(net->name), "%s%%d", netname);
- if (get_ether_addr(dev_addr, net->dev_addr))
+ if (get_ether_addr(dev_addr, net->dev_addr)) {
+ eth_random_addr(net->dev_addr);
dev_warn(&g->dev,
"using random %s ethernet address\n", "self");
- if (get_ether_addr(host_addr, dev->host_mac))
+ }
+
+ if (get_ether_addr(host_addr, dev->host_mac)) {
+ eth_random_addr(dev->host_mac);
dev_warn(&g->dev,
"using random %s ethernet address\n", "host");
+ }
if (ethaddr)
memcpy(ethaddr, dev->host_mac, ETH_ALEN);
This function is also called in a few other places. Is there some reason
why you didn't change it? For example in gether_set_dev_addr() which is
used in many gadget modules.
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
