Re: [PATCH] media: sun4i-csi: fix async connection leak when notifier registration fails

On Sat, Aug 8, 2026 at 4:23 PM <[email protected]> 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 <[email protected]>
>
> 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/[email protected]?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