Re: [PATCH] dmaengine: qcom: hidma: fix autosuspend cleanup during teardown

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

Pre-existing issues:
- [High] The probe error path does not explicitly free the non-MSI interrupt before tearing down hardware data structures, leading to a potential Use-After-Free.
--

--- Patch [1]: [PATCH] dmaengine: qcom: hidma: fix autosuspend cleanup during teardown ---
commit 70b55f9899639dda5787b14a1ceb8890d7e7d005
Author: Guangshuo Li <[email protected]>

dmaengine: qcom: hidma: fix autosuspend cleanup during teardown

This patch fixes unbalanced usage counts during runtime PM teardown in the
Qualcomm HIDMA driver. It adds missing pm_runtime_dont_use_autosuspend() calls
to both the probe failure path and the device removal path.

> diff --git a/drivers/dma/qcom/hidma.c b/drivers/dma/qcom/hidma.c
> index 7a7f302a96991..d60d5f9a6182f 100644
> --- a/drivers/dma/qcom/hidma.c
> +++ b/drivers/dma/qcom/hidma.c
> @@ -894,6 +894,7 @@ static int hidma_probe(struct platform_device *pdev)
>  		hidma_free(dmadev);
>  bailout:
>  	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_dont_use_autosuspend(&pdev->dev);

[Severity: High]
This is a pre-existing issue, but does the probe error path have a
use-after-free risk for the non-MSI interrupt?

Looking at the uninit path just above this in hidma_probe():

drivers/dma/qcom/hidma.c:hidma_probe() {
...
uninit:
	if (msi)
		hidma_free_msis(dmadev);

	hidma_ll_uninit(dmadev->lldev);
...
}

If this is not an MSI interrupt, it remains active when hidma_ll_uninit()
is called because it was registered with devm_request_irq().

The hidma_ll_uninit() function calls tasklet_kill(&lldev->task) while
the hardware interrupt is still enabled.

If an interrupt fires during this window, the ISR hidma_ll_inthandler()
can call hidma_post_completed(), which reschedules the tasklet via
tasklet_schedule(&lldev->task). 

Since devres will eventually free the IRQ and the lldev memory, could
the re-scheduled tasklet remain queued and execute later, dereferencing
the freed lldev pointer? 

Should the non-MSI interrupt be explicitly freed using devm_free_irq()
before calling hidma_ll_uninit()?

>  	pm_runtime_disable(&pdev->dev);
>  	return rc;
>  }

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