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

David Carlier posted 1 patch 5 days, 14 hours ago
There is a newer version of this series
.../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
1 file changed, 19 insertions(+), 5 deletions(-)
[PATCH] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by David Carlier 5 days, 14 hours ago
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 <devnexen@gmail.com>
---
 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 4bf8570e1b9e..ab32c5b6ac9c 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -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);
 }
-- 
2.53.0

Re: [PATCH] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by Frank Li 2 days, 21 hours ago
On Fri, Mar 27, 2026 at 10:27:11PM +0000, David Carlier wrote:
> 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 <devnexen@gmail.com>
> ---

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

Frank

>  .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
>  1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> index 4bf8570e1b9e..ab32c5b6ac9c 100644
> --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> @@ -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");
> +	}
>
>
Re: [PATCH] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by Laurent Pinchart 2 days, 20 hours ago
On Mon, Mar 30, 2026 at 10:46:43AM -0400, Frank Li wrote:
> On Fri, Mar 27, 2026 at 10:27:11PM +0000, David Carlier wrote:
> > 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 <devnexen@gmail.com>
> > ---
> 
> 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.

> >  .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
> >  1 file changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > index 4bf8570e1b9e..ab32c5b6ac9c 100644
> > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
> > @@ -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");
> > +	}
> >
> >

-- 
Regards,

Laurent Pinchart
[PATCH v2] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by David Carlier 5 days, 2 hours ago
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 ISI driver")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 .../platform/nxp/imx8-isi/imx8-isi-core.c     | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
index 4bf8570e1b9e..ab32c5b6ac9c 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.c
@@ -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);
 }
-- 
2.53.0

Re: [PATCH v2] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by Greg KH 5 days, 2 hours ago
On Sat, Mar 28, 2026 at 10:00:10AM +0000, David Carlier wrote:
> 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 ISI driver")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---

<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.

</formletter>
Re: [PATCH v2] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by David CARLIER 5 days, 1 hour ago
On Sat, 28 Mar 2026 at 10:21, Greg KH <greg@kroah.com> wrote:
>
> On Sat, Mar 28, 2026 at 10:00:10AM +0000, David Carlier wrote:
> > 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 ISI driver")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
>
> <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.

>
> </formletter>
Re: [PATCH v2] media: nxp: imx8-isi: fix memory leaks in probe error paths and remove
Posted by Greg KH 5 days, 1 hour ago
On Sat, Mar 28, 2026 at 11:15:18AM +0000, David CARLIER wrote:
> On Sat, 28 Mar 2026 at 10:21, Greg KH <greg@kroah.com> wrote:
> >
> > On Sat, Mar 28, 2026 at 10:00:10AM +0000, David Carlier wrote:
> > > 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 ISI driver")
> > > Signed-off-by: David Carlier <devnexen@gmail.com>
> > > ---
> >
> > <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.

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