Re: [PATCH v2 5/8] net: of_net: Add of_get_nvmem_eui48() helper for EUI-48 lookup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] of_get_mac_address_nvmem() corrupts the output buffer on failure, causing network drivers to use invalid MAC addresses.
--

commit dfaa504d49a935a1aa8c630022d2ce8a6b303cce
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>

net: of_net: Add of_get_nvmem_eui48() helper for EUI-48 lookup

Factor out the common NVMEM EUI-48 retrieval logic from
of_get_mac_address_nvmem() into a new of_get_nvmem_eui48() helper that
accepts the NVMEM cell name as a parameter.

> diff --git a/net/core/of_net.c b/net/core/of_net.c
> index 93ea425b9248a..79b289de0f16a 100644
> --- a/net/core/of_net.c
> +++ b/net/core/of_net.c
[ ... ]
> @@ -75,27 +72,53 @@ int of_get_mac_address_nvmem(struct device_node *np, u8 *addr)
>  		return ret;
>  	}
>  
> -	cell = of_nvmem_cell_get(np, "mac-address");
> +	ret = of_get_nvmem_eui48(np, "mac-address", addr);
> +	if (ret)
> +		return ret;
> +
> +	if (!is_valid_ether_addr(addr))
> +		return -EINVAL;

Does this change inadvertently clobber the caller's output buffer when an
invalid MAC address is read?

Before this patch, of_get_mac_address_nvmem() only modified the output
buffer if the NVMEM-provided MAC address passed is_valid_ether_addr().

Now, it calls of_get_nvmem_eui48() which unconditionally copies any non-zero
6-byte value into the caller's buffer, and only then checks
is_valid_ether_addr(addr). If the MAC is invalid (e.g. multicast or
broadcast), it returns -EINVAL but leaves the invalid MAC in the caller's
buffer.

This can break existing callers that rely on the buffer remaining unmodified
on error. For example, in drivers/net/ethernet/freescale/enetc/enetc_pf_common.c,
enetc_setup_mac_address() initializes the buffer to zeroes, ignores non-defer
errors, and falls back to a hardware or random MAC if the buffer is still
all-zeroes. Because the buffer is clobbered with a non-zero invalid MAC,
the fallback is bypassed and enetc_set_si_hw_addr() programs the hardware
with the invalid MAC.

Similarly, in drivers/net/ethernet/renesas/sh_eth.c, read_mac_address()
checks if the buffer is non-zero to decide whether to read the MAC from
hardware registers. Clobbering the buffer causes the driver to skip the
valid hardware MAC and instead fall back to a random MAC.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507-block-as-nvmem-v2-0-bf17edd5134e@oss.qualcomm.com?part=5