Re: [PATCH v4 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] The refactored `of_get_mac_address_nvmem()` allocates `u8 mac[ETH_ALEN]` on the stack without an explicit alignment attribute, but passes it to `is_valid_ether_addr()` and `ether_addr_copy()`, which explicitly require 2-byte alignment. Additionally, it replaces `memcpy()` with `ether_addr_copy()` for the output buffer `addr`, which places a new 16-bit alignment requirement on the caller-provided destination buffer. On architectures where `CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS` is disabled, passing unaligned buffers to these inline functions triggers hardware alignment faults (kernel panics).
--

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

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

This commit factors out the common NVMEM EUI-48 retrieval logic from
of_get_mac_address_nvmem() into a new of_get_nvmem_eui48() helper. It
updates of_get_mac_address_nvmem() to use this new function to preserve
the existing behavior while making the logic reusable.

> diff --git a/net/core/of_net.c b/net/core/of_net.c
> --- a/net/core/of_net.c
> +++ b/net/core/of_net.c
> @@ -61,9 +61,7 @@ static int of_get_mac_addr(struct device_node *np, const char *name, u8 *addr)
>  int of_get_mac_address_nvmem(struct device_node *np, u8 *addr)
>  {
>  	struct platform_device *pdev = of_find_device_by_node(np);
> -	struct nvmem_cell *cell;
> -	const void *mac;
> -	size_t len;
> +	u8 mac[ETH_ALEN];

[Severity: High]
Does this stack-allocated array need an explicit alignment attribute?

Because it is a u8 array, it only guarantees 1-byte alignment.

>  	int ret;
>  
[ ... ]
> -	mac = nvmem_cell_read(cell, &len);
> -	nvmem_cell_put(cell);
>  
> -	if (IS_ERR(mac))
> -		return PTR_ERR(mac);
> +	ret = of_get_nvmem_eui48(np, "mac-address", mac);
> +	if (ret)
> +		return ret;
> +
> +	if (!is_valid_ether_addr(mac))

[Severity: High]
Could this cause an alignment trap on architectures without efficient
unaligned access?

is_valid_ether_addr() expects its argument to be 2-byte aligned.
The previous implementation used the pointer returned by nvmem_cell_read(),
which was allocated via kzalloc() and naturally provided an 8-byte alignment
guarantee.

> +		return -EINVAL;
> +
> +	ether_addr_copy(addr, mac);

[Severity: High]
Can this ether_addr_copy() call cause an alignment fault? 

ether_addr_copy() requires both the source and destination pointers to be
2-byte aligned. This replaces the safe memcpy() used in the previous
implementation and introduces a new 2-byte alignment requirement for the
caller-provided addr buffer, as well as the locally allocated mac array.

> +	return 0;
>  }
>  EXPORT_SYMBOL(of_get_mac_address_nvmem);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-block-as-nvmem-v4-0-45712e6b22c6@oss.qualcomm.com?part=5