Re: [PATCH v2 1/4] net: airoha: Fix an error handling path in airoha_alloc_gdm_port()
From: Christophe JAILLET <hidden>
Date: 2025-05-08 16:06:39
Also in:
kernel-janitors, linux-arm-kernel, linux-mediatek, lkml
Le 08/05/2025 à 17:31, Lorenzo Bianconi a écrit :
quoted hunk ↗ jump to hunk
quoted
If register_netdev() fails, the error handling path of the probe will not free the memory allocated by the previous airoha_metadata_dst_alloc() call because port->dev->reg_state will not be NETREG_REGISTERED. So, an explicit airoha_metadata_dst_free() call is needed in this case to avoid a memory leak. Fixes: af3cf757d5c9 ("net: airoha: Move DSA tag in DMA descriptor") Signed-off-by: Christophe JAILLET <redacted> --- Changes in v2: - New patch Compile tested only. --- drivers/net/ethernet/airoha/airoha_eth.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c index 16c7896f931f..af8c4015938c 100644 --- a/drivers/net/ethernet/airoha/airoha_eth.c +++ b/drivers/net/ethernet/airoha/airoha_eth.c@@ -2873,7 +2873,15 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth, if (err) return err; - return register_netdev(dev); + err = register_netdev(dev); + if (err) + goto free_metadata_dst; + + return 0; + +free_metadata_dst: + airoha_metadata_dst_free(port); + return err; } static int airoha_probe(struct platform_device *pdev)-- 2.49.0I have not tested it but I think the right fix here would be something like:diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c index b1ca8322d4eb..33f8926bba25 100644 --- a/drivers/net/ethernet/airoha/airoha_eth.c +++ b/drivers/net/ethernet/airoha/airoha_eth.c@@ -2996,10 +2996,12 @@ static int airoha_probe(struct platform_device *pdev) for (i = 0; i < ARRAY_SIZE(eth->ports); i++) { struct airoha_gdm_port *port = eth->ports[i]; - if (port && port->dev->reg_state == NETREG_REGISTERED) { + if (!port) + continue;
I think it works. We can still have port non NULL and airoha_metadata_dst_alloc() which fails, but airoha_metadata_dst_free() seems to handle it correctly. CJ
+ + if (port->dev->reg_state == NETREG_REGISTERED) unregister_netdev(port->dev); - airoha_metadata_dst_free(port); - } + airoha_metadata_dst_free(port); } free_netdev(eth->napi_dev); platform_set_drvdata(pdev, NULL); Regards, Lorenzo