Re: [PATCH v3] media: qcom: iris: guard IRQ handler with runtime PM check

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Early return from threaded IRQ handler skips enable_irq(), permanently masking the interrupt and completely breaking CONFIG_PM=n builds.

Pre-existing issues:
- [High] Tearing down shared queues and destroying locks in iris_remove() without synchronizing the threaded IRQ leads to Use-After-Free.
--

commit 21ac80fbe9eb1a25413cc6f4a32a9f240d73c9b7
Author: Hungyu Lin <dennylin0707@gmail.com>

media: qcom: iris: guard IRQ handler with runtime PM check

This commit adds a runtime PM check to the threaded IRQ handler using
pm_runtime_get_if_active(). It skips interrupt processing if the device
is not active and autosuspends the device after processing.

> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> index 621c66593d88d..59040cce8cf16 100644
> --- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
> +++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
> @@ -100,10 +100,15 @@ irqreturn_t iris_hfi_isr(int irq, void *data)
>  irqreturn_t iris_hfi_isr_handler(int irq, void *data)
>  {
>  	struct iris_core *core = data;
> +	int ret;
>  
>  	if (!core)
>  		return IRQ_NONE;
>  
> +	ret = pm_runtime_get_if_active(core->dev);
> +	if (ret <= 0)
> +		return IRQ_NONE;
> +

[Severity: Critical]
Does this early return permanently mask the interrupt?

The top-half handler iris_hfi_isr() calls disable_irq_nosync() before
waking this threaded handler. By returning early here in
iris_hfi_isr_handler(), does the code skip the enable_irq() call at the end
of the handler and leave the interrupt permanently disabled?

Additionally, on builds without CONFIG_PM, pm_runtime_get_if_active()
always returns -EINVAL. Will this cause the handler to unconditionally
return early on the first interrupt?

>  	mutex_lock(&core->lock);
>  	pm_runtime_mark_last_busy(core->dev);
>  	iris_vpu_clear_interrupt(core);
> @@ -111,6 +116,8 @@ irqreturn_t iris_hfi_isr_handler(int irq, void *data)
>  
>  	core->hfi_response_ops->hfi_response_handler(core);
>  
> +	pm_runtime_put_autosuspend(core->dev);
> +
>  	if (!iris_vpu_watchdog(core, core->intr_status))
>  		enable_irq(irq);
>  

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for this
threaded handler to execute after the queues and locks are destroyed during
device removal?

During device removal, iris_remove() calls iris_core_deinit() which disables
the IRQ via disable_irq_nosync() and frees the queues using
iris_hfi_queues_deinit(). Since disable_irq_nosync() does not wait for an
executing threaded handler to complete, could iris_hfi_isr_handler() run
concurrently and attempt to access the freed queues or the destroyed lock?

Would calling synchronize_irq() before tearing down these software structures
prevent this use-after-free?

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