media: nxp: imx8-isi: fix memory leaks in probe error paths and remove

PATCH Superseded David Carlier 4 participants 7 messages Mar 27 – Mar 30

Subsystems (4)

Stable branch Media (V4L/DVB) Arm i.MX and Layerscape NXP i.MX 8M ISI driver

lore

Share & tools

lore

David Carlier

author

2026-03-27 22:27 UTC #0 lore raw

PATCH 1/1 media: nxp: imx8-isi: fix memory leaks in probe error paths and remove

mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.

Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.

Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.

Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISP Channel driver")

Signed-off-by David Carlier <[email protected]>

---
 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c +19 -5

index 4bf8570e1b9e..ab32c5b6ac9c 100644
@@ -490,33 +490,43 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
-	if (isi->num_clks < 0)
+	if (isi->num_clks < 0) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
+	}
 
 	isi->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(isi->regs))
+	if (IS_ERR(isi->regs)) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, PTR_ERR(isi->regs),
 				     "Failed to get ISI register map\n");
+	}
 
 	if (isi->pdata->gasket_ops) {
 		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
 							      "fsl,blk-ctrl");
-		if (IS_ERR(isi->gasket))
+		if (IS_ERR(isi->gasket)) {
+			kfree(isi->pipes);
 			return dev_err_probe(dev, PTR_ERR(isi->gasket),
 					     "failed to get gasket\n");
+		}
 	}
 
 	dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
 	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
 
 	ret = devm_pm_runtime_enable(dev);
-	if (ret)
+	if (ret) {
+		kfree(isi->pipes);
 		return ret;
+	}
 
 	ret = mxc_isi_crossbar_init(isi);
-	if (ret)
+	if (ret) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, ret,
 				     "Failed to initialize crossbar\n");
+	}
 
 	for (i = 0; i < isi->pdata->num_channels; ++i) {
 		ret = mxc_isi_pipe_init(isi, i);

@@ -538,7 +548,10 @@ static int mxc_isi_probe(struct platform_device *pdev)
 	return 0;
 
 err_xbar:
+	while (i--)
+		mxc_isi_pipe_cleanup(&isi->pipes[i]);
 	mxc_isi_crossbar_cleanup(&isi->crossbar);
+	kfree(isi->pipes);
 
 	return ret;
 }

@@ -556,6 +569,7 @@ static void mxc_isi_remove(struct platform_device *pdev)
 		mxc_isi_pipe_cleanup(pipe);
 	}
 
+	kfree(isi->pipes);
 	mxc_isi_crossbar_cleanup(&isi->crossbar);
 	mxc_isi_v4l2_cleanup(isi);
 }

Signature · 1 line

2.53.0

David Carlier

author

2026-03-28 10:00 UTC #1 lore raw

PATCH v2 1/1 media: nxp: imx8-isi: fix memory leaks in probe error paths and remove

Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.

Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")

Signed-off-by David Carlier <[email protected]>

---
 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c +19 -5

index 4bf8570e1b9e..ab32c5b6ac9c 100644
@@ -490,33 +490,43 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
-	if (isi->num_clks < 0)
+	if (isi->num_clks < 0) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
+	}
 
 	isi->regs = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(isi->regs))
+	if (IS_ERR(isi->regs)) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, PTR_ERR(isi->regs),
 				     "Failed to get ISI register map\n");
+	}
 
 	if (isi->pdata->gasket_ops) {
 		isi->gasket = syscon_regmap_lookup_by_phandle(dev->of_node,
 							      "fsl,blk-ctrl");
-		if (IS_ERR(isi->gasket))
+		if (IS_ERR(isi->gasket)) {
+			kfree(isi->pipes);
 			return dev_err_probe(dev, PTR_ERR(isi->gasket),
 					     "failed to get gasket\n");
+		}
 	}
 
 	dma_size = isi->pdata->has_36bit_dma ? 36 : 32;
 	dma_set_mask_and_coherent(dev, DMA_BIT_MASK(dma_size));
 
 	ret = devm_pm_runtime_enable(dev);
-	if (ret)
+	if (ret) {
+		kfree(isi->pipes);
 		return ret;
+	}
 
 	ret = mxc_isi_crossbar_init(isi);
-	if (ret)
+	if (ret) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, ret,
 				     "Failed to initialize crossbar\n");
+	}
 
 	for (i = 0; i < isi->pdata->num_channels; ++i) {
 		ret = mxc_isi_pipe_init(isi, i);

@@ -538,7 +548,10 @@ static int mxc_isi_probe(struct platform_device *pdev)
 	return 0;
 
 err_xbar:
+	while (i--)
+		mxc_isi_pipe_cleanup(&isi->pipes[i]);
 	mxc_isi_crossbar_cleanup(&isi->crossbar);
+	kfree(isi->pipes);
 
 	return ret;
 }

@@ -556,6 +569,7 @@ static void mxc_isi_remove(struct platform_device *pdev)
 		mxc_isi_pipe_cleanup(pipe);
 	}
 
+	kfree(isi->pipes);
 	mxc_isi_crossbar_cleanup(&isi->crossbar);
 	mxc_isi_v4l2_cleanup(isi);
 }

Signature · 1 line

2.53.0

Greg KH

2026-03-28 10:21 UTC #2 lore raw

Quoted 8 lines · David Carlier

mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.

Quoted 7 lines

Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.
Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Signed-off-by David Carlier <[email protected]>
---

This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

David CARLIER

author

2026-03-28 11:15 UTC #3 lore raw

Quoted 18 lines · Greg KH

Quoted 8 lines · David Carlier
mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.
Quoted 7 lines
Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.
Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Signed-off-by David Carlier <[email protected]>
---

Quoted 6 lines

<formletter>
This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Apologies for the confusion — I wasn't submitting this for stable inclusion directly. The Cc was added based on CI bot feedback since the Fixes target is in the
stable tree, but I understand the correct flow is to let it go
through the maintainer tree first and let the Fixes tag handle stable backporting.

Cheers.

Quoted 2 lines

</formletter>

Greg KH

2026-03-28 11:37 UTC #4 lore raw

Quoted 26 lines · David CARLIER

Quoted 18 lines · Greg KH
Quoted 8 lines · David Carlier
mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.
Quoted 7 lines
Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.
Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Signed-off-by David Carlier <[email protected]>
---
Quoted 6 lines
<formletter>
This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

Quoted 6 lines

Apologies for the confusion — I wasn't submitting this for stable inclusion directly. The Cc was added based on CI bot feedback since the Fixes target is in the
stable tree, but I understand the correct flow is to let it go
through the maintainer tree first and let the Fixes tag handle stable backporting.

If you read the above, "Fixes:" does not guarantee backporting at all, so NEVER rely on that if you know you want something applied to a stable kernel tree.

thanks,

greg k-h

Frank Li

maintainer

2026-03-30 14:46 UTC #5 lore raw

In reply to David Carlier

Quoted 8 lines · David Carlier

mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.

Quoted 7 lines

Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.
Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISP Channel driver")
Signed-off-by David Carlier <[email protected]>
---

I think provide a helper function, devm_kzalloc_objs(), or using old devm_kzalloc is better fix method.

Frank

Quoted 18 lines

 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c +3 -1

index 4bf8570e1b9e..ab32c5b6ac9c 100644
@@ -490,33 +490,43 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
-	if (isi->num_clks < 0)
+	if (isi->num_clks < 0) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
+	}

Laurent Pinchart

maintainer

2026-03-30 15:41 UTC #6 lore raw

Quoted 13 lines · Frank Li

Quoted 12 lines · David Carlier
mxc_isi_probe() allocates isi->pipes with kzalloc_objs() but never frees it on any probe failure path or in mxc_isi_remove(), leaking the allocation on every failed probe and every normal unbind.
Additionally, when mxc_isi_pipe_init() fails partway through the channel loop or when mxc_isi_v4l2_init() fails, the already initialized pipes are not cleaned up — their media entities and mutexes are leaked.
Fix both by adding kfree(isi->pipes) to all probe error paths and to mxc_isi_remove(), and cleaning up already-initialized pipes in the err_xbar error path.

Quoted 6 lines

Quoted 3 lines
Fixes cf21f328fcaf ("media: nxp: Add i.MX8 ISP Channel driver")
Signed-off-by David Carlier <[email protected]>
---
I think provide a helper function, devm_kzalloc_objs(), or using old devm_kzalloc is better fix method.

I quite agree. Kees said he's planning to introduce devm_kzalloc_objs(). I assume this will come in the next kernel version. I think we can wait for that to fix this leak as it's a really minor issue.

Quoted 18 lines

 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c +3 -1

index 4bf8570e1b9e..ab32c5b6ac9c 100644
@@ -490,33 +490,43 @@ static int mxc_isi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	isi->num_clks = devm_clk_bulk_get_all(dev, &isi->clks);
-	if (isi->num_clks < 0)
+	if (isi->num_clks < 0) {
+		kfree(isi->pipes);
 		return dev_err_probe(dev, isi->num_clks, "Failed to get clocks\n");
+	}

Signature · 3 lines

Regards,

Laurent Pinchart