netdev - Re: [PATCH v1] smsc911x: add second read of EEPROM mac when possible corruption seen

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: <20250901135712.272f72a9@kernel.org>
Date: Mon, 1 Sep 2025 13:57:12 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Colin Foster <colin.foster@...advantage.com>
Cc: linux-kernel@...r.kernel.org, netdev@...r.kernel.org, Steve Glendinning
 <steve.glendinning@...well.net>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH v1] smsc911x: add second read of EEPROM mac when
 possible corruption seen

On Thu, 28 Aug 2025 16:44:52 -0500 Colin Foster wrote:
> When the EEPROM MAC is read by way of ADDRH, it can return all 0s the
> first time. Subsequent reads succeed.
> 
> Re-read the ADDRH when this behaviour is observed, in an attempt to
> correctly apply the EEPROM MAC address.

Please name the device, and FW version if applicable, on which you
observe the issue.

> Signed-off-by: Colin Foster <colin.foster@...advantage.com>
> ---
>  drivers/net/ethernet/smsc/smsc911x.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
> index a2e511912e6a9..63ed221edc00a 100644
> --- a/drivers/net/ethernet/smsc/smsc911x.c
> +++ b/drivers/net/ethernet/smsc/smsc911x.c
> @@ -2162,8 +2162,20 @@ static const struct net_device_ops smsc911x_netdev_ops = {
>  static void smsc911x_read_mac_address(struct net_device *dev)
>  {
>  	struct smsc911x_data *pdata = netdev_priv(dev);
> -	u32 mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> -	u32 mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> +	u32 mac_high16, mac_low32;
> +
> +	mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> +	mac_low32 = smsc911x_mac_read(pdata, ADDRL);
> +
> +	/*

nit: netdev multi-line comment style doesn't place /* on a separate
line:
	

> +	 * The first mac_read always returns 0. Re-read it to get the
> +	 * full MAC

Always? Strange, why did nobody notice until now?

> +	 */
> +	if (mac_high16 == 0) {
> +		SMSC_TRACE(pdata, probe, "Re-read MAC ADDRH\n");
> +		mac_high16 = smsc911x_mac_read(pdata, ADDRH);
> +	}

> 	u8 addr[ETH_ALEN];

Please don't add code in the middle of variable declarations
-- 
pw-bot: cr