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

[sashiko-bot@kernel.org]

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