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

Reply via email to