netdev - Re: [PATCH v5] igb: Assign random MAC address instead of fail in case of invalid one

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] [day] [month] [year] [list]

Message-ID: <9d83c7c5-9699-1e21-ac8c-99ac55a75401@molgen.mpg.de>
Date:   Thu, 9 Jun 2022 13:07:16 +0200
From:   Paul Menzel <pmenzel@...gen.mpg.de>
To:     Lixue Liang <lianglixuehao@....com>
Cc:     anthony.l.nguyen@...el.com, intel-wired-lan@...ts.osuosl.org,
        jesse.brandeburg@...el.com, kuba@...nel.org,
        lianglixue@...atwall.com.cn, netdev@...r.kernel.org
Subject: Re: [PATCH v5] igb: Assign random MAC address instead of fail in case
 of invalid one

Dear Lixue,


Thank you for all the iterations.

Am 09.06.22 um 10:39 schrieb Lixue Liang:
> From: Lixue Liang <lianglixue@...atwall.com.cn>
> 
> In some cases, when the user uses igb_set_eeprom to modify the MAC address
> to be invalid, or an invalid MAC address appears when with uninitialized
> samples, the igb driver will fail to load. If there is no network card
> device, the user could not conveniently modify it to a valid MAC address,
> for example using ethtool to modify.
> 
> Through module parameter to set，when the MAC address is invalid, a random
> valid MAC address can be used to continue loading and output relevant log
> reminders. In this way, users can conveniently correct invalid MAC address.

Maybe:

Add the module parameter `…` to control the behavior. When set to true, 
a random MAC address is assigned, and the driver can be loaded, allowing 
the user to correct the invalid MAC address.

> 
> Signed-off-by: Lixue Liang <lianglixue@...atwall.com.cn>
> ---

[…]

> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 34b33b21e0dc..8162e8999ccb 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -238,8 +238,11 @@ MODULE_LICENSE("GPL v2");
>   
>   #define DEFAULT_MSG_ENABLE (NETIF_MSG_DRV|NETIF_MSG_PROBE|NETIF_MSG_LINK)
>   static int debug = -1;
> +static unsigned int invalid_mac_address_allow;

Make it a boolean?

>   module_param(debug, int, 0);
> +module_param(invalid_mac_address_allow, uint, 0);

Name it `allow_invalid_mac_address`?

>   MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
> +MODULE_PARM_DESC(invalid_mac_address_allow, "Allow NIC driver to be loaded with invalid MAC address");
>   
>   struct igb_reg_info {
>   	u32 ofs;
> @@ -3359,9 +3362,16 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	eth_hw_addr_set(netdev, hw->mac.addr);
>   
>   	if (!is_valid_ether_addr(netdev->dev_addr)) {
> -		dev_err(&pdev->dev, "Invalid MAC Address\n");
> -		err = -EIO;
> -		goto err_eeprom;
> +		if (!invalid_mac_address_allow) {
> +			dev_err(&pdev->dev, "Invalid MAC Address\n");

Correct the spelling in a patch in front of this patch?

> +			err = -EIO;
> +			goto err_eeprom;
> +		} else {
> +			eth_hw_addr_random(netdev);
> +			ether_addr_copy(hw->mac.addr, netdev->dev_addr);
> +			dev_err(&pdev->dev,
> +				"Invalid MAC address. Assigned random MAC address\n");
> +		}
>   	}
>   
>   	igb_set_default_mac_filter(adapter);


Kind regards,

Paul