netdev - [net-next,v2,09/15] net: ftgmac100: Always register the MDIO bus when it exists

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <SEYPR06MB5134A65B925BAC871927C9C69D9EA@SEYPR06MB5134.apcprd06.prod.outlook.com>
Date: Thu, 29 Jan 2026 06:07:20 +0000
From: Jacky Chou <jacky_chou@...eedtech.com>
To: Andrew Lunn <andrew@...n.ch>
CC: Simon Horman <horms@...nel.org>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "kuba@...nel.org"
	<kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: [net-next,v2,09/15] net: ftgmac100: Always register the MDIO bus when
 it exists

> > > > > +	if (priv->mac_id == FTGMAC100_FARADAY ||
> > > > > +	    priv->mac_id == FTGMAC100_AST2400 ||
> > > > > +	    priv->mac_id == FTGMAC100_AST2500) {
> > > > > +		err = ftgmac100_setup_mdio(netdev);
> > > > > +		if (err)
> > > > > +			goto err_phy_connect;
> > > > > +	}
> > > >
> > > > If ftgmac100_setup_mdio() fails after of_mdiobus_register()
> > > > returns an error, it calls mdiobus_free(priv->mii_bus) but does
> > > > not set
> > > > priv->mii_bus to NULL. The goto err_phy_connect then leads to
> > > > ftgmac100_destroy_mdio() which checks if (!priv->mii_bus) but
> > > > finds a dangling pointer, resulting in mdiobus_unregister() on
> > > > freed memory
> > > followed by a double free.
> 
> I took another look at this. The goto err_phy_connect is wrong.  At this point,
> if there is an error nothing else needs cleaning up, so the goto should be
> replaced with a return err;
> 

Agreed.
When ftgmac100_setup_mdio() fails, the MDIO bus is not registered and the
probe function returns an error immediately.

> It is not however the full solution. If ftgmac100_probe_ncsi() fails, it jumps to
> err_setup_mdio, which currently just does a return, leaking the mdiobus. The
> ftgmac100_destroy_mdio() call needs moving after the err_setup_mdio: label.
> 

In NC-SI mode, MDIO is not required.
Therefore, always registering the MDIO bus is redundant when the interface
operates in NC-SI mode.

Perhaps the adjustments in this patch could be moved to follow
"net: ftgmac100: Move DT probe into a helper".
And add to this location in ftgmac100_probe_dt() like below.

static int ftgmac100_probe_dt(struct net_device *netdev,
			      struct platform_device *pdev,
			      struct ftgmac100 *priv,
			      struct device_node *np)
{
	struct phy_device *phy;
	int err;

	if (of_get_property(np, "use-ncsi", NULL))
		return ftgmac100_probe_ncsi(netdev, priv, pdev);


======== > Add here
    if (priv->mac_id == FTGMAC100_FARADAY ||
	    priv->mac_id == FTGMAC100_AST2400 ||
	    priv->mac_id == FTGMAC100_AST2500) {
		err = ftgmac100_setup_mdio(netdev);
		if (err)
			return err;
	}
========

	if (of_phy_is_fixed_link(np) ||
	    of_get_property(np, "phy-handle", NULL)) {
		/* Support "mdio"/"phy" child nodes for ast2400/2500
		 * with an embedded MDIO controller. Automatically
		 * scan the DTS for available PHYs and register
		 * them. 2600 has an independent MDIO controller, not
		 * part of the MAC.
		 */
		phy = of_phy_get_and_connect(priv->netdev, np,
					     &ftgmac100_adjust_link);
		if (!phy) {
			dev_err(&pdev->dev, "Failed to connect to phy\n");
			return -EINVAL;
		}

		/* Indicate that we support PAUSE frames (see comment in
		 * Documentation/networking/phy.rst)
		 */
		phy_support_asym_pause(phy);

		/* Display what we found */
		phy_attached_info(phy);
		return 0;
	}
....
}

Thanks,
Jacky