Re: [PATCH v2] media: nuvoton: npcm-video: quiesce VCD IRQ before teardown

[Hans Verkuil <hverkuil+cisco@kernel.org>]

Can Nuvoton test this patch? I feel happier if this is given a quick test.

Regards,

	Hans

On 16/07/2026 12:15, Fan Wu wrote:
> The VCD IRQ is devm-requested, but npcm_video_remove() frees the video
> object before devres releases that IRQ.  The threaded handler dereferences
> video->vcd_regmap before checking VIDEO_STREAMING, so an interrupt in that
> interval can access freed memory.
> 
> Request the IRQ with IRQF_NO_AUTOEN.  Enable it after starting capture and
> setting VIDEO_STREAMING, and disable it first in stop_streaming().
> disable_irq() waits for an in-flight threaded handler to finish, after
> which stop_streaming() can mask and reset the VCD without a handler
> re-enabling it.
> 
> Use vb2_video_unregister_device() during remove.  It releases the vb2
> queue and calls stop_streaming() for an active stream, ensuring that the
> IRQ is disabled before the video object is freed.  Do not release the queue
> separately.
> 
> If streaming is never started, IRQF_NO_AUTOEN keeps the IRQ disabled
> until devres releases it.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 46c15a4ff1f4 ("media: nuvoton: Add driver for NPCM video capture and encoding engine")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.6
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
> Changes since v1:
> - Follow Hans Verkuil's suggestion to tie IRQ enablement to the streaming
>   lifecycle (IRQF_NO_AUTOEN + enable_irq/disable_irq) and to use
>   vb2_video_unregister_device() for teardown.
> 
> Compile-tested only; I do not have NPCM hardware, so runtime testing by
> the Nuvoton maintainers would be appreciated.
> ---
>  drivers/media/platform/nuvoton/npcm-video.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/nuvoton/npcm-video.c b/drivers/media/platform/nuvoton/npcm-video.c
> index 52505af35c08..c28d9d7edd83 100644
> --- a/drivers/media/platform/nuvoton/npcm-video.c
> +++ b/drivers/media/platform/nuvoton/npcm-video.c
> @@ -120,6 +120,7 @@ struct npcm_video {
>  
>  	struct list_head buffers;
>  	struct mutex buffer_lock; /* buffer list lock */
> +	int irq;
>  	unsigned long flags;
>  	unsigned int sequence;
>  
> @@ -1486,6 +1487,7 @@ static int npcm_video_start_streaming(struct vb2_queue *q, unsigned int count)
>  	}
>  
>  	set_bit(VIDEO_STREAMING, &video->flags);
> +	enable_irq(video->irq);
>  	return 0;
>  }
>  
> @@ -1494,6 +1496,7 @@ static void npcm_video_stop_streaming(struct vb2_queue *q)
>  	struct npcm_video *video = vb2_get_drv_priv(q);
>  	struct regmap *vcd = video->vcd_regmap;
>  
> +	disable_irq(video->irq);
>  	clear_bit(VIDEO_STREAMING, &video->flags);
>  	regmap_write(vcd, VCD_INTE, 0);
>  	regmap_write(vcd, VCD_STAT, VCD_STAT_CLEAR);
> @@ -1707,9 +1710,10 @@ static int npcm_video_init(struct npcm_video *video)
>  		dev_err(dev, "Failed to find VCD IRQ\n");
>  		return -ENODEV;
>  	}
> +	video->irq = irq;
>  
>  	rc = devm_request_threaded_irq(dev, irq, NULL, npcm_video_irq,
> -				       IRQF_ONESHOT, DEVICE_NAME, video);
> +				       IRQF_ONESHOT | IRQF_NO_AUTOEN, DEVICE_NAME, video);
>  	if (rc < 0) {
>  		dev_err(dev, "Failed to request IRQ %d\n", irq);
>  		return rc;
> @@ -1807,8 +1811,7 @@ static void npcm_video_remove(struct platform_device *pdev)
>  	struct v4l2_device *v4l2_dev = dev_get_drvdata(dev);
>  	struct npcm_video *video = to_npcm_video(v4l2_dev);
>  
> -	video_unregister_device(&video->vdev);
> -	vb2_queue_release(&video->queue);
> +	vb2_video_unregister_device(&video->vdev);
>  	v4l2_ctrl_handler_free(&video->ctrl_handler);
>  	v4l2_device_unregister(v4l2_dev);
>  	if (video->ece.enable)