[PATCH v1 0/3] media: sun4i-csi: fix lifecycle bugs (leak, IRQ, unbind)

[PATCH v1 0/3] media: sun4i-csi: fix lifecycle bugs (leak, IRQ, unbind)

[Cong Nguyen]

While addressing an async-notifier cleanup leak in this driver, I audited
the surrounding device lifecycle and found three separate pre-existing
issues, all present since the driver was introduced. They are independent
of each other; this series groups them because they touch the same driver.

1/3: sun4i_csi_notify_complete() registers the video device and the bridge
     subdev before creating the media links. On a failure in the later
     steps it only unregistered the media device, leaving /dev/videoX
     registered. Since probe then aborts and the devm-managed context
     (embedding the video_device) is freed, an open() would touch freed
     memory. Unwind the registrations in reverse order.

2/3: sun4i_csi_stop_streaming() stops the capture engine but never disables
     the frame-done interrupt or synchronizes with the handler. A late IRQ
     after the block is runtime-suspended (on release) can access gated
     registers and crash. Clear CSI_INT_EN_REG and synchronize_irq().

3/3: The async notifier ops lack an .unbind callback, so csi->src_subdev is
     left dangling when the remote sensor is unbound; a later STREAMON
     dereferences the freed subdev. Add .unbind to unregister the video
     device and clear the pointer, matching stm32-dcmi/atmel-isi/mcam.

All three are marked for stable. Compile-tested (COMPILE_TEST) and
checkpatch --strict clean. I do not have the hardware, so testing on a
real A10/A20 CSI setup would be appreciated.

Cong Nguyen (3):
  media: sun4i-csi: fix video device and subdev leak in
    notify_complete()
  media: sun4i-csi: disable interrupts when stopping streaming
  media: sun4i-csi: add notifier unbind callback to drop the source
    subdev

 .../platform/sunxi/sun4i-csi/sun4i_csi.c      | 25 +++++++++++++++++--
 .../platform/sunxi/sun4i-csi/sun4i_csi.h      |  1 +
 .../platform/sunxi/sun4i-csi/sun4i_dma.c      | 11 ++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v1 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete()

[Cong Nguyen]

sun4i_csi_notify_complete() registers the bridge subdev with
v4l2_device_register_subdev() and the video device with
sun4i_csi_v4l2_register() (which calls video_register_device()) before it
creates the media pad links and registers the subdev nodes. If any of the
later steps fail, the error path only unregistered the media device:

	err_clean_media:
		media_device_unregister(&csi->mdev);
		return ret;

The already registered video device and bridge subdev were left behind.
Because this failure propagates back through v4l2_async_nf_register() and
aborts probe, the driver's devm-managed struct sun4i_csi (which embeds the
video_device) is freed while /dev/videoX is still registered, so a
subsequent open() from userspace dereferences freed memory.

Unwind the registrations in reverse order on error, mirroring the teardown
in sun4i_csi_remove(): unregister the video device with
vb2_video_unregister_device() and the bridge subdev with
v4l2_device_unregister_subdev(). Also unwind the intermediate v4l2/media
registration steps so every early return leaves no half-registered state.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index e53a07b770b7..a8711336a754 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -85,11 +85,11 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 
 	ret = sun4i_csi_v4l2_register(csi);
 	if (ret < 0)
-		return ret;
+		goto err_unregister_subdev;
 
 	ret = media_device_register(&csi->mdev);
 	if (ret)
-		return ret;
+		goto err_unregister_video;
 
 	/* Create link from subdev to main device */
 	ret = media_create_pad_link(&subdev->entity, CSI_SUBDEV_SOURCE,
@@ -114,6 +114,10 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 
 err_clean_media:
 	media_device_unregister(&csi->mdev);
+err_unregister_video:
+	vb2_video_unregister_device(&csi->vdev);
+err_unregister_subdev:
+	v4l2_device_unregister_subdev(subdev);
 
 	return ret;
 }
-- 
2.25.1

[PATCH v1 2/3] media: sun4i-csi: disable interrupts when stopping streaming

[Cong Nguyen]

sun4i_csi_start_streaming() enables the frame-done interrupt in
CSI_INT_EN_REG, but sun4i_csi_stop_streaming() only stops the capture
engine (CSI_CPT_CTRL_REG) via sun4i_csi_capture_stop(). It never disables
the interrupt source nor synchronizes with the handler.

Capture stops at the end of the current frame, so a frame-done interrupt
can still fire shortly after stop_streaming() returns. If userspace then
closes the device, sun4i_csi_release() calls pm_runtime_put() and the CSI
block is powered down (clocks gated, reset asserted). A delayed interrupt
handler would then read/write CSI registers on the gated block, which can
hang or crash the system.

Clear CSI_INT_EN_REG and call synchronize_irq() in stop_streaming(), before
returning the active buffers and freeing the scratch buffer, so no handler
can run past this point. Store the IRQ number in struct sun4i_csi so it is
available here.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h |  1 +
 drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
index 4e0c2df45d4d..51173faea871 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
@@ -112,6 +112,7 @@ struct sun4i_csi {
 	const struct sun4i_csi_traits	*traits;
 
 	void __iomem			*regs;
+	int				irq;
 	struct clk			*bus_clk;
 	struct clk			*isp_clk;
 	struct clk			*ram_clk;
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
index e911c7f7acc5..da697f39f2bc 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
@@ -354,6 +354,16 @@ static void sun4i_csi_stop_streaming(struct vb2_queue *vq)
 	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
 	sun4i_csi_capture_stop(csi);
 
+	/*
+	 * Disable the frame done interrupt and wait for the handler to
+	 * finish. A frame may complete right as capture is stopped, so an
+	 * interrupt can still be pending here; without this the handler could
+	 * run after the device is powered down (pm_runtime_put() on release)
+	 * and access registers on a gated block.
+	 */
+	writel(0, csi->regs + CSI_INT_EN_REG);
+	synchronize_irq(csi->irq);
+
 	/* Release all active buffers */
 	spin_lock_irqsave(&csi->qlock, flags);
 	return_all_buffers(csi, VB2_BUF_STATE_ERROR);
@@ -438,6 +448,7 @@ int sun4i_csi_dma_register(struct sun4i_csi *csi, int irq)
 		dev_err(csi->dev, "Couldn't register our interrupt\n");
 		goto err_unregister_device;
 	}
+	csi->irq = irq;
 
 	return 0;
 
-- 
2.25.1

[PATCH v1 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev

[Cong Nguyen]

sun4i_csi_notify_ops only implements .bound and .complete. When the remote
sensor's subdevice goes away (e.g. its module is unloaded), the V4L2 async
core unbinds and frees it, but the driver keeps the stale pointer in
csi->src_subdev and leaves the video node registered.

A subsequent VIDIOC_STREAMON reaches sun4i_csi_start_streaming(), which
calls v4l2_subdev_call(csi->src_subdev, video, s_stream, 1) on the freed
subdev, resulting in a use-after-free.

Add an .unbind callback that unregisters the video device so userspace can
no longer start streaming, and clears csi->src_subdev. Unregistering the
already-unregistered video device again in sun4i_csi_remove() is harmless
(vb2_video_unregister_device() is a no-op when it is not registered).

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 .../media/platform/sunxi/sun4i-csi/sun4i_csi.c  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index a8711336a754..6610ada1c06d 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -122,8 +122,25 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 	return ret;
 }
 
+static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
+				    struct v4l2_subdev *subdev,
+				    struct v4l2_async_connection *asd)
+{
+	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
+					     notifier);
+
+	/*
+	 * The remote subdev is being freed. Tear down the video node so
+	 * userspace can no longer reach sun4i_csi_start_streaming() and
+	 * dereference the now dangling source subdev, and drop the pointer.
+	 */
+	vb2_video_unregister_device(&csi->vdev);
+	csi->src_subdev = NULL;
+}
+
 static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
 	.bound		= sun4i_csi_notify_bound,
+	.unbind		= sun4i_csi_notify_unbind,
 	.complete	= sun4i_csi_notify_complete,
 };
 
-- 
2.25.1

Re: [PATCH v1 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The driver attempts to re-register an embedded `video_device` after it has been unregistered, which violates kobject lifecycle rules.

Pre-existing issues:
- [High] The `.unbind` callback performs incomplete resource teardown, leading to list corruption on re-bind.
- [High] The `sun4i_csi` structure is allocated using `devm_kzalloc`, but its embedded `video_device` relies on `video_device_release_empty`, creating a Use-After-Free.
--

--- Patch [3]: [PATCH v1 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev ---
commit 682f5e1386d2733cf9627a62093a8bdd3a593a37
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: add notifier unbind callback to drop the source subdev

This commit adds an .unbind callback to the V4L2 async notifier for the
sun4i-csi driver. It unregisters the video device and drops the source
subdev pointer. This is intended to prevent a use-after-free when the
remote sensor module is unloaded.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index a8711336a7540..6610ada1c06d1 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -122,8 +122,25 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  	return ret;
>  }
>  
> +static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
> +				    struct v4l2_subdev *subdev,
> +				    struct v4l2_async_connection *asd)
> +{
> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +
> +	/*
> +	 * The remote subdev is being freed. Tear down the video node so
> +	 * userspace can no longer reach sun4i_csi_start_streaming() and
> +	 * dereference the now dangling source subdev, and drop the pointer.
> +	 */
> +	vb2_video_unregister_device(&csi->vdev);

[Severity: High]
This is a pre-existing issue, but the new .unbind callback performs an 
asymmetric teardown.

If the remote sensor module is unloaded, the .unbind callback is triggered 
but fails to undo the local subdev and media device registrations. When the 
sensor is reloaded, sun4i_csi_notify_complete() runs again and calls 
v4l2_device_register_subdev() on the already-registered subdev.

Can this lead to list corruption and a potential kernel panic?


[Severity: High]
When this unbind callback unregisters the embedded video device (csi->vdev),
it ends the lifecycle of its kobject.

When the sensor is re-bound, sun4i_csi_notify_complete() invokes 
sun4i_csi_v4l2_register(), which calls video_register_device() on the 
same unregistered, embedded video_device. Re-initializing a released 
kobject is forbidden and can lead to kernel warnings and potential panics.

Can the driver be restructured to avoid re-registering an embedded video
device that has already been unregistered?


[Severity: High]
This is a pre-existing issue, but the sun4i_csi structure is allocated using 
devm_kzalloc() in sun4i_csi_probe(), while its embedded video_device 
relies on video_device_release_empty.

If an unprivileged user opens /dev/videoX and the driver is unbound, 
the video device is unregistered. This eventually causes devres to free the 
sun4i_csi structure. When the user later closes the file descriptor, 
sun4i_csi_release() accesses the freed csi pointer to lock csi->lock.

Does this create a Use-After-Free condition when the file descriptor is 
closed after the driver is unbound?

> +	csi->src_subdev = NULL;
> +}
> +
>  static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
>  	.bound		= sun4i_csi_notify_bound,
> +	.unbind		= sun4i_csi_notify_unbind,
>  	.complete	= sun4i_csi_notify_complete,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786184456.git.congnt264@gmail.com?part=3

Re: [PATCH v1 2/3] media: sun4i-csi: disable interrupts when stopping streaming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] Missing read-back to flush the posted write before `synchronize_irq()`.

Pre-existing issues:
- [High] The error path in `sun4i_csi_start_streaming()` fails to disable the frame-done interrupt and synchronize the IRQ, causing DMA use-after-free and system crashes.
- [High] The `sun4i_csi` structure is subjected to a Use-After-Free because its embedded `video_device` uses an empty release callback.
- [High] The driver frees DMA buffers while the hardware may still be actively writing to them.
--

--- Patch [2]: [PATCH v1 2/3] media: sun4i-csi: disable interrupts when stopping streaming ---
commit dbface35706640e625b0b1ec0a6ec2b6918d0fc4
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: disable interrupts when stopping streaming

sun4i_csi_start_streaming() enables the frame-done interrupt in
CSI_INT_EN_REG, but sun4i_csi_stop_streaming() only stops the capture
engine (CSI_CPT_CTRL_REG) via sun4i_csi_capture_stop().

Clear CSI_INT_EN_REG and call synchronize_irq() in stop_streaming(), before
returning the active buffers and freeing the scratch buffer, so no handler
can run past this point.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> index 4e0c2df45d4d2..51173faea8719 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
> @@ -112,6 +112,7 @@ struct sun4i_csi {
>  	const struct sun4i_csi_traits	*traits;
>  
>  	void __iomem			*regs;
> +	int				irq;
>  	struct clk			*bus_clk;
>  	struct clk			*isp_clk;
>  	struct clk			*ram_clk;

[Severity: High]
This isn't a regression introduced by this patch, but since the struct is
modified here, is it possible this structure suffers from a use-after-free?

The sun4i_csi structure is allocated using devm_kzalloc() in sun4i_csi_probe():

    csi = devm_kzalloc(&pdev->dev, sizeof(*csi), GFP_KERNEL);

However, the embedded video_device struct sets its release callback to
video_device_release_empty in sun4i_csi_v4l2_register():

    vdev->release = video_device_release_empty;

This breaks the V4L2 reference counting model. If the device is unbound while
a userspace application holds the video device file descriptor open,
devm_kzalloc frees the csi structure. A subsequent close of the file
descriptor or ioctl will access the freed csi structure, leading to a
use-after-free.

[ ... ]

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> index 4781db21c2055..8804189a6e38c 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> @@ -356,6 +356,16 @@ static void sun4i_csi_stop_streaming(struct vb2_queue *vq)
>  	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
>  	sun4i_csi_capture_stop(csi);
>  
> +	/*
> +	 * Disable the frame done interrupt and wait for the handler to
> +	 * finish. A frame may complete right as capture is stopped, so an
> +	 * interrupt can still be pending here; without this the handler could
> +	 * run after the device is powered down (pm_runtime_put() on release)
> +	 * and access registers on a gated block.
> +	 */
> +	writel(0, csi->regs + CSI_INT_EN_REG);
> +	synchronize_irq(csi->irq);

[Severity: High]
Does this regression need a read back to flush the posted write before calling
synchronize_irq()?

writel() performs a posted write on the memory bus, meaning it is buffered
and may not reach the interrupt controller immediately. Without a dummy
readl() to the same register to flush the write buffer, synchronize_irq()
may execute and return before the hardware actually disables the interrupt.

This could leave a race window where the device could raise a new interrupt
just after synchronize_irq() finishes, leading to a handler running after
the device is powered down or buffers are freed.

> +
>  	/* Release all active buffers */
>  	spin_lock_irqsave(&csi->qlock, flags);
>  	return_all_buffers(csi, VB2_BUF_STATE_ERROR);

[Severity: High]
This isn't a regression introduced by this patch, but could this code free DMA
buffers while the hardware may still be actively writing to them?

As noted in the commit message, stopping the capture engine via
sun4i_csi_capture_stop() only stops it at the end of the current frame.
However, sun4i_csi_stop_streaming() immediately frees the DMA scratch buffer
and returns all active buffers to the core without waiting for the hardware
to become idle.

Although the interrupt is disabled, the hardware DMA engine remains active
and will continue writing the remainder of the frame into the freed scratch
memory or into buffers that userspace might have already reclaimed.

Additionally, would the error path in sun4i_csi_start_streaming() also suffer
from the same problem?

drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c:sun4i_csi_start_streaming() {
    ...
    ret = v4l2_subdev_call(csi->src_subdev, video, s_stream, 1);
    if (ret < 0 && ret != -ENOIOCTLCMD)
        goto err_disable_device;

    return 0;

err_disable_device:
    sun4i_csi_capture_stop(csi);
    ...
err_free_scratch_buffer:
    dma_free_coherent(csi->dev, csi->scratch.size, csi->scratch.vaddr,
                      csi->scratch.paddr);

err_clear_dma_queue:
    spin_lock_irqsave(&csi->qlock, flags);
    return_all_buffers(csi, VB2_BUF_STATE_QUEUED);
    spin_unlock_irqrestore(&csi->qlock, flags);
    ...
}

The err_disable_device path stops capture but does not disable the interrupt
or wait for the hardware to finish before freeing the scratch buffer and
returning all queued buffers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786184456.git.congnt264@gmail.com?part=2

Re: [PATCH v1 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver embeds `struct video_device` inside a `devm`-managed structure and uses `video_device_release_empty`, causing a Use-After-Free (UAF) vulnerability. This problem wasn't introduced by this patch, but the patch fails to resolve the underlying race condition when unwinding registrations.
- [Medium] Missing `v4l2_async_nf_cleanup()` in the `sun4i_csi_probe()` error path causes a memory and fwnode reference leak of async connections. This problem wasn't introduced by this patch.
--

--- Patch [1]: [PATCH v1 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete() ---
commit 1bfbebedd93671de734cc0e7671a5f4e99b9ff15
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: fix video device and subdev leak in notify_complete()

This patch fixes an issue in sun4i_csi_notify_complete() where the video
device and bridge subdev were left registered if later initialization steps
failed. It unwinds the registrations in reverse order to avoid leaving the
driver in a half-registered state.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index e53a07b770b7e..a8711336a7540 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c

[ ... ]

> @@ -114,6 +114,10 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  
>  err_clean_media:
>  	media_device_unregister(&csi->mdev);
> +err_unregister_video:
> +	vb2_video_unregister_device(&csi->vdev);

[Severity: High]
This is a pre-existing issue, but could this error handling path lead to a
use-after-free regression if userspace races to open the device?

Because the csi structure is allocated with devm_kzalloc() in
sun4i_csi_probe(), it will be automatically freed when probe fails.
Meanwhile, sun4i_csi_v4l2_register() sets vdev->release to
video_device_release_empty.

If media_device_register() fails, this new error path calls
vb2_video_unregister_device(), which unregisters the sysfs node but cannot
revoke file descriptors if userspace has already opened /dev/videoX.

When the file descriptor is eventually closed, could this lead to a
dereference of the freed csi structure in sun4i_csi_release() and when
the V4L2 core attempts to put the device?

> +err_unregister_subdev:
> +	v4l2_device_unregister_subdev(subdev);
>  
>  	return ret;
>  }

[Severity: Medium]
This isn't a bug introduced by this patch, but does the error path in
sun4i_csi_probe() cause a memory leak regression for async connection
structures and fwnode references?

When sun4i_csi_notifier_init() succeeds, it allocates an async connection
and grabs a fwnode reference. If the subsequent v4l2_async_nf_register()
call fails, the error path exits without calling v4l2_async_nf_cleanup():

drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c:sun4i_csi_probe() {
    ...
err_unregister_media:
	media_device_unregister(&csi->mdev);
    ...
}

Could this result in abandoning those allocations and leaking fwnode
references since the cleanup isn't performed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1786184456.git.congnt264@gmail.com?part=1

[PATCH v2 0/3] media: sun4i-csi: fix probe/streaming lifecycle bugs

[Cong Nguyen]

This series fixes three independent, pre-existing lifecycle bugs in the
sun4i-csi driver, all present since the driver was added (577bbf23b758):

  1/3 - notify_complete() left the video device and bridge subdev
        registered if a later step failed, so /dev/videoX stayed live over
        the freed devm context.
  2/3 - the frame-done interrupt was never disabled on the stop path, so a
        late IRQ could touch a gated block after runtime suspend.
  3/3 - the async notifier had no .unbind, so the cached source subdev
        pointer dangled after the sensor unbound.

Changes in v2:
  - 2/3: read CSI_INT_EN_REG back after disabling it to flush the posted
    write before synchronize_irq() (Sashiko AI review).
  - 2/3: apply the same interrupt teardown to the start_streaming() error
    path (err_disable_device), which had the identical defect - the IRQ is
    enabled and capture started before the s_stream call that can fail.
    Both paths now share a new sun4i_csi_disable_irq() helper.
  - 3/3: only clear csi->src_subdev in .unbind; drop the
    vb2_video_unregister_device() call. Since notify_complete() registers
    the video device, unregistering it on unbind would make it re-register
    an already-unregistered embedded video_device on a subsequent re-bind.
    Clearing the cached pointer alone prevents the use-after-free.
  - No functional changes to 1/3.

Two further pre-existing issues raised in review are not addressed here, as
they are out of scope for these fixes: the embedded video_device using
video_device_release_empty in a devm-allocated struct (a broader lifetime
rework), and notify_complete() not being re-bind safe (it re-registers the
bridge subdev, which needs a larger restructuring of the notifier flow).

Cong Nguyen (3):
  media: sun4i-csi: fix video device and subdev leak in
    notify_complete()
  media: sun4i-csi: disable interrupts when stopping streaming
  media: sun4i-csi: add notifier unbind callback to drop the source
    subdev

 .../platform/sunxi/sun4i-csi/sun4i_csi.c      | 25 +++++++++++++++++--
 .../platform/sunxi/sun4i-csi/sun4i_csi.h      |  1 +
 .../platform/sunxi/sun4i-csi/sun4i_dma.c      | 20 +++++++++++++++
 3 files changed, 44 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete()

[Cong Nguyen]

sun4i_csi_notify_complete() registers the bridge subdev with
v4l2_device_register_subdev() and the video device with
sun4i_csi_v4l2_register() (which calls video_register_device()) before it
creates the media pad links and registers the subdev nodes. If any of the
later steps fail, the error path only unregistered the media device:

	err_clean_media:
		media_device_unregister(&csi->mdev);
		return ret;

The already registered video device and bridge subdev were left behind.
Because this failure propagates back through v4l2_async_nf_register() and
aborts probe, the driver's devm-managed struct sun4i_csi (which embeds the
video_device) is freed while /dev/videoX is still registered, so a
subsequent open() from userspace dereferences freed memory.

Unwind the registrations in reverse order on error, mirroring the teardown
in sun4i_csi_remove(): unregister the video device with
vb2_video_unregister_device() and the bridge subdev with
v4l2_device_unregister_subdev(). Also unwind the intermediate v4l2/media
registration steps so every early return leaves no half-registered state.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index e53a07b770b7..a8711336a754 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -85,11 +85,11 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 
 	ret = sun4i_csi_v4l2_register(csi);
 	if (ret < 0)
-		return ret;
+		goto err_unregister_subdev;
 
 	ret = media_device_register(&csi->mdev);
 	if (ret)
-		return ret;
+		goto err_unregister_video;
 
 	/* Create link from subdev to main device */
 	ret = media_create_pad_link(&subdev->entity, CSI_SUBDEV_SOURCE,
@@ -114,6 +114,10 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 
 err_clean_media:
 	media_device_unregister(&csi->mdev);
+err_unregister_video:
+	vb2_video_unregister_device(&csi->vdev);
+err_unregister_subdev:
+	v4l2_device_unregister_subdev(subdev);
 
 	return ret;
 }
-- 
2.25.1

[PATCH v2 2/3] media: sun4i-csi: disable interrupts when stopping streaming

[Cong Nguyen]

sun4i_csi_start_streaming() enables the frame-done interrupt in
CSI_INT_EN_REG, but sun4i_csi_stop_streaming() only stops the capture
engine (CSI_CPT_CTRL_REG) via sun4i_csi_capture_stop(). It never disables
the interrupt source nor synchronizes with the handler.

Capture stops at the end of the current frame, so a frame-done interrupt
can still fire shortly after stop_streaming() returns. If userspace then
closes the device, sun4i_csi_release() calls pm_runtime_put() and the CSI
block is powered down (clocks gated, reset asserted). A delayed interrupt
handler would then read/write CSI registers on the gated block, which can
hang or crash the system.

Add a helper that clears CSI_INT_EN_REG, reads it back to flush the posted
write, and calls synchronize_irq(), so no handler can run past that point.
Call it from stop_streaming() before returning the active buffers and
freeing the scratch buffer.

The start_streaming() error path (err_disable_device) has the same
problem: the frame-done interrupt is enabled and capture is started
before the s_stream call that can fail, so a handler can run while the
error path frees the scratch buffer and returns the queued buffers.
Apply the same teardown there. Store the IRQ number in struct sun4i_csi
so it is available in both paths.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 .../platform/sunxi/sun4i-csi/sun4i_csi.h      |  1 +
 .../platform/sunxi/sun4i-csi/sun4i_dma.c      | 20 +++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
index 4e0c2df45d4d..51173faea871 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.h
@@ -112,6 +112,7 @@ struct sun4i_csi {
 	const struct sun4i_csi_traits	*traits;
 
 	void __iomem			*regs;
+	int				irq;
 	struct clk			*bus_clk;
 	struct clk			*isp_clk;
 	struct clk			*ram_clk;
diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
index e911c7f7acc5..fd83dbd1a3ff 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
@@ -43,6 +43,23 @@ static void sun4i_csi_capture_stop(struct sun4i_csi *csi)
 	writel(0, csi->regs + CSI_CPT_CTRL_REG);
 }
 
+static void sun4i_csi_disable_irq(struct sun4i_csi *csi)
+{
+	/*
+	 * Disable the frame done interrupt and wait for the handler to
+	 * finish. A frame may complete right as capture is stopped, so an
+	 * interrupt can still be pending here; without this the handler could
+	 * run after the device is powered down (pm_runtime_put() on release)
+	 * and access registers on a gated block.
+	 *
+	 * Read the register back to flush the posted write so the disable has
+	 * reached the device before synchronize_irq() waits for the handler.
+	 */
+	writel(0, csi->regs + CSI_INT_EN_REG);
+	readl(csi->regs + CSI_INT_EN_REG);
+	synchronize_irq(csi->irq);
+}
+
 static int sun4i_csi_queue_setup(struct vb2_queue *vq,
 				 unsigned int *nbuffers,
 				 unsigned int *nplanes,
@@ -328,6 +345,7 @@ static int sun4i_csi_start_streaming(struct vb2_queue *vq, unsigned int count)
 
 err_disable_device:
 	sun4i_csi_capture_stop(csi);
+	sun4i_csi_disable_irq(csi);
 
 err_disable_pipeline:
 	video_device_pipeline_stop(&csi->vdev);
@@ -353,6 +371,7 @@ static void sun4i_csi_stop_streaming(struct vb2_queue *vq)
 
 	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
 	sun4i_csi_capture_stop(csi);
+	sun4i_csi_disable_irq(csi);
 
 	/* Release all active buffers */
 	spin_lock_irqsave(&csi->qlock, flags);
@@ -438,6 +457,7 @@ int sun4i_csi_dma_register(struct sun4i_csi *csi, int irq)
 		dev_err(csi->dev, "Couldn't register our interrupt\n");
 		goto err_unregister_device;
 	}
+	csi->irq = irq;
 
 	return 0;
 
-- 
2.25.1

[PATCH v2 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev

[Cong Nguyen]

sun4i_csi_notify_ops only implements .bound and .complete. The .bound
callback caches the remote sensor's subdevice in csi->src_subdev. When
that subdevice goes away (e.g. its module is unloaded), the V4L2 async
core frees it, but without an .unbind callback the driver keeps the
stale pointer.

A subsequent VIDIOC_STREAMON reaches sun4i_csi_start_streaming(), which
calls v4l2_subdev_call(csi->src_subdev, video, s_stream, 1) on the freed
subdev, resulting in a use-after-free.

Add an .unbind callback that clears csi->src_subdev. The pointer is only
dereferenced by v4l2_subdev_call() (NULL-guarded, returns -ENODEV, so
streaming fails cleanly) and by sun4i_csi_notify_complete() (which only
runs while a subdev is bound), so clearing it is sufficient to prevent
the use-after-free.

Fixes: 577bbf23b758 ("media: sunxi: Add A10 CSI driver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4
Signed-off-by: Cong Nguyen <congnt264@gmail.com>
---
 .../media/platform/sunxi/sun4i-csi/sun4i_csi.c  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index a8711336a754..f19508f2c7ed 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -122,8 +122,25 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
 	return ret;
 }
 
+static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
+				    struct v4l2_subdev *subdev,
+				    struct v4l2_async_connection *asd)
+{
+	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
+					     notifier);
+
+	/*
+	 * The remote subdev is being freed. Drop our cached pointer so that a
+	 * subsequent sun4i_csi_start_streaming() does not issue a
+	 * v4l2_subdev_call() on the now dangling source subdev. The call is
+	 * NULL-guarded and returns -ENODEV, so streaming fails cleanly.
+	 */
+	csi->src_subdev = NULL;
+}
+
 static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
 	.bound		= sun4i_csi_notify_bound,
+	.unbind		= sun4i_csi_notify_unbind,
 	.complete	= sun4i_csi_notify_complete,
 };
 
-- 
2.25.1

Re: [PATCH v2 2/3] media: sun4i-csi: disable interrupts when stopping streaming

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] DMA use-after-free and memory corruption due to freeing buffers while capture engine is still running
--

--- Patch [2]: [PATCH v2 2/3] media: sun4i-csi: disable interrupts when stopping streaming ---
commit f908e51d92a92db663d06b38deb5bee451c659e7
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: disable interrupts when stopping streaming

This commit addresses a crash where a delayed frame-done interrupt could
access the device after it is powered down. It fixes this by explicitly
disabling the interrupt and waiting for the handler to complete before
proceeding with device shutdown.

Cc: stable@vger.kernel.org

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> index 4781db21c2055..3401a91cb310f 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
[ ... ]
> @@ -330,6 +347,7 @@ static int sun4i_csi_start_streaming(struct vb2_queue *vq, unsigned int count)
>  
>  err_disable_device:
>  	sun4i_csi_capture_stop(csi);
> +	sun4i_csi_disable_irq(csi);
>  
>  err_disable_pipeline:
>  	video_device_pipeline_stop(&csi->vdev);

[Severity: Critical]
This isn't a bug introduced by this patch, but does the hardware DMA engine
continue writing to memory after sun4i_csi_capture_stop() returns?

As noted in the commit message, capture stops at the end of the current
frame. If the DMA engine is still running, it looks like we might free the
scratch buffer immediately afterwards in the error path:

err_free_scratch_buffer:
	dma_free_coherent(csi->dev, csi->scratch.size, csi->scratch.vaddr,
			  csi->scratch.paddr);

Could this cause memory corruption if the hardware writes to the freed
scratch buffer before it finishes the current frame?

> @@ -355,6 +373,7 @@ static void sun4i_csi_stop_streaming(struct vb2_queue *vq)
>  
>  	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
>  	sun4i_csi_capture_stop(csi);
> +	sun4i_csi_disable_irq(csi);
>  
>  	/* Release all active buffers */
>  	spin_lock_irqsave(&csi->qlock, flags);

[Severity: Critical]
This is a pre-existing issue, but similar to the error path above, are we
returning active buffers to userspace while the DMA engine might still be
actively writing to them?

	sun4i_csi_capture_stop(csi);
	sun4i_csi_disable_irq(csi);

	/* Release all active buffers */
	spin_lock_irqsave(&csi->qlock, flags);
	return_all_buffers(csi, VB2_BUF_STATE_ERROR);
	spin_unlock_irqrestore(&csi->qlock, flags);

	video_device_pipeline_stop(&csi->vdev);

	dma_free_coherent(csi->dev, csi->scratch.size, csi->scratch.vaddr,
			  csi->scratch.paddr);

If capture stops asynchronously at the end of the frame, returning these
buffers and freeing the scratch buffer immediately afterwards could allow
the DMA engine to overwrite memory that has already been reclaimed or
reused. Is there a need to poll or wait for the DMA engine to completely
halt before cleaning up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810062521.1709379-1-congnt264@gmail.com?part=2

Re: [PATCH v2 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race Condition / Use-After-Free via TOCTOU during probe failure
--

--- Patch [1]: [PATCH v2 1/3] media: sun4i-csi: fix video device and subdev leak in notify_complete() ---
commit 5302a06684e2be656ddb94e9e33fdbca0536135f
Author: Cong Nguyen <congnt264@gmail.com>
media: sun4i-csi: fix video device and subdev leak in notify_complete()

This commit fixes resource leaks in sun4i_csi_notify_complete() by unwinding
registrations in reverse order on error paths. It explicitly unregisters the
video device and bridge subdev if intermediate registration steps fail.

Cc: stable@vger.kernel.org

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
[ ... ]
> @@ -114,6 +114,10 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  
>  err_clean_media:
>  	media_device_unregister(&csi->mdev);
> +err_unregister_video:
> +	vb2_video_unregister_device(&csi->vdev);

[Severity: High]
This is a pre-existing issue, but does this error path leave a use-after-free
vulnerability if userspace opens the device node before it gets unregistered?

The video_device is embedded in struct sun4i_csi which is allocated with
devm_kzalloc() during sun4i_csi_probe(). In sun4i_csi_v4l2_register(), the
release callback is overridden:

vdev->release = video_device_release_empty;

If sun4i_csi_notify_complete() executes synchronously during probe and exposes
the device node, userspace can open it immediately. If a subsequent setup step
fails, this error path unregisters the video device and returns the error back
up to probe().

When probe() fails, the devres core automatically frees the sun4i_csi memory.
Because video_device_release_empty is used instead of a refcounted release,
any active file descriptors held by userspace will now point to freed memory,
causing a use-after-free on subsequent access or close.

> +err_unregister_subdev:
> +	v4l2_device_unregister_subdev(subdev);
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810062521.1709379-1-congnt264@gmail.com?part=1

Re: [PATCH v2 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The newly added `.unbind` callback clears `csi->src_subdev` without holding `csi->lock`, creating a time-of-check to time-of-use (TOCTOU) race condition with streaming operations.

Pre-existing issues:
- [High] The `.unbind` callback fails to tear down video and media devices registered in `.complete`, leading to double-registration crashes and resource leaks upon dynamic rebind.
--

--- Patch [3]: [PATCH v2 3/3] media: sun4i-csi: add notifier unbind callback to drop the source subdev ---
commit 25c69500b21e1d763945dbe2dba9e12acab04857
Author: Cong Nguyen <congnt264@gmail.com>

    media: sun4i-csi: add notifier unbind callback to drop the source subdev
    
    sun4i_csi_notify_ops only implements .bound and .complete. The .bound
    callback caches the remote sensor's subdevice in csi->src_subdev. When
    that subdevice goes away (e.g. its module is unloaded), the V4L2 async
    core frees it, but without an .unbind callback the driver keeps the
    stale pointer.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index a8711336a7540..f19508f2c7ed4 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -122,8 +122,25 @@ static int sun4i_csi_notify_complete(struct v4l2_async_notifier *notifier)
>  	return ret;
>  }
>  
> +static void sun4i_csi_notify_unbind(struct v4l2_async_notifier *notifier,
> +				    struct v4l2_subdev *subdev,
> +				    struct v4l2_async_connection *asd)
> +{

[Severity: High]
This is a pre-existing issue, but doesn't this callback need to clean up the
video and media devices that were registered in sun4i_csi_notify_complete()?

If the remote sensor module is unbound and then re-bound,
sun4i_csi_notify_complete() will execute again. Since the previous devices
were not unregistered, calling video_register_device() on an already-registered
device can perform a double list_add and corrupt video_device_list.

> +	struct sun4i_csi *csi = container_of(notifier, struct sun4i_csi,
> +					     notifier);
> +
> +	/*
> +	 * The remote subdev is being freed. Drop our cached pointer so that a
> +	 * subsequent sun4i_csi_start_streaming() does not issue a
> +	 * v4l2_subdev_call() on the now dangling source subdev. The call is
> +	 * NULL-guarded and returns -ENODEV, so streaming fails cleanly.
> +	 */
> +	csi->src_subdev = NULL;

[Severity: High]
Could this introduce a time-of-check to time-of-use race condition with
streaming operations?

This clears csi->src_subdev without holding csi->lock. Meanwhile, functions
like sun4i_csi_start_streaming() and sun4i_csi_stop_streaming() in sun4i_dma.c
evaluate csi->src_subdev via v4l2_subdev_call() while holding csi->lock.

If the streamon ioctl enters sun4i_csi_start_streaming(), v4l2_subdev_call()
will cache the csi->src_subdev pointer into a local variable. If the remote
sensor module is concurrently unloaded, sun4i_csi_notify_unbind() will clear
csi->src_subdev without acquiring the lock and return, allowing the async core
to free the subdevice. The streaming thread could then resume and dereference
the freed subdevice pointer.

> +}
> +
>  static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
>  	.bound		= sun4i_csi_notify_bound,
> +	.unbind		= sun4i_csi_notify_unbind,
>  	.complete	= sun4i_csi_notify_complete,
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810062521.1709379-1-congnt264@gmail.com?part=3