netdev - Re: [RFC] usbnet: assign unique random MAC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231116105146.50d2ae66@hermes.local>
Date: Thu, 16 Nov 2023 10:51:46 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Oliver Neukum <oneukum@...e.com>
Cc: bjorn@...k.no, netdev@...r.kernel.org
Subject: Re: [RFC] usbnet: assign unique random MAC

On Thu, 16 Nov 2023 15:05:52 +0100
Oliver Neukum <oneukum@...e.com> wrote:

> The old method had the bug of issuing the same
> random MAC over and over even to every device.
> This bug is as old as the driver.
> 
> This new method generates each device whose minidriver
> does not provide its own MAC its own unique random
> MAC.
> 
> Signed-off-by: Oliver Neukum <oneukum@...e.com>
> ---
>  drivers/net/usb/usbnet.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 2d14b0d78541..37e3bb2170bc 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -61,9 +61,6 @@
>  
>  /*-------------------------------------------------------------------------*/
>  
> -// randomly generated ethernet address
> -static u8	node_id [ETH_ALEN];
> -
>  /* use ethtool to change the level for any given device */
>  static int msg_level = -1;
>  module_param (msg_level, int, 0);
> @@ -1731,7 +1728,6 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
>  
>  	dev->net = net;
>  	strscpy(net->name, "usb%d", sizeof(net->name));
> -	eth_hw_addr_set(net, node_id);
>  
>  	/* rx and tx sides can use different message sizes;
>  	 * bind() should set rx_urb_size in that case.
> @@ -1805,9 +1801,13 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
>  		goto out4;
>  	}
>  
> -	/* let userspace know we have a random address */
> -	if (ether_addr_equal(net->dev_addr, node_id))
> -		net->addr_assign_type = NET_ADDR_RANDOM;
> +	/*
> +	 * if the device does not come with a MAC
> +	 * we ask the network core to generate us one
> +	 * and flag the device accordingly
> +	 */
> +	if (!is_valid_ether_addr(net->dev_addr))
> +			eth_hw_addr_random(net);
>  
>  	if ((dev->driver_info->flags & FLAG_WLAN) != 0)
>  		SET_NETDEV_DEVTYPE(net, &wlan_type);
> @@ -2217,7 +2217,6 @@ static int __init usbnet_init(void)
>  	BUILD_BUG_ON(
>  		sizeof_field(struct sk_buff, cb) < sizeof(struct skb_data));
>  
> -	eth_random_addr(node_id);
>  	return 0;
>  }
>  module_init(usbnet_init);

The code part looks fine. Fix the style issues though.

 $ ./scripts/checkpatch.pl /tmp/mac.mbox 
WARNING: networking block comments don't use an empty /* line, use /* Comment...
#163: FILE: drivers/net/usb/usbnet.c:1805:
+	/*
+	 * if the device does not come with a MAC

WARNING: suspect code indent for conditional statements (8, 24)
#167: FILE: drivers/net/usb/usbnet.c:1809:
+	if (!is_valid_ether_addr(net->dev_addr))
+			eth_hw_addr_random(net);

total: 0 errors, 2 warnings, 0 checks, 39 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/mac.mbox has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.