netdev - Re: [PATCH] net: ethernet: Fixe issue in nvmem_get_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 for Android: free password hash cracker in your pocket

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

Message-ID: <c18ef0d0-d716-4d04-9a01-59defc8bb56e@lunn.ch>
Date: Thu, 8 May 2025 14:01:28 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ozgur Kara <ozgur@...sey.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Alexei Starovoitov <ast@...nel.org>,
	Daniel Borkmann <daniel@...earbox.net>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] net: ethernet: Fixe issue in nvmem_get_mac_address()
 where invalid mac addresses

On Thu, May 08, 2025 at 02:14:00AM +0000, Ozgur Kara wrote:
> From: Ozgur Karatas <ozgur@...sey.org>
> 
> it's necessary to log error returned from
> fwnode_property_read_u8_array because there is no detailed information
> when addr returns an invalid mac address.
> 
> kfree(mac) should actually be marked as kfree((void *)mac) because mac
> pointer is of type const void * and type conversion is required so
> data returned from nvmem_cell_read() is of same type.

What warning do you see from the compiler?

> @@ -565,11 +565,16 @@ static int fwnode_get_mac_addr(struct
> fwnode_handle *fwnode,
>         int ret;
> 
>         ret = fwnode_property_read_u8_array(fwnode, name, addr, ETH_ALEN);
> -       if (ret)
> +       if (ret) {
> +               pr_err("Failed to read MAC address property %s\n", name);
>                 return ret;
> +        }
> 
> -       if (!is_valid_ether_addr(addr))
> +       if (!is_valid_ether_addr(addr)) {
> +               pr_err("Invalid MAC address read for %s\n", name);
>                 return -EINVAL;
> +        }
> +
>         return 0;
>  }

Look at how it is used:

int of_get_mac_address(struct device_node *np, u8 *addr)
{
	int ret;

	if (!np)
		return -ENODEV;

	ret = of_get_mac_addr(np, "mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "local-mac-address", addr);
	if (!ret)
		return 0;

	ret = of_get_mac_addr(np, "address", addr);
	if (!ret)
		return 0;

	return of_get_mac_address_nvmem(np, addr);
}

We keep trying until we find a MAC address. It is not an error if a
source does not have a MAC address, we just keep going and try the
next.

So you should not print an message if the property does not
exist. Other errors, EIO, EINVAL, etc, are O.K. to print a warning.

    Andrew

---
pw-bot: cr