netdev - Re: [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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aW9zZpb1RTuhhIqG@horms.kernel.org>
Date: Tue, 20 Jan 2026 12:21:58 +0000
From: Simon Horman <horms@...nel.org>
To: jacky_chou@...eedtech.com
Cc: davem@...emloft.net, edumazet@...gle.com, andrew+netdev@...n.ch,
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [net-next,v2,09/15] net: ftgmac100: Always register the MDIO bus
 when it exists

On Tue, Jan 20, 2026 at 12:14:07PM +0000, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://netdev-ai.bots.linux.dev/ai-review.html
> ---
> net: ftgmac100: Always register the MDIO bus when it exists
> 
> This patch moves MDIO bus registration earlier in the probe function
> for FARADAY, AST2400, and AST2500 MACs, simplifying the code by
> registering it unconditionally when the hardware has an MDIO controller.
> 
> Note: These issues appear to be fixed later in the series by commits
> 8961daa5f2c0 ("Simplify error handling for ftgmac100_setup_mdio") and
> 046e51399f0f ("Move DT probe into a helper").
> 
> > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> > index f1cb5dc37919..931fdf3d07d1 100644
> > --- a/drivers/net/ethernet/faraday/ftgmac100.c
> > +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> > @@ -1957,6 +1957,14 @@ static int ftgmac100_probe(struct platform_device *pdev)
> >  		priv->txdes0_edotr_mask = BIT(15);
> >  	}
> >
> > +	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.
> 
> Could this cause a use-after-free and double-free when MDIO registration
> fails? The error path in ftgmac100_setup_mdio() frees mii_bus without
> clearing the pointer, and err_phy_connect eventually calls
> ftgmac100_destroy_mdio() which operates on the stale pointer.
> 
> > +
> > +	if (np && of_get_property(np, "use-ncsi", NULL)) {
> > +		err = ftgmac100_probe_ncsi(netdev, priv, pdev);
> > +		if (err)
> > +			goto err_setup_mdio;
> 
> Since MDIO is now set up before the NCSI probe for FARADAY/AST2400/AST2500,
> if ftgmac100_probe_ncsi() fails, the goto err_setup_mdio skips the MDIO
> cleanup at ftgmac100_destroy_mdio(). Does this leak the mii_bus allocation
> when NCSI probe fails on these MAC variants?
> 
> [ ... ]
> -- 
> pw-bot: changes-requested
> 
> This is an AI-generated review. Report issues at https://example.com

Sorry for the bogus last line here. A local configuration error on my side.
But the rest of what is above is good, IMHO.