Re: [PATCH] accel/ivpu: fix usage_count leak when autosuspend_delay is negative

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

Pre-existing issues:
- [High] Background work items scheduled by IRQ handlers are not flushed or canceled on the probe error path, leading to a Use-After-Free when devres frees the device.
--

--- Patch [1]: [PATCH] accel/ivpu: fix usage_count leak when autosuspend_delay is negative ---
commit db53366ef5dbe7e33ab10b0d9f2d73d71bbd452a
Author: Guangshuo Li <[email protected]>

accel/ivpu: fix usage_count leak when autosuspend_delay is negative

ivpu_pm_init() calls pm_runtime_use_autosuspend(), but the autosuspend
setting is not disabled when the device is removed or when initialization
fails. This patch adds pm_runtime_dont_use_autosuspend() to ivpu_pm_disable()
and the ivpu_dev_init() error path to prevent a usage_count leak.

> diff --git a/drivers/accel/ivpu/ivpu_drv.c b/drivers/accel/ivpu/ivpu_drv.c
> index 95120957f42a5..f8460aca189cd 100644
> --- a/drivers/accel/ivpu/ivpu_drv.c
> +++ b/drivers/accel/ivpu/ivpu_drv.c
[ ... ]
> @@ -776,13 +776,15 @@ static int ivpu_dev_init(struct ivpu_device *vdev)
>  
>  	ret = ivpu_boot(vdev);
>  	if (ret)
> -		goto err_ipc_fini;
> +		goto err_pm_fini;
>  
>  	ivpu_job_done_consumer_init(vdev);
>  	ivpu_pm_enable(vdev);
>  
>  	return 0;
>  
> +err_pm_fini:
> +	pm_runtime_dont_use_autosuspend(vdev->drm.dev);

[Severity: High]
This is a pre-existing issue, but since we are modifying the error paths
here, I noticed that background work items scheduled by IRQ handlers might
not be flushed or canceled on the probe error path.

If a hardware interrupt occurs during ivpu_boot() after enable_irq() is called,
the handler may queue irq_dct_work or context_abort_work. If a subsequent
initialization step like ivpu_pm_dct_init() fails, the error path disables
interrupts but skips flushing the queued works:

ivpu_boot() {
        ...
        enable_irq(vdev->irq);
        ...
        ret = ivpu_pm_dct_init(vdev);
        if (ret)
                goto err_disable_ipc;
        ...
err_disable_ipc:
        ivpu_ipc_disable(vdev);
        ivpu_hw_irq_disable(vdev);
        disable_irq(vdev->irq);
        ...
}

The error is then returned to ivpu_dev_init(), which jumps to err_pm_fini
where this new cleanup code is placed.

Could this lead to a memory corruption scenario? Without cancel_work_sync()
or flush_work() to halt pending tasks, devres might free the ivpu_device
structure when probe fails. Any pending work items might later execute and
dereference the freed pointer.

>  err_ipc_fini:
>  	ivpu_ipc_fini(vdev);
>  err_fw_fini:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1