Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1442628 > unrolled thread

[PATCH] rndis_host: Set random MAC for ZTE MF910

Back to article view | Back to linux.kernel

Contents

  [PATCH] rndis_host: Set random MAC for ZTE MF910 Kristian Evensen <kristian.evensen@gmail.com> - 2016-07-13 18:50 +0200
    Re: [PATCH] rndis_host: Set random MAC for ZTE MF910 Bjørn Mork <bjorn@mork.no> - 2016-07-14 00:30 +0200
      Re: [PATCH] rndis_host: Set random MAC for ZTE MF910 Kristian Evensen <kristian.evensen@gmail.com> - 2016-07-14 10:00 +0200
        Re: [PATCH] rndis_host: Set random MAC for ZTE MF910 Kristian Evensen <kristian.evensen@gmail.com> - 2016-07-14 10:10 +0200
      RE: [PATCH] rndis_host: Set random MAC for ZTE MF910 David Laight <David.Laight@ACULAB.COM> - 2016-07-15 18:50 +0200
        Re: [PATCH] rndis_host: Set random MAC for ZTE MF910 Bjørn Mork <bjorn@mork.no> - 2016-07-15 19:50 +0200
          Re: [PATCH] rndis_host: Set random MAC for ZTE MF910 David Miller <davem@davemloft.net> - 2016-07-17 05:10 +0200

#1442628 — [PATCH] rndis_host: Set random MAC for ZTE MF910

From: Kristian Evensen <kristian.evensen@gmail.com>

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@gmail.com>
---
 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

[toc] | [next] | [standalone]

#1442920

Kristian Evensen <kristian.evensen@gmail.com> writes:

> From: Kristian Evensen <kristian.evensen@gmail.com>
>
> 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@gmail.com>
> ---
>  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

[toc] | [prev] | [next] | [standalone]

#1443149

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.

-Kristian

[toc] | [prev] | [next] | [standalone]

#1443152

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

[toc] | [prev] | [next] | [standalone]

#1444440

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

[toc] | [prev] | [next] | [standalone]

#1444481

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

[toc] | [prev] | [next] | [standalone]

#1445004

From: Bjørn Mork <bjorn@mork.no>
Date: Fri, 15 Jul 2016 19:42:28 +0200

> David Laight <David.Laight@ACULAB.COM> 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web