netdev - Re: [net-next,v2,06/22] net: stmmac: rk: add SoC specific ->init() method

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: <aXgMIYPiL9JK-c_K@shell.armlinux.org.uk>
Date: Tue, 27 Jan 2026 00:51:45 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Jakub Kicinski <kuba@...nel.org>
Cc: linux-stm32@...md-mailman.stormreply.com, andrew+netdev@...n.ch,
	linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org,
	pabeni@...hat.com, linux-rockchip@...ts.infradead.org,
	heiko@...ech.de, alexandre.torgue@...s.st.com, andrew@...n.ch,
	davem@...emloft.net, edumazet@...gle.com
Subject: Re: [net-next,v2,06/22] net: stmmac: rk: add SoC specific ->init()
 method

On Mon, Jan 26, 2026 at 04:40:20PM -0800, Jakub Kicinski 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.
> 
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> net: stmmac: rk: add SoC specific ->init() method
> 
> This commit adds infrastructure for SoC-specific initialization by adding
> an init callback to rk_gmac_ops and calling it from rk_gmac_setup().
> 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index 7f8ffd1549bc..4117f6863ff0 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> 
> [ ... ]
> 
> > @@ -1618,6 +1619,14 @@ static struct rk_priv_data *rk_gmac_setup(struct platform_device *pdev,
> >
> >  	bsp_priv->dev = dev;
> >
> > +	if (ops->init) {
> > +		ret = ops->init(bsp_priv);
> > +		if (ret) {
> > +			dev_err_probe(dev, ret, "failed to init BSP\n");
> > +			return ERR_PTR(ret);
>                         ^^^^^^^^^^^^^^^
> 
> If ops->init() fails here, does bsp_priv->phy_reset need to be released?
> 
> Earlier in rk_gmac_setup(), at line 1610, the code calls:
> 
>     bsp_priv->phy_reset = of_reset_control_get(plat->phy_node, NULL);
> 
> of_reset_control_get() is not a devm-managed function, so the reset control
> would need to be explicitly released with reset_control_put() on error paths.
> 
> This new error return happens after phy_reset could have been acquired (when
> the device has an integrated PHY), but before the function successfully
> returns. If this path is taken, it appears the reset control would be leaked.
> 
> Currently no SoC sets ops->init so this path cannot trigger, but when a
> future SoC implements this callback, the leak would occur on init failure
> for devices with integrated PHYs.

However, the driver does not release this resource when cleaning up, so
that's already a bug as the driver currently stands. I think this could
be converted to devm_reset_control_get(), which would resolve both
leakages, but not sure.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!