* [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open
@ 2023-06-14 7:32 Christian Marangi
2023-06-14 14:55 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Christian Marangi @ 2023-06-14 7:32 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin,
Russell King, Christian Marangi, netdev, linux-stm32,
linux-arm-kernel, linux-kernel
Cc: Jose Abreu, stable
Fix a possible memory leak in __stmmac_open when stmmac_init_phy fails.
It's also needed to free everything allocated by stmmac_setup_dma_desc
and not just the dma_conf struct.
Correctly call free_dma_desc_resources on the new dma_conf passed to
__stmmac_open on error.
Reported-by: Jose Abreu <Jose.Abreu@synopsys.com>
Fixes: ba39b344e924 ("net: ethernet: stmicro: stmmac: generate stmmac dma conf before open")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Cc: stable@vger.kernel.org
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fa07b0d50b46..0966ab86fde2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3877,10 +3877,10 @@ static int __stmmac_open(struct net_device *dev,
stmmac_hw_teardown(dev);
init_error:
- free_dma_desc_resources(priv, &priv->dma_conf);
phylink_disconnect_phy(priv->phylink);
init_phy_error:
pm_runtime_put(priv->device);
+ free_dma_desc_resources(priv, dma_conf);
return ret;
}
--
2.40.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open 2023-06-14 7:32 [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open Christian Marangi @ 2023-06-14 14:55 ` Simon Horman 2023-06-14 8:18 ` Christian Marangi 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2023-06-14 14:55 UTC (permalink / raw) To: Christian Marangi Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Russell King, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Jose Abreu, stable On Wed, Jun 14, 2023 at 09:32:41AM +0200, Christian Marangi wrote: > Fix a possible memory leak in __stmmac_open when stmmac_init_phy fails. > It's also needed to free everything allocated by stmmac_setup_dma_desc > and not just the dma_conf struct. > > Correctly call free_dma_desc_resources on the new dma_conf passed to > __stmmac_open on error. > > Reported-by: Jose Abreu <Jose.Abreu@synopsys.com> > Fixes: ba39b344e924 ("net: ethernet: stmicro: stmmac: generate stmmac dma conf before open") > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > Cc: stable@vger.kernel.org > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index fa07b0d50b46..0966ab86fde2 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -3877,10 +3877,10 @@ static int __stmmac_open(struct net_device *dev, > > stmmac_hw_teardown(dev); > init_error: > - free_dma_desc_resources(priv, &priv->dma_conf); > phylink_disconnect_phy(priv->phylink); > init_phy_error: > pm_runtime_put(priv->device); > + free_dma_desc_resources(priv, dma_conf); Hi Christian, Are these resources allocated by the caller? If so, perhaps it would be clearer if a symmetric approach was taken and the caller handled freeing them on error. > return ret; > } > > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open 2023-06-14 14:55 ` Simon Horman @ 2023-06-14 8:18 ` Christian Marangi 2023-06-14 15:41 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Christian Marangi @ 2023-06-14 8:18 UTC (permalink / raw) To: Simon Horman Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Russell King, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Jose Abreu, stable On Wed, Jun 14, 2023 at 04:55:10PM +0200, Simon Horman wrote: > On Wed, Jun 14, 2023 at 09:32:41AM +0200, Christian Marangi wrote: > > Fix a possible memory leak in __stmmac_open when stmmac_init_phy fails. > > It's also needed to free everything allocated by stmmac_setup_dma_desc > > and not just the dma_conf struct. > > > > Correctly call free_dma_desc_resources on the new dma_conf passed to > > __stmmac_open on error. > > > > Reported-by: Jose Abreu <Jose.Abreu@synopsys.com> > > Fixes: ba39b344e924 ("net: ethernet: stmicro: stmmac: generate stmmac dma conf before open") > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > Cc: stable@vger.kernel.org > > --- > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index fa07b0d50b46..0966ab86fde2 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -3877,10 +3877,10 @@ static int __stmmac_open(struct net_device *dev, > > > > stmmac_hw_teardown(dev); > > init_error: > > - free_dma_desc_resources(priv, &priv->dma_conf); > > phylink_disconnect_phy(priv->phylink); > > init_phy_error: > > pm_runtime_put(priv->device); > > + free_dma_desc_resources(priv, dma_conf); > > Hi Christian, > > Are these resources allocated by the caller? > If so, perhaps it would be clearer if a symmetric approach > was taken and the caller handled freeing them on error. > Yes, they are. Handling in the caller would require some additional delta to this and some duplicated code but if preferred I can implement it. I can provide a v2 shortly if it's ok and you prefer this implementation. > > return ret; > > } > > > > -- > > 2.40.1 > > > > -- Ansuel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open 2023-06-14 8:18 ` Christian Marangi @ 2023-06-14 15:41 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2023-06-14 15:41 UTC (permalink / raw) To: Christian Marangi Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Russell King, netdev, linux-stm32, linux-arm-kernel, linux-kernel, Jose Abreu, stable On Wed, Jun 14, 2023 at 10:18:20AM +0200, Christian Marangi wrote: > On Wed, Jun 14, 2023 at 04:55:10PM +0200, Simon Horman wrote: > > On Wed, Jun 14, 2023 at 09:32:41AM +0200, Christian Marangi wrote: > > > Fix a possible memory leak in __stmmac_open when stmmac_init_phy fails. > > > It's also needed to free everything allocated by stmmac_setup_dma_desc > > > and not just the dma_conf struct. > > > > > > Correctly call free_dma_desc_resources on the new dma_conf passed to > > > __stmmac_open on error. > > > > > > Reported-by: Jose Abreu <Jose.Abreu@synopsys.com> > > > Fixes: ba39b344e924 ("net: ethernet: stmicro: stmmac: generate stmmac dma conf before open") > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > Cc: stable@vger.kernel.org > > > --- > > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > index fa07b0d50b46..0966ab86fde2 100644 > > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > > @@ -3877,10 +3877,10 @@ static int __stmmac_open(struct net_device *dev, > > > > > > stmmac_hw_teardown(dev); > > > init_error: > > > - free_dma_desc_resources(priv, &priv->dma_conf); > > > phylink_disconnect_phy(priv->phylink); > > > init_phy_error: > > > pm_runtime_put(priv->device); > > > + free_dma_desc_resources(priv, dma_conf); > > > > Hi Christian, > > > > Are these resources allocated by the caller? > > If so, perhaps it would be clearer if a symmetric approach > > was taken and the caller handled freeing them on error. > > > > Yes, they are. Handling in the caller would require some additional > delta to this and some duplicated code but if preferred I can implement > it. I can provide a v2 shortly if it's ok and you prefer this > implementation. Thanks, I think that would be best, unless the new delta is enormous for some reason. -- pw-bot: cr ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-06-14 15:41 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-14 7:32 [net PATCH] net: ethernet: stmicro: stmmac: fix possible memory leak in __stmmac_open Christian Marangi 2023-06-14 14:55 ` Simon Horman 2023-06-14 8:18 ` Christian Marangi 2023-06-14 15:41 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).