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

[PATCH v3 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.

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.

Changes in v3:
- Moved the research background and static analysis explanation to the
  cover letter.
- Changed Zilin's Signed-off-by to Reported-by.

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

Fixes: 9d4fa1a16b28 ("media: atomisp: cleanup directory hierarchy")
Reported-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 v3 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")
Reported-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 v3 1/2] media: atomisp: fix memory leak in atomisp_pci_probe()

[Andy Shevchenko]

On Tue, Jun 16, 2026 at 09:43:18PM +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.

...

>  void atomisp_mipi_csi2_cleanup(struct atomisp_device *isp)
>  {
> +	unsigned int i;
> +
> +	for (i = 0; i < ATOMISP_CAMERA_NR_PORTS; i++)

In case you need a new version

	for (unsigned int i = 0; i < ATOMISP_CAMERA_NR_PORTS; i++)

> +		media_entity_cleanup(&isp->csi2_port[i].subdev.entity);
>  }

Or maybe Sakari can tweak this whilst applying.

-- 
With Best Regards,
Andy Shevchenko