Re: [PATCH v5 04/16] drm/panthor: Drop a needless check in panthor_fw_unplug()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The reset work `panthor_device_reset_work` can execute before device initialization completes, leading to a NULL pointer dereference.
--

commit b7f6d51a0860da42553b0a2ac207a7c13c51cd47
Author: Boris Brezillon <boris.brezillon@collabora.com>

drm/panthor: Drop a needless check in panthor_fw_unplug()

panthor_fw_unplug() is only called if we at least managed to initialize
the IRQ, so it's safe to drop the "is IRQ initialized" check.

> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c
> index 986151681b246..4fbddb9e18c8b 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c
> @@ -1279,9 +1279,7 @@ void panthor_fw_unplug(struct panthor_device *ptdev)
>  
>  	if (!IS_ENABLED(CONFIG_PM) || pm_runtime_active(ptdev->base.dev)) {
>  		/* Make sure the IRQ handler cannot be called after that point. */
> -		if (ptdev->fw->irq.irq)
> -			panthor_job_irq_suspend(&ptdev->fw->irq);
> -
> +		panthor_job_irq_suspend(&ptdev->fw->irq);
>  		panthor_fw_stop(ptdev);
>  	}
>  

[Severity: High]
This is a pre-existing issue, but can the reset work panthor_device_reset_work
execute before device initialization completes?

During device initialization in panthor_device_init(), panthor_fw_init() is
called, which invokes panthor_vm_active(). This in turn calls
panthor_mmu_as_enable() and as_send_cmd_and_wait().

If the MMU AS_ACTIVE bit gets stuck and times out,
panthor_device_schedule_reset() is called. Because the device PM state has
already been transitioned to ACTIVE earlier via pm_runtime_resume_and_get(),
the reset work is queued and executes asynchronously.

The reset worker unconditionally calls panthor_sched_pre_reset(), which
immediately dereferences ptdev->scheduler:

drivers/gpu/drm/panthor/panthor_sched.c:panthor_sched_pre_reset() {
        struct panthor_scheduler *sched = ptdev->scheduler;
        u32 i;

        mutex_lock(&sched->reset.lock);
        ...
}

Since ptdev->scheduler is only allocated later in the probe sequence by
panthor_sched_init(), could this result in a NULL pointer dereference if the
reset happens during the probe window?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-panthor-signal-from-irq-v5-0-8836a74e0ef9@collabora.com?part=4