All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails
@ 2026-08-08  9:11 Cong Nguyen
  2026-08-08  9:23 ` sashiko-bot
  0 siblings, 1 reply; 3+ messages in thread
From: Cong Nguyen @ 2026-08-08  9:11 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
	Mauro Carvalho Chehab, linux-media, linux-sunxi, linux-arm-kernel,
	linux-kernel

sun4i_csi_notifier_init() adds a v4l2_async_connection to the notifier's
waiting list with v4l2_async_nf_add_fwnode_remote(). If the subsequent
v4l2_async_nf_register() then fails, probe jumps to err_unregister_media,
which never calls v4l2_async_nf_cleanup(). The core does not free the
waiting list on a failed registration, so the async connection is leaked.

Add an err_clean_notifier label that cleans up the notifier on the
registration-failure path, matching the teardown done in
sun4i_csi_remove(). The notifier-init failure path adds nothing to the
notifier and continues to skip the cleanup.

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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
index e53a07b770b7..a8db851ab5a5 100644
--- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
+++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
@@ -255,13 +255,15 @@ static int sun4i_csi_probe(struct platform_device *pdev)
 	ret = v4l2_async_nf_register(&csi->notifier);
 	if (ret) {
 		dev_err(csi->dev, "Couldn't register our notifier.\n");
-		goto err_unregister_media;
+		goto err_clean_notifier;
 	}
 
 	pm_runtime_enable(&pdev->dev);
 
 	return 0;
 
+err_clean_notifier:
+	v4l2_async_nf_cleanup(&csi->notifier);
 err_unregister_media:
 	media_device_unregister(&csi->mdev);
 	sun4i_csi_dma_unregister(csi);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails
  2026-08-08  9:11 [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails Cong Nguyen
@ 2026-08-08  9:23 ` sashiko-bot
  2026-08-08 11:20   ` Nguyễn Công
  0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-08-08  9:23 UTC (permalink / raw)
  To: Cong Nguyen; +Cc: media-ci, linux-sunxi

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

Pre-existing issues:
- [High] Missing cleanup of registered video and subdev devices in `sun4i_csi_notify_complete()` error paths leads to device leaks and potential Use-After-Free.
- [High] The driver does not disable hardware interrupts or synchronize the IRQ handler when stopping capture, allowing an interrupt to crash the system after device suspension.
- [High] Missing `.unbind` callback allows the video device to retain and access a freed subdevice pointer (`csi->src_subdev`).
--

--- Patch [1]: [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails ---
commit d13febea573e64b9e585c2c1f9b472e41737683c
Author: Cong Nguyen <congnt264@gmail.com>

media: sun4i-csi: fix async connection leak when notifier registration fails
    
sun4i_csi_notifier_init() adds a v4l2_async_connection to the notifier's
waiting list with v4l2_async_nf_add_fwnode_remote(). If the subsequent
v4l2_async_nf_register() then fails, probe jumps to err_unregister_media,
which never calls v4l2_async_nf_cleanup(). The core does not free the
waiting list on a failed registration, so the async connection is leaked.
    
Add an err_clean_notifier label that cleans up the notifier on the
registration-failure path, matching the teardown done in
sun4i_csi_remove(). The notifier-init failure path adds nothing to the
notifier and continues to skip the cleanup.

> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> index e53a07b770b7e..a8db851ab5a59 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> @@ -255,13 +255,15 @@ static int sun4i_csi_probe(struct platform_device *pdev)
>  	ret = v4l2_async_nf_register(&csi->notifier);
>  	if (ret) {
>  		dev_err(csi->dev, "Couldn't register our notifier.\n");
> -		goto err_unregister_media;
> +		goto err_clean_notifier;
>  	}
>  
>  	pm_runtime_enable(&pdev->dev);
>  
>  	return 0;
>  
> +err_clean_notifier:
> +	v4l2_async_nf_cleanup(&csi->notifier);
>  err_unregister_media:
>  	media_device_unregister(&csi->mdev);
>  	sun4i_csi_dma_unregister(csi);

[Severity: High]
This is a pre-existing issue, but does sun4i_csi_notify_complete() leak
registered video and subdev devices in its error paths?

If media_create_pad_link() or other subsequent operations fail, the error
path err_clean_media only unregisters the media device:

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

	return ret;
}

It doesn't seem to unregister the video device or the subdev. Because this
failure propagates back to v4l2_async_nf_register(), the probe will fail
and the driver's devm-managed memory is freed.

Since the /dev/videoX node remains active but points to freed memory,
could userspace access trigger a use-after-free?

[Severity: High]
This isn't a bug introduced by this patch, but is the driver leaving
hardware interrupts enabled when stopping capture?

In sun4i_csi_stop_streaming(), the hardware capture is stopped, but the
interrupt enable register doesn't appear to be cleared and there is no
call to synchronize_irq():

drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c:sun4i_csi_stop_streaming() {
    ...
	v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
	sun4i_csi_capture_stop(csi);

	/* Release all active buffers */
    ...
}

Because capture stops at the end of the current frame, a frame-done
interrupt could fire shortly after this function returns. If userspace
closes the device, sun4i_csi_release() invokes pm_runtime_put(), powering
down the hardware block.

When the delayed interrupt fires, could the interrupt handler attempt to read
registers on powered-down hardware and cause a system crash?

drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c:sun4i_csi_irq() {
    ...
	reg = readl(csi->regs + CSI_INT_STA_REG);

	/* Acknowledge the interrupts */
	writel(reg, csi->regs + CSI_INT_STA_REG);
    ...
}

[Severity: High]
This is a pre-existing issue, but is sun4i_csi_notify_ops missing an
.unbind callback?

drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c:
    ...
static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
	.bound		= sun4i_csi_notify_bound,
	.complete	= sun4i_csi_notify_complete,
};
    ...

If the remote sensor module is unloaded, the V4L2 async core unbinds
and frees the subdevice, but it appears csi->src_subdev is never cleared
and the video node is never unregistered.

If userspace calls VIDIOC_STREAMON afterward, does sun4i_csi_start_streaming()
dereference the freed csi->src_subdev pointer here?

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails
  2026-08-08  9:23 ` sashiko-bot
@ 2026-08-08 11:20   ` Nguyễn Công
  0 siblings, 0 replies; 3+ messages in thread
From: Nguyễn Công @ 2026-08-08 11:20 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: media-ci, linux-sunxi

On Sat, Aug 8, 2026 at 4:23 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Missing cleanup of registered video and subdev devices in `sun4i_csi_notify_complete()` error paths leads to device leaks and potential Use-After-Free.
> - [High] The driver does not disable hardware interrupts or synchronize the IRQ handler when stopping capture, allowing an interrupt to crash the system after device suspension.
> - [High] Missing `.unbind` callback allows the video device to retain and access a freed subdevice pointer (`csi->src_subdev`).
> --
>
> --- Patch [1]: [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails ---
> commit d13febea573e64b9e585c2c1f9b472e41737683c
> Author: Cong Nguyen <congnt264@gmail.com>
>
> media: sun4i-csi: fix async connection leak when notifier registration fails
>
> sun4i_csi_notifier_init() adds a v4l2_async_connection to the notifier's
> waiting list with v4l2_async_nf_add_fwnode_remote(). If the subsequent
> v4l2_async_nf_register() then fails, probe jumps to err_unregister_media,
> which never calls v4l2_async_nf_cleanup(). The core does not free the
> waiting list on a failed registration, so the async connection is leaked.
>
> Add an err_clean_notifier label that cleans up the notifier on the
> registration-failure path, matching the teardown done in
> sun4i_csi_remove(). The notifier-init failure path adds nothing to the
> notifier and continues to skip the cleanup.
>
> > diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > index e53a07b770b7e..a8db851ab5a59 100644
> > --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c
> > @@ -255,13 +255,15 @@ static int sun4i_csi_probe(struct platform_device *pdev)
> >       ret = v4l2_async_nf_register(&csi->notifier);
> >       if (ret) {
> >               dev_err(csi->dev, "Couldn't register our notifier.\n");
> > -             goto err_unregister_media;
> > +             goto err_clean_notifier;
> >       }
> >
> >       pm_runtime_enable(&pdev->dev);
> >
> >       return 0;
> >
> > +err_clean_notifier:
> > +     v4l2_async_nf_cleanup(&csi->notifier);
> >  err_unregister_media:
> >       media_device_unregister(&csi->mdev);
> >       sun4i_csi_dma_unregister(csi);
>
> [Severity: High]
> This is a pre-existing issue, but does sun4i_csi_notify_complete() leak
> registered video and subdev devices in its error paths?
>
> If media_create_pad_link() or other subsequent operations fail, the error
> path err_clean_media only unregisters the media device:
>
> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c:sun4i_csi_notify_complete() {
>     ...
> err_clean_media:
>         media_device_unregister(&csi->mdev);
>
>         return ret;
> }
>
> It doesn't seem to unregister the video device or the subdev. Because this
> failure propagates back to v4l2_async_nf_register(), the probe will fail
> and the driver's devm-managed memory is freed.
>
> Since the /dev/videoX node remains active but points to freed memory,
> could userspace access trigger a use-after-free?
>
> [Severity: High]
> This isn't a bug introduced by this patch, but is the driver leaving
> hardware interrupts enabled when stopping capture?
>
> In sun4i_csi_stop_streaming(), the hardware capture is stopped, but the
> interrupt enable register doesn't appear to be cleared and there is no
> call to synchronize_irq():
>
> drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c:sun4i_csi_stop_streaming() {
>     ...
>         v4l2_subdev_call(csi->src_subdev, video, s_stream, 0);
>         sun4i_csi_capture_stop(csi);
>
>         /* Release all active buffers */
>     ...
> }
>
> Because capture stops at the end of the current frame, a frame-done
> interrupt could fire shortly after this function returns. If userspace
> closes the device, sun4i_csi_release() invokes pm_runtime_put(), powering
> down the hardware block.
>
> When the delayed interrupt fires, could the interrupt handler attempt to read
> registers on powered-down hardware and cause a system crash?
>
> drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c:sun4i_csi_irq() {
>     ...
>         reg = readl(csi->regs + CSI_INT_STA_REG);
>
>         /* Acknowledge the interrupts */
>         writel(reg, csi->regs + CSI_INT_STA_REG);
>     ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but is sun4i_csi_notify_ops missing an
> .unbind callback?
>
> drivers/media/platform/sunxi/sun4i-csi/sun4i_csi.c:
>     ...
> static const struct v4l2_async_notifier_operations sun4i_csi_notify_ops = {
>         .bound          = sun4i_csi_notify_bound,
>         .complete       = sun4i_csi_notify_complete,
> };
>     ...
>
> If the remote sensor module is unloaded, the V4L2 async core unbinds
> and frees the subdevice, but it appears csi->src_subdev is never cleared
> and the video node is never unregistered.
>
> If userspace calls VIDIOC_STREAMON afterward, does sun4i_csi_start_streaming()
> dereference the freed csi->src_subdev pointer here?
>
> 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;
>     ...
> }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260808091151.2691482-1-congnt264@gmail.com?part=1

Thanks for the review. I checked all three against the code — they are
all real pre-existing bugs, independent of this async-notifier cleanup
patch. I've prepared a separate 3-patch series to address them:

1. Missing cleanup in sun4i_csi_notify_complete() error paths

   Confirmed. sun4i_csi_v4l2_register() calls video_register_device(), so
   /dev/videoX is live before the pad links and subdev nodes are set up.
   On failure err_clean_media only unregisters the media device, leaving
   the video device (and the bridge subdev) registered. Since the failure
   aborts probe, the devm-managed sun4i_csi (which embeds the video_device)
   is freed while the node is still registered -> UAF on open(). Fixed by
   unwinding the registrations in reverse order, matching sun4i_csi_remove().

2. Interrupts left enabled in sun4i_csi_stop_streaming()

   Confirmed. start_streaming() enables CSI_INT_FRM_DONE in CSI_INT_EN_REG,
   but capture_stop() only clears CSI_CPT_CTRL_REG and stop_streaming() has
   no synchronize_irq(). A frame-done IRQ can fire after streaming stops;
   if userspace then closes the device, pm_runtime_put() gates the clocks
   and asserts reset, and a late handler touches registers on a dead block.
   Fixed by clearing CSI_INT_EN_REG and calling synchronize_irq() in
   stop_streaming() before the buffers/scratch are released.

3. Missing .unbind callback in sun4i_csi_notify_ops

   Confirmed. Without .unbind, csi->src_subdev is left dangling when the
   remote sensor is unbound/freed, and a later VIDIOC_STREAMON dereferences
   it in sun4i_csi_start_streaming() -> UAF. Fixed by adding an .unbind that
   unregisters the video device and clears csi->src_subdev.

I'll send these as a follow-up series ("media: sun4i-csi: fixes"). This
patch stands on its own; the series can be applied on top.

Thanks,
Cong

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-08 11:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-08  9:11 [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails Cong Nguyen
2026-08-08  9:23 ` sashiko-bot
2026-08-08 11:20   ` Nguyễn Công

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.