Re: [PATCH net-next v22 4/9] mfd: an8855: Add support for Airoha AN8855 Switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mfd: airoha-an8855: use-after-free of phy_np fwnode handle
- [Medium] mfd: airoha-an8855: memory leak of mdio_np on error path
--

commit 1917f4cbbcfe1c5b1f8816f88eed9bb439f23478
Author: Christian Marangi <ansuelsmth@gmail.com>

mfd: an8855: Add support for Airoha AN8855 Switch

This commit adds support for the Airoha AN8855 switch, which provides a DSA 
switch and an NVMEM provider. It introduces MDIO-based regmaps for each 
internal PHY, allowing them to act as both standard PHYs and access points 
for the switch fabric.

> diff --git a/drivers/mfd/airoha-an8855.c b/drivers/mfd/airoha-an8855.c
> new file mode 100644
> index 0000000000000..2b3b5f4a5a21e
> --- /dev/null
> +++ b/drivers/mfd/airoha-an8855.c
[ ... ]
> +static int an8855_phy_bus_register(struct device *dev, struct an8855_core_priv *priv,
> +				   struct device_node *phy_np,
> +				   struct an8855_phy_bus *phy_bus_info)
> +{
[ ... ]
> +	phy_bus_info->bus = bus;
> +	phy_bus_info->addr = addr;
> +	phy_bus_info->node = of_fwnode_handle(phy_np);

[Severity: High]
Does this leave the fwnode handle without an active reference?

The device node phy_np is supplied by a scoped iterator in
an8855_mdio_register(), which automatically drops the reference on each
iteration or loop exit.

Because of_node_get() is not used here to acquire an additional reference, the
node is saved without incrementing the reference count. This can lead to a
use-after-free later in an8855_core_probe() when the stale fwnode pointer is
passed to fwnode_mdiobus_register_phy().

> +
> +	return 0;
> +}
> +
> +static int an8855_mdio_register(struct device *dev, struct an8855_core_priv *priv,
> +				struct an8855_phy_bus *phy_bus_infos)
> +{
> +	struct device_node *mdio_np;
> +	int ret = 0;
> +	int i = 0;
> +
> +	mdio_np = of_get_child_by_name(dev->of_node, "mdio");
> +	if (!mdio_np)
> +		return -ENODEV;
> +
> +	for_each_available_child_of_node_scoped(mdio_np, phy_np) {
> +		if (i >= AN8855_MAX_PHY_PORT)
> +			return -EINVAL;

[Severity: Medium]
Does this early return leak the mdio_np reference?

When returning -EINVAL directly, the function bypasses the of_node_put(mdio_np)
cleanup at the end of the function. This permanently leaks the device node
reference that was acquired by of_get_child_by_name() before the loop.

> +
> +		ret = an8855_phy_bus_register(dev, priv, phy_np,
> +					      &phy_bus_infos[i]);
> +		if (ret)
> +			break;
> +
> +		i++;
> +	}
> +
> +	of_node_put(mdio_np);
> +	return ret ? ret : i;
> +}
[ ... ]
> +static int an8855_core_probe(struct mdio_device *mdiodev)
> +{
[ ... ]
> +	/* Register virtual MDIO for each PHY */
> +	num_phys = an8855_mdio_register(dev, priv, phy_bus_infos);
> +	if (num_phys < 0)
> +		return num_phys;
[ ... ]
> +	/* Register PHYs */
> +	for (i = 0; i < num_phys; i++) {
> +		ret = fwnode_mdiobus_register_phy(phy_bus_infos[i].bus,
> +						  phy_bus_infos[i].node,
> +						  phy_bus_infos[i].addr);
> +		if (ret)
> +			return ret;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915123812.14747-1-ansuelsmth@gmail.com?part=4