[PATCH v2 0/2] media: atomisp: fix probe memory leaks

[PATCH v2 0/2] media: atomisp: fix probe memory leaks

[Dawei Feng]

This series fixes two memory leaks in the atomisp PCI probe and adjusts
cleanup paths.

Patch 1 fixes the cleanup boundary for media entities created during
module initialization. atomisp_uninitialize_modules() did not release all
module-owned state and instead left some media entity cleanup to unregister
helpers. That split is incomplete for probe failures that happen after
module initialization but before all entities are registered, so the
module cleanup path now owns the corresponding media entity cleanup.

Patch 2 adds cleanup for the V4L2 async notifier state initialized by
atomisp_csi2_bridge_parse_firmware(), including notifier connection
cleanup on probe failures and notifier unregister on remove.

Changes in v2:
- Rework patch 1 around the module-init cleanup ownership boundary and
  move media_device_cleanup() after module cleanup.
- Add async notifier cleanup as patch 2.

Dawei Feng (2):
  media: atomisp: fix memory leak in atomisp_pci_probe()
  media: atomisp: fix memory leak in
    atomisp_csi2_bridge_parse_firmware()

 .../staging/media/atomisp/pci/atomisp_csi2.c  |  5 +++-
 .../media/atomisp/pci/atomisp_subdev.c        |  9 +++++--
 .../staging/media/atomisp/pci/atomisp_v4l2.c  | 26 ++++++++++++++-----
 3 files changed, 30 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Dawei Feng]

atomisp_initialize_modules() creates CSI2 and ISP subdev media entities
before atomisp_pci_probe() registers them. Its counterpart,
atomisp_uninitialize_modules(), only releases part of that module-owned
state and leaves some media entity cleanup to the entity unregister path.

That ownership split is incomplete for probe error paths. If
atomisp_pci_probe() fails after module initialization but before all
entities are registered, the unwind path cannot rely on unregister
helpers to release media entity state whose lifetime started in module
initialization. The CSI2 and ISP subdev media entities can therefore be
left allocated.

Refactor the cleanup boundary so module cleanup releases media entities
created by module initialization, while unregister helpers only undo
registered V4L2 and media device state. Move CSI2 and ISP subdev media
entity cleanup into atomisp_mipi_csi2_cleanup() and the new
atomisp_subdev_cleanup(), and run media_device_cleanup() after module
cleanup in the probe unwind and remove paths.

If atomisp_mipi_csi2_init() itself fails, it has already unwound its
partial setup, so return the error directly. Only the later
atomisp_subdev_init() failure path needs to clean up CSI2 from the
caller.

The bug was first flagged by an experimental analysis tool we are
developing for kernel memory-management bugs while analyzing
v6.13-rc1. The tool is still under development and is not yet publicly
available. Manual inspection confirms that the bug is still present in
v7.1-rc7.

An x86_64 allyesconfig build showed no new warnings. As we do not have
an Intel Atom ISP platform with the required camera sensor hardware to
test with, no runtime testing was able to be performed.

Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/staging/media/atomisp/pci/atomisp_csi2.c   |  5 ++++-
 drivers/staging/media/atomisp/pci/atomisp_subdev.c |  9 +++++++--
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c   | 12 +++++-------
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_csi2.c b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
index 95b9113d75e9..2a85d04ade81 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_csi2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_csi2.c
@@ -185,7 +185,6 @@ static int mipi_csi2_init_entities(struct atomisp_mipi_csi2_device *csi2,
 void
 atomisp_mipi_csi2_unregister_entities(struct atomisp_mipi_csi2_device *csi2)
 {
-	media_entity_cleanup(&csi2->subdev.entity);
 	v4l2_device_unregister_subdev(&csi2->subdev);
 }
 
@@ -331,6 +330,10 @@ void atomisp_csi2_configure(struct atomisp_sub_device *asd)
  */
 void atomisp_mipi_csi2_cleanup(struct atomisp_device *isp)
 {
+	unsigned int i;
+
+	for (i = 0; i < ATOMISP_CAMERA_NR_PORTS; i++)
+		media_entity_cleanup(&isp->csi2_port[i].subdev.entity);
 }
 
 int atomisp_mipi_csi2_init(struct atomisp_device *isp)
diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.c b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
index 3d56ca83ecb7..11d7e04d3ec5 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.c
@@ -886,11 +886,16 @@ void atomisp_subdev_cleanup_pending_events(struct atomisp_sub_device *asd)
 
 void atomisp_subdev_unregister_entities(struct atomisp_sub_device *asd)
 {
-	atomisp_subdev_cleanup_entities(asd);
 	v4l2_device_unregister_subdev(&asd->subdev);
 	atomisp_video_unregister(&asd->video_out);
 }
 
+void atomisp_subdev_cleanup(struct atomisp_device *isp)
+{
+	atomisp_subdev_cleanup_entities(&isp->asd);
+	media_entity_cleanup(&isp->asd.video_out.vdev.entity);
+}
+
 int atomisp_subdev_register_subdev(struct atomisp_sub_device *asd,
 				   struct v4l2_device *vdev)
 {
@@ -913,7 +918,7 @@ int atomisp_subdev_init(struct atomisp_device *isp)
 	isp_subdev_init_params(&isp->asd);
 	ret = isp_subdev_init_entities(&isp->asd);
 	if (ret < 0)
-		atomisp_subdev_cleanup_entities(&isp->asd);
+		atomisp_subdev_cleanup(isp);
 
 	return ret;
 }
diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 900a67552d6a..5ba9584b81d7 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -334,10 +334,8 @@ int atomisp_video_init(struct atomisp_video_pipe *video)
 
 void atomisp_video_unregister(struct atomisp_video_pipe *video)
 {
-	if (video_is_registered(&video->vdev)) {
-		media_entity_cleanup(&video->vdev.entity);
+	if (video_is_registered(&video->vdev))
 		video_unregister_device(&video->vdev);
-	}
 }
 
 static int atomisp_save_iunit_reg(struct atomisp_device *isp)
@@ -814,7 +812,6 @@ static void atomisp_unregister_entities(struct atomisp_device *isp)
 
 	v4l2_device_unregister(&isp->v4l2_dev);
 	media_device_unregister(&isp->media_dev);
-	media_device_cleanup(&isp->media_dev);
 
 	for (i = 0; i < isp->input_cnt; i++)
 		__v4l2_subdev_state_free(isp->inputs[i].try_sd_state);
@@ -875,7 +872,6 @@ static int atomisp_register_entities(struct atomisp_device *isp)
 	v4l2_device_unregister(&isp->v4l2_dev);
 v4l2_device_failed:
 	media_device_unregister(&isp->media_dev);
-	media_device_cleanup(&isp->media_dev);
 	return ret;
 }
 
@@ -1086,7 +1082,7 @@ static int atomisp_initialize_modules(struct atomisp_device *isp)
 	ret = atomisp_mipi_csi2_init(isp);
 	if (ret < 0) {
 		dev_err(isp->dev, "mipi csi2 initialization failed\n");
-		goto error_mipi_csi2;
+		return ret;
 	}
 
 	ret = atomisp_subdev_init(isp);
@@ -1098,13 +1094,13 @@ static int atomisp_initialize_modules(struct atomisp_device *isp)
 	return 0;
 
 error_isp_subdev:
-error_mipi_csi2:
 	atomisp_mipi_csi2_cleanup(isp);
 	return ret;
 }
 
 static void atomisp_uninitialize_modules(struct atomisp_device *isp)
 {
+	atomisp_subdev_cleanup(isp);
 	atomisp_mipi_csi2_cleanup(isp);
 }
 
@@ -1451,6 +1447,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	atomisp_unregister_entities(isp);
 error_uninitialize_modules:
 	atomisp_uninitialize_modules(isp);
+	media_device_cleanup(&isp->media_dev);
 error_irq_uninit:
 	atomisp_msi_irq_uninit(isp);
 	pci_free_irq_vectors(pdev);
@@ -1476,6 +1473,7 @@ static void atomisp_pci_remove(struct pci_dev *pdev)
 
 	atomisp_unregister_entities(isp);
 	atomisp_uninitialize_modules(isp);
+	media_device_cleanup(&isp->media_dev);
 	atomisp_msi_irq_uninit(isp);
 	pci_free_irq_vectors(pdev);
 }
-- 
2.34.1

[PATCH v2 2/2] media: atomisp: fix memory leak in atomisp_csi2_bridge_parse_firmware()

[Dawei Feng]

atomisp_csi2_bridge_parse_firmware() initializes isp->notifier and may
allocate async notifier connections via v4l2_async_nf_add_fwnode_remote().
However, these resources are currently leaked if a subsequent entity
registration or probe step fails, or when the driver is removed.

Fix this by introducing dedicated helpers to clean up and unregister the
async notifier state. Call atomisp_notifier_cleanup() to release the
allocated connections in both the entity registration failure path and the
overall probe unwind path.

Additionally, invoke atomisp_notifier_unregister() during the device
remove path to ensure the notifier is properly unregistered from the V4L2
core before its underlying resources are freed.

Fixes: 8d28ec7e9145 ("media: atomisp: Add support for v4l2-async sensor registration")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
---
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 5ba9584b81d7..1073d9e5eafb 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -798,6 +798,17 @@ static int atomisp_subdev_probe(struct atomisp_device *isp)
 	return atomisp_csi_lane_config(isp);
 }
 
+static void atomisp_notifier_cleanup(struct atomisp_device *isp)
+{
+	v4l2_async_nf_cleanup(&isp->notifier);
+}
+
+static void atomisp_notifier_unregister(struct atomisp_device *isp)
+{
+	v4l2_async_nf_unregister(&isp->notifier);
+	atomisp_notifier_cleanup(isp);
+}
+
 static void atomisp_unregister_entities(struct atomisp_device *isp)
 {
 	unsigned int i;
@@ -869,6 +880,7 @@ static int atomisp_register_entities(struct atomisp_device *isp)
 	for (i = 0; i < ATOMISP_CAMERA_NR_PORTS; i++)
 		atomisp_mipi_csi2_unregister_entities(&isp->csi2_port[i]);
 csi_and_subdev_probe_failed:
+	atomisp_notifier_cleanup(isp);
 	v4l2_device_unregister(&isp->v4l2_dev);
 v4l2_device_failed:
 	media_device_unregister(&isp->media_dev);
@@ -1444,6 +1456,7 @@ static int atomisp_pci_probe(struct pci_dev *pdev, const struct pci_device_id *i
 	devm_free_irq(&pdev->dev, pdev->irq, isp);
 error_unregister_entities:
 	hmm_cleanup();
+	atomisp_notifier_cleanup(isp);
 	atomisp_unregister_entities(isp);
 error_uninitialize_modules:
 	atomisp_uninitialize_modules(isp);
@@ -1471,6 +1484,7 @@ static void atomisp_pci_remove(struct pci_dev *pdev)
 	devm_free_irq(&pdev->dev, pdev->irq, isp);
 	hmm_cleanup();
 
+	atomisp_notifier_unregister(isp);
 	atomisp_unregister_entities(isp);
 	atomisp_uninitialize_modules(isp);
 	media_device_cleanup(&isp->media_dev);
-- 
2.34.1

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Andy Shevchenko]

On Mon, Jun 15, 2026 at 03:28:40PM +0800, Dawei Feng wrote:
> atomisp_initialize_modules() creates CSI2 and ISP subdev media entities
> before atomisp_pci_probe() registers them. Its counterpart,
> atomisp_uninitialize_modules(), only releases part of that module-owned
> state and leaves some media entity cleanup to the entity unregister path.
> 
> That ownership split is incomplete for probe error paths. If
> atomisp_pci_probe() fails after module initialization but before all
> entities are registered, the unwind path cannot rely on unregister
> helpers to release media entity state whose lifetime started in module
> initialization. The CSI2 and ISP subdev media entities can therefore be
> left allocated.
> 
> Refactor the cleanup boundary so module cleanup releases media entities
> created by module initialization, while unregister helpers only undo
> registered V4L2 and media device state. Move CSI2 and ISP subdev media
> entity cleanup into atomisp_mipi_csi2_cleanup() and the new
> atomisp_subdev_cleanup(), and run media_device_cleanup() after module
> cleanup in the probe unwind and remove paths.
> 
> If atomisp_mipi_csi2_init() itself fails, it has already unwound its
> partial setup, so return the error directly. Only the later
> atomisp_subdev_init() failure path needs to clean up CSI2 from the
> caller.

> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available. Manual inspection confirms that the bug is still present in
> v7.1-rc7.
> 
> An x86_64 allyesconfig build showed no new warnings. As we do not have
> an Intel Atom ISP platform with the required camera sensor hardware to
> test with, no runtime testing was able to be performed.

These last two paragraphs do not suit the commit message. Please, drop them
here and better to describe all this in the cover letter (if not yet).

> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>

This SoB chain is wrong. Who is Zilin and why is he here?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] media: atomisp: fix memory leak in atomisp_csi2_bridge_parse_firmware()

[Andy Shevchenko]

On Mon, Jun 15, 2026 at 03:28:41PM +0800, Dawei Feng wrote:
> atomisp_csi2_bridge_parse_firmware() initializes isp->notifier and may
> allocate async notifier connections via v4l2_async_nf_add_fwnode_remote().
> However, these resources are currently leaked if a subsequent entity
> registration or probe step fails, or when the driver is removed.
> 
> Fix this by introducing dedicated helpers to clean up and unregister the
> async notifier state. Call atomisp_notifier_cleanup() to release the
> allocated connections in both the entity registration failure path and the
> overall probe unwind path.
> 
> Additionally, invoke atomisp_notifier_unregister() during the device
> remove path to ensure the notifier is properly unregistered from the V4L2
> core before its underlying resources are freed.

> Fixes: 8d28ec7e9145 ("media: atomisp: Add support for v4l2-async sensor registration")
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>

Same comment as per previous patch regarding to SoB chain.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Dan Carpenter]

On Mon, Jun 15, 2026 at 02:11:12PM +0300, Andy Shevchenko wrote:
> > The bug was first flagged by an experimental analysis tool we are
> > developing for kernel memory-management bugs while analyzing
> > v6.13-rc1. The tool is still under development and is not yet publicly
> > available. Manual inspection confirms that the bug is still present in
> > v7.1-rc7.
> > 
> > An x86_64 allyesconfig build showed no new warnings. As we do not have
> > an Intel Atom ISP platform with the required camera sensor hardware to
> > test with, no runtime testing was able to be performed.
> 
> These last two paragraphs do not suit the commit message. Please, drop them
> here and better to describe all this in the cover letter (if not yet).

This is how the documentation says to write commit messages.

https://lore.kernel.org/all/ahgaOigklcDCYvRp@stanley.mountain/

regards,
dan carpenter

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Dawei Feng]

Hi Andy,

Thanks for the review.

On Mon, Jun 15, 2026 at 02:11:12PM +0300, Andy Shevchenko wrote:
>> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
>> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
>> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
>
>This SoB chain is wrong. Who is Zilin and why is he here?

Zilin is the discoverer of this bug. We are in the same research group,
and he actively participated in reviewing this patch. 

To better align with the kernel submission guidelines, I will add a
"Co-developed-by:" tag in the v3 patch for Zilin to properly reflect his
contributions. Would this be acceptable?

>These last two paragraphs do not suit the commit message. Please, drop them
>here and better to describe all this in the cover letter (if not yet).

As Dan mentioned, I included those paragraphs following the exapmle in
researcher-guidelines[1].

[1] https://docs.kernel.org/process/researcher-guidelines.html

Best regards,
Dawei Feng

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Andy Shevchenko]

On Mon, Jun 15, 2026 at 02:39:42PM +0300, Dan Carpenter wrote:
> On Mon, Jun 15, 2026 at 02:11:12PM +0300, Andy Shevchenko wrote:

...

> > > The bug was first flagged by an experimental analysis tool we are
> > > developing for kernel memory-management bugs while analyzing
> > > v6.13-rc1. The tool is still under development and is not yet publicly
> > > available. Manual inspection confirms that the bug is still present in
> > > v7.1-rc7.
> > > 
> > > An x86_64 allyesconfig build showed no new warnings. As we do not have
> > > an Intel Atom ISP platform with the required camera sensor hardware to
> > > test with, no runtime testing was able to be performed.
> > 
> > These last two paragraphs do not suit the commit message. Please, drop them
> > here and better to describe all this in the cover letter (if not yet).
> 
> This is how the documentation says to write commit messages.
> 
> https://lore.kernel.org/all/ahgaOigklcDCYvRp@stanley.mountain/

Isn't it enough to have in the cover letter?

Thanks for commenting there, but I would insist to move these two paragraphs
from the commit message here. Maybe Kees is okay with that, I'm thinking that
this is too much (since we have lore archives).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Andy Shevchenko]

On Mon, Jun 15, 2026 at 08:35:15PM +0800, Dawei Feng wrote:
> On Mon, Jun 15, 2026 at 02:11:12PM +0300, Andy Shevchenko wrote:
> >> Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
> >> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> >> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> >
> >This SoB chain is wrong. Who is Zilin and why is he here?
> 
> Zilin is the discoverer of this bug. We are in the same research group,
> and he actively participated in reviewing this patch. 

> To better align with the kernel submission guidelines, I will add a
> "Co-developed-by:" tag in the v3 patch for Zilin to properly reflect his
> contributions. Would this be acceptable?

But did he _develop_ any parts of this patch? Otherwise Reported-by is more
suitable.

> >These last two paragraphs do not suit the commit message. Please, drop them
> >here and better to describe all this in the cover letter (if not yet).
> 
> As Dan mentioned, I included those paragraphs following the exapmle in
> researcher-guidelines[1].

Yes, and I still insist to move them to the cover letter. In any case those are
not present in the second patch anyway, moving that to cover letter covers the
entire series (and I believe you tested the entire series, didn't you?).

> [1] https://docs.kernel.org/process/researcher-guidelines.html

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Dan Carpenter]

On Mon, Jun 15, 2026 at 04:49:28PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 15, 2026 at 02:39:42PM +0300, Dan Carpenter wrote:
> > On Mon, Jun 15, 2026 at 02:11:12PM +0300, Andy Shevchenko wrote:
> 
> ...
> 
> > > > The bug was first flagged by an experimental analysis tool we are
> > > > developing for kernel memory-management bugs while analyzing
> > > > v6.13-rc1. The tool is still under development and is not yet publicly
> > > > available. Manual inspection confirms that the bug is still present in
> > > > v7.1-rc7.
> > > > 
> > > > An x86_64 allyesconfig build showed no new warnings. As we do not have
> > > > an Intel Atom ISP platform with the required camera sensor hardware to
> > > > test with, no runtime testing was able to be performed.
> > > 
> > > These last two paragraphs do not suit the commit message. Please, drop them
> > > here and better to describe all this in the cover letter (if not yet).
> > 
> > This is how the documentation says to write commit messages.
> > 
> > https://lore.kernel.org/all/ahgaOigklcDCYvRp@stanley.mountain/
> 
> Isn't it enough to have in the cover letter?
> 
> Thanks for commenting there, but I would insist to move these two paragraphs
> from the commit message here. Maybe Kees is okay with that, I'm thinking that
> this is too much (since we have lore archives).

To me the "An x86_64 allyesconfig build showed no new warnings" information
adds no value at all.  It is assumed and if you didn't do that then a lot
of people are going to know and complain.

regards,
dan carpenter

Re: [PATCH v2 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Dawei Feng]

Hi Andy,

Thanks for the review.

On Mon, 15 Jun 2026 16:51:33 +0300, Andy Shevchenko wrote:
>But did he _develop_ any parts of this patch? Otherwise Reported-by is more
>suitable.

I agree. I will set Zilin as Reported-by in the v3 patch.

>Yes, and I still insist to move them to the cover letter. In any case those are
>not present in the second patch anyway, moving that to cover letter covers the
>entire series (and I believe you tested the entire series, didn't you?).

Yes, I compile-tested the series. I'll move those paragraphs to the v3
cover letter.

Best regards,
Dawei Feng