| Message ID | 20221116051407.1679342-1-nalanzeyu@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] net: ethernet: mtk_eth_soc: fix memory leak in error path | expand |
On Wed, Nov 16, 2022 at 01:14:07PM +0800, Yan Cangang wrote: > In mtk_ppe_init(), when dmam_alloc_coherent() or devm_kzalloc() failed, > the rhashtable ppe->l2_flows isn't destroyed. Fix it. > > In mtk_probe(), when mtk_eth_offload_init() or register_netdev() failed, > have the same problem. Also, call to mtk_mdio_cleanup() is missed in this > case. Fix it. > > Fixes: 33fc42de3327 ("net: ethernet: mtk_eth_soc: support creating mac address based offload entries") > Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") > Fixes: 502e84e2382d ("net: ethernet: mtk_eth_soc: add flow offloading support") > Signed-off-by: Yan Cangang <nalanzeyu@gmail.com> > --- > v1: https://lore.kernel.org/netdev/20221112233239.824389-1-nalanzeyu@gmail.com/T/ > v2: > - clean up commit message > - new mtk_ppe_deinit() function, call it before calling mtk_mdio_cleanup() Subject line should include target [PATCH net v2] .... > > drivers/net/ethernet/mediatek/mtk_eth_soc.c | 9 +++++---- > drivers/net/ethernet/mediatek/mtk_ppe.c | 19 +++++++++++++++++-- > drivers/net/ethernet/mediatek/mtk_ppe.h | 1 + > 3 files changed, 23 insertions(+), 6 deletions(-) The change seems ok, but please be aware that mtk_eth_offload_init() calls to rhashtable_init() which is not destroyed too. Thanks, Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
On Wed, 16 Nov 2022 13:14:07 +0800 Yan Cangang wrote: > In mtk_ppe_init(), when dmam_alloc_coherent() or devm_kzalloc() failed, > the rhashtable ppe->l2_flows isn't destroyed. Fix it. > > In mtk_probe(), when mtk_eth_offload_init() or register_netdev() failed, > have the same problem. Also, call to mtk_mdio_cleanup() is missed in this > case. Fix it. > > Fixes: 33fc42de3327 ("net: ethernet: mtk_eth_soc: support creating mac address based offload entries") > Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") > Fixes: 502e84e2382d ("net: ethernet: mtk_eth_soc: add flow offloading support") > Signed-off-by: Yan Cangang <nalanzeyu@gmail.com> Please break this fix up into a series of two patches, first one fixing the issue in ba37b7caf1ed and 502e84e2382d, and second one fixing the issue in 33fc42de3327. Those are separate fixes, and should be backported differently (the former is needed in LTS). Please repost with the subject changed as Leon suggested and do not repost in this thread, start a new thread.
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c index 7cd381530aa4..3f806a1358b7 100644 --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c @@ -4143,13 +4143,13 @@ static int mtk_probe(struct platform_device *pdev) eth->soc->offload_version, i); if (!eth->ppe[i]) { err = -ENOMEM; - goto err_free_dev; + goto err_deinit_ppe; } } err = mtk_eth_offload_init(eth); if (err) - goto err_free_dev; + goto err_deinit_ppe; } for (i = 0; i < MTK_MAX_DEVS; i++) { @@ -4159,7 +4159,7 @@ static int mtk_probe(struct platform_device *pdev) err = register_netdev(eth->netdev[i]); if (err) { dev_err(eth->dev, "error bringing up device\n"); - goto err_deinit_mdio; + goto err_deinit_ppe; } else netif_info(eth, probe, eth->netdev[i], "mediatek frame engine at 0x%08lx, irq %d\n", @@ -4177,7 +4177,8 @@ static int mtk_probe(struct platform_device *pdev) return 0; -err_deinit_mdio: +err_deinit_ppe: + mtk_ppe_deinit(eth); mtk_mdio_cleanup(eth); err_free_dev: mtk_free_dev(eth); diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.c b/drivers/net/ethernet/mediatek/mtk_ppe.c index 2d8ca99f2467..784ecb2dc9fb 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.c +++ b/drivers/net/ethernet/mediatek/mtk_ppe.c @@ -737,7 +737,7 @@ struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base, MTK_PPE_ENTRIES * soc->foe_entry_size, &ppe->foe_phys, GFP_KERNEL); if (!foe) - return NULL; + goto err_free_l2_flows; ppe->foe_table = foe; @@ -745,11 +745,26 @@ struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base, sizeof(*ppe->foe_flow); ppe->foe_flow = devm_kzalloc(dev, foe_flow_size, GFP_KERNEL); if (!ppe->foe_flow) - return NULL; + goto err_free_l2_flows; mtk_ppe_debugfs_init(ppe, index); return ppe; + +err_free_l2_flows: + rhashtable_destroy(&ppe->l2_flows); + return NULL; +} + +void mtk_ppe_deinit(struct mtk_eth *eth) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(eth->ppe); i++) { + if (!eth->ppe[i]) + return; + rhashtable_destroy(ð->ppe[i]->l2_flows); + } } static void mtk_ppe_init_foe_table(struct mtk_ppe *ppe) diff --git a/drivers/net/ethernet/mediatek/mtk_ppe.h b/drivers/net/ethernet/mediatek/mtk_ppe.h index 0b7a67a958e4..a09c32539bcc 100644 --- a/drivers/net/ethernet/mediatek/mtk_ppe.h +++ b/drivers/net/ethernet/mediatek/mtk_ppe.h @@ -304,6 +304,7 @@ struct mtk_ppe { struct mtk_ppe *mtk_ppe_init(struct mtk_eth *eth, void __iomem *base, int version, int index); +void mtk_ppe_deinit(struct mtk_eth *eth); void mtk_ppe_start(struct mtk_ppe *ppe); int mtk_ppe_stop(struct mtk_ppe *ppe);
In mtk_ppe_init(), when dmam_alloc_coherent() or devm_kzalloc() failed, the rhashtable ppe->l2_flows isn't destroyed. Fix it. In mtk_probe(), when mtk_eth_offload_init() or register_netdev() failed, have the same problem. Also, call to mtk_mdio_cleanup() is missed in this case. Fix it. Fixes: 33fc42de3327 ("net: ethernet: mtk_eth_soc: support creating mac address based offload entries") Fixes: ba37b7caf1ed ("net: ethernet: mtk_eth_soc: add support for initializing the PPE") Fixes: 502e84e2382d ("net: ethernet: mtk_eth_soc: add flow offloading support") Signed-off-by: Yan Cangang <nalanzeyu@gmail.com> --- v1: https://lore.kernel.org/netdev/20221112233239.824389-1-nalanzeyu@gmail.com/T/ v2: - clean up commit message - new mtk_ppe_deinit() function, call it before calling mtk_mdio_cleanup() drivers/net/ethernet/mediatek/mtk_eth_soc.c | 9 +++++---- drivers/net/ethernet/mediatek/mtk_ppe.c | 19 +++++++++++++++++-- drivers/net/ethernet/mediatek/mtk_ppe.h | 1 + 3 files changed, 23 insertions(+), 6 deletions(-)