netdev - Re: [PATCH net-next 1/3] net: stmmac: rk: fix missing reset_control

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: <aXn-7LWRk5cZjno8@shell.armlinux.org.uk>
Date: Wed, 28 Jan 2026 12:19:56 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Philipp Zabel <p.zabel@...gutronix.de>
Cc: Andrew Lunn <andrew@...n.ch>,
	Alexandre Torgue <alexandre.torgue@...s.st.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Heiko Stuebner <heiko@...ech.de>, Jakub Kicinski <kuba@...nel.org>,
	linux-arm-kernel@...ts.infradead.org,
	linux-rockchip@...ts.infradead.org,
	linux-stm32@...md-mailman.stormreply.com, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 1/3] net: stmmac: rk: fix missing
 reset_control_put()

On Wed, Jan 28, 2026 at 01:04:21PM +0100, Philipp Zabel wrote:
> On Mi, 2026-01-28 at 10:58 +0000, Russell King (Oracle) wrote:
> > rk_gmac_setup() delves into the PHY's DT node to retrieve its reset
> > control using of_reset_control_get(). However, it never releases it
> > when the driver is removed. Add reset_control_put() to rk_gmac_exit()
> > to clean this up.
> > 
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---
> >  drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > index 5f8d2031b97c..bc69cbb5a7d4 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rk.c
> > @@ -1784,6 +1784,8 @@ static void rk_gmac_exit(struct device *dev, void *bsp_priv_)
> >  
> >  	if (priv->plat->phy_node && bsp_priv->integrated_phy)
> >  		clk_put(bsp_priv->clk_phy);
> > +
> > +	reset_control_put(bsp_priv->phy_reset);
> >  }
> >  
> >  static int rk_gmac_probe(struct platform_device *pdev)
> 
> This is fine because the driver sets plat_dat->suspend, and so
> rk_gmac_exit() is never called via stmmac_pltfr_exit() during suspend.
> 
> It does look a bit sketchy to release resources in the rk_gmac_exit()
> counterpart to rk_gmac_init(), which never requested the resources,
> though. Maybe use devm_add_action_or_reset() to register the release of
> the reset during remove?

Thanks, but I think a sense of proportion is required here. This
patch is the result of introducing the ->init() method, and AI
noticing that there was no cleanup of this resource. This was the
simplest way to implement that cleanup.

However, your review commit also applies to bsp_priv->clk_phy which
has the same problem - this also isn't obtained in rk_gmac_init(),
but in rk_gmac_clk_init().

Given that, and the fact that this entire series is already
considerably big (it was 21 patches, then 22, now 23, and with this
it's going to become 24 patches) I'm going to say that this issue
can be addressed at a later time.

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