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] [day] [month] [year] [list]

Message-ID: <52ea6b98b27235f10008406de45cdcecc2a7b105.camel@pengutronix.de>
Date: Wed, 28 Jan 2026 14:52:57 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
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 Mi, 2026-01-28 at 12:19 +0000, Russell King (Oracle) wrote:
> 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.

No objection,

Reviewed-by: Philipp Zabel <p.zabel@...gutronix.de>

regards
Philipp