Re: [PATCH net-next v9 6/6] net: airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The MAC address MSB validation introduces a configuration deadlock, permanently preventing MAC address MSB updates when multiple interfaces are registered.
- [High] Unconditional overwrite of `ref_addr` in the interface evaluation loop breaks MSB validation and corrupts hardware L2 bounds.
--

commit ca5c4a90447cc1f9bdc3a2831da1cfe304652070
Author: Lorenzo Bianconi <lorenzo@kernel.org>

net: airoha: Support multiple LAN/WAN interfaces for hw MAC address configuration

The EN7581 and AN7583 SoCs provide registers to configure hardware LAN/WAN
MAC addresses. This commit extends the logic to derive the correct minimum
and maximum values for MAC LSB bounds when multiple interfaces are configured.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 64ee526da2414..8f2608293bb72 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -71,20 +71,76 @@ static void airoha_qdma_irq_disable(struct airoha_irq_bank *irq_bank,
[ ... ]
> -static void airoha_set_macaddr(struct airoha_gdm_dev *dev, const u8 *addr)
> +static int airoha_set_macaddr(struct airoha_gdm_dev *dev, const u8 *addr)
>  {
> +	u8 ref_addr[ETH_ALEN] __aligned(2);
>  	struct airoha_eth *eth = dev->eth;
> -	u32 val, reg;
> +	u32 reg, val, lmin, lmax;
> +	int i;
> +
> +	eth_zero_addr(ref_addr);
> +	lmin = (addr[3] << 16) | (addr[4] << 8) | addr[5];
> +	lmax = lmin;
> +
> +	for (i = 0; i < ARRAY_SIZE(eth->ports); i++) {
> +		struct airoha_gdm_port *port = eth->ports[i];
> +		int j;
> +
> +		if (!port)
> +			continue;
> +
> +		for (j = 0; j < ARRAY_SIZE(port->devs); j++) {
> +			struct airoha_gdm_dev *iter_dev;
> +			struct net_device *netdev;
[ ... ]
> +			if (netdev->reg_state != NETREG_REGISTERED)
> +				continue;
> +
> +			ether_addr_copy(ref_addr, netdev->dev_addr);
> +			val = (netdev->dev_addr[3] << 16) |
> +			      (netdev->dev_addr[4] << 8) | netdev->dev_addr[5];
> +			if (val < lmin)
> +				lmin = val;
> +			if (val > lmax)
> +				lmax = val;
> +		}
> +	}

[Severity: High]
Does this loop unconditionally overwrite ref_addr with the last evaluated
interface? If there are multiple interfaces with mismatched MSBs (e.g., from
random boot MACs), it seems ref_addr will only retain the MSB of the final
interface.

Could this cause the validation check below to incorrectly pass if the new
MAC matches the last interface's MSB but not the earlier ones, while
erroneously accumulating LSBs from mismatched interfaces into lmin and
lmax?

> +
> +	if (!is_zero_ether_addr(ref_addr) && memcmp(ref_addr, addr, 3)) {

[Severity: High]
Can this validation check create a configuration deadlock?

Since the driver requires all LAN or WAN interfaces to share the same
MAC address 3-byte MSB, and userspace APIs configure interfaces sequentially
via ndo_set_mac_address, changing the first interface's MSB will always fail
this check against the remaining interfaces.

How can a user successfully configure a unified custom MAC MSB across all
devices if the interfaces are assigned random MAC addresses at boot with
different MSBs?

> +		/* According to the HW design, hw mac address MSBs must be
> +		 * the same for each net_device with the same LAN/WAN
> +		 * configuration.
> +		 */
> +		struct net_device *netdev = netdev_from_priv(dev);
> +
> +		dev_warn(eth->dev,
> +			 "%s: wrong mac addr, MSBs must be %02x:%02x:%02x\n",
> +			 netdev->name, ref_addr[0], ref_addr[1],
> +			 ref_addr[2]);
> +		dev_warn(eth->dev, "FE hw forwarding won't work properly\n");
> +
> +		return -EINVAL;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-airoha-eth-multi-serdes-v9-0-5d476bc2f426@kernel.org?part=6