[PATCH] rndis_host: Set random MAC for ZTE MF910

[PATCH] rndis_host: Set random MAC for ZTE MF910

[Kristian Evensen]

From: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

All ZTE MF910 mifis, at least on some revisions, export the same MAC
address (36:4b:50:b7:ef:da). Check for this MAC address and set a random
MAC if detected.

Also, changed the memcpy() to ether_addr_copy(), as pointed out by
checkpatch.

Signed-off-by: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/net/usb/rndis_host.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
index 524a47a281..85bdbdf 100644
--- a/drivers/net/usb/rndis_host.c
+++ b/drivers/net/usb/rndis_host.c
@@ -295,6 +295,9 @@ static const struct net_device_ops rndis_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 };
 
+/* well-known buggy ZTE MF910 MAC address */
+static const u8 buggy_zte_addr[ETH_ALEN] = {0x36, 0x4b, 0x50, 0xb7, 0xef, 0xda};
+
 int
 generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
 {
@@ -428,7 +431,11 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
 		dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval);
 		goto halt_fail_and_release;
 	}
-	memcpy(net->dev_addr, bp, ETH_ALEN);
+
+	if (ether_addr_equal(bp, buggy_zte_addr))
+		eth_hw_addr_random(net);
+	else
+		ether_addr_copy(net->dev_addr, bp);
 
 	/* set a nonzero filter to enable data transfers */
 	memset(u.set, 0, sizeof *u.set);
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rndis_host: Set random MAC for ZTE MF910

[Bjørn Mork]

Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> From: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> All ZTE MF910 mifis, at least on some revisions, export the same MAC
> address (36:4b:50:b7:ef:da). Check for this MAC address and set a random
> MAC if detected.
>
> Also, changed the memcpy() to ether_addr_copy(), as pointed out by
> checkpatch.
>
> Signed-off-by: Kristian Evensen <kristian.evensen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/net/usb/rndis_host.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index 524a47a281..85bdbdf 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -295,6 +295,9 @@ static const struct net_device_ops rndis_netdev_ops = {
>  	.ndo_validate_addr	= eth_validate_addr,
>  };
>  
> +/* well-known buggy ZTE MF910 MAC address */
> +static const u8 buggy_zte_addr[ETH_ALEN] = {0x36, 0x4b, 0x50, 0xb7, 0xef, 0xda};
> +
>  int
>  generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
>  {
> @@ -428,7 +431,11 @@ generic_rndis_bind(struct usbnet *dev, struct usb_interface *intf, int flags)
>  		dev_err(&intf->dev, "rndis get ethaddr, %d\n", retval);
>  		goto halt_fail_and_release;
>  	}
> -	memcpy(net->dev_addr, bp, ETH_ALEN);
> +
> +	if (ether_addr_equal(bp, buggy_zte_addr))
> +		eth_hw_addr_random(net);
> +	else
> +		ether_addr_copy(net->dev_addr, bp);
>  
>  	/* set a nonzero filter to enable data transfers */
>  	memset(u.set, 0, sizeof *u.set);


Or how about the more generic?:

        if (bp[0] & 0x02)
   		eth_hw_addr_random(net);
	else
		ether_addr_copy(net->dev_addr, bp);

That would catch similar screwups from other vendors too.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rndis_host: Set random MAC for ZTE MF910

[Kristian Evensen]

Hi Bjørn,

On Thu, Jul 14, 2016 at 12:23 AM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>
> Or how about the more generic?:
>
>         if (bp[0] & 0x02)
>                 eth_hw_addr_random(net);
>         else
>                 ether_addr_copy(net->dev_addr, bp);
>
> That would catch similar screwups from other vendors too.

Great idea, thanks. After submitting the patch I found some other
devices with a similar bug, and there are probably even more out
there. I will update patch and resubmit.

-Kristian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rndis_host: Set random MAC for ZTE MF910

[Kristian Evensen]

On Thu, Jul 14, 2016 at 9:54 AM, Kristian Evensen
<kristian.evensen@gmail.com> wrote:
> Hi Bjørn,
>
> On Thu, Jul 14, 2016 at 12:23 AM, Bjørn Mork <bjorn@mork.no> wrote:
>>
>> Or how about the more generic?:
>>
>>         if (bp[0] & 0x02)
>>                 eth_hw_addr_random(net);
>>         else
>>                 ether_addr_copy(net->dev_addr, bp);
>>
>> That would catch similar screwups from other vendors too.
>
> Great idea, thanks. After submitting the patch I found some other
> devices with a similar bug, and there are probably even more out
> there. I will update patch and resubmit.

Oh, and I forgot to say, please ignore this patch. I will change the
title and description to better describe the functionality.

-Kristian

RE: [PATCH] rndis_host: Set random MAC for ZTE MF910

[David Laight]

From: Bjørn Mork
> Sent: 13 July 2016 23:23
...
> Or how about the more generic?:
> 
>         if (bp[0] & 0x02)
>    		eth_hw_addr_random(net);
> 	else
> 		ether_addr_copy(net->dev_addr, bp);
> 
> That would catch similar screwups from other vendors too.

Not really, that disables 'locally administered' addresses. If a vendor
has used the same address on lots of cards it could easily be a 'real' address.

Not only that, there certainly used to be manufacturers that used 'locally
administered' addresses on all their cards (as well as those that used unallocated
address blocks).
Not to mention the bit-revered addresses....

	David

Re: [PATCH] rndis_host: Set random MAC for ZTE MF910

[Bjørn Mork]

David Laight <David.Laight@ACULAB.COM> writes:
> From: Bjørn Mork
>> Sent: 13 July 2016 23:23
> ...
>> Or how about the more generic?:
>> 
>>         if (bp[0] & 0x02)
>>    		eth_hw_addr_random(net);
>> 	else
>> 		ether_addr_copy(net->dev_addr, bp);
>> 
>> That would catch similar screwups from other vendors too.
>
> Not really, that disables 'locally administered' addresses.

... when the 'locally administered' addresses comes from firmeare, yes.
That was the idea.  We are better off using our own random locally
administered address if some vendor has been cheap/stupid enough to
program that into firmware.

The aminstrator is of course still free to set any address, 'locally
administered' or whatever.  This is not the question here.

> If a vendor has used the same address on lots of cards it could easily
> be a 'real' address.

Sure.  We cannot easily detect that.  The only way is to keep a
blacklist of such  'real' addresses, the way Kristian initially
proposed.

But I thought that we could simplify this particular screwup since the
address in question had the local bit set, and catch every other similar
abuse at the same time. If you get the local bit from formware, then you
know for sure that there is something wrong.

> Not only that, there certainly used to be manufacturers that used 'locally
> administered' addresses on all their cards (as well as those that used unallocated
> address blocks).

Sure. But is there any reason to care about those addresses?

> Not to mention the bit-revered addresses....

Listing all the ways vendors have screwed is going to be a long and
rather boring thread ;)


Bjørn

Re: [PATCH] rndis_host: Set random MAC for ZTE MF910

[David Miller]

From: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Date: Fri, 15 Jul 2016 19:42:28 +0200

> David Laight <David.Laight-ZS65k/vG3HxXrIkS9f7CXA@public.gmane.org> writes:
>> From: Bjørn Mork
>> Not only that, there certainly used to be manufacturers that used 'locally
>> administered' addresses on all their cards (as well as those that used unallocated
>> address blocks).
> 
> Sure. But is there any reason to care about those addresses?

And for that case, this patch we are discussing would be a bug fix.

I'm going to apply the "bp[0] & 0x02" patch.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html