Re: [PATCH 17/26] dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The IRQ is requested before dependent resources are initialized, and the probe error path destroys resources before the devm-managed IRQ is disabled, risking NULL pointer dereferences, uninitialized spinlock access, and Use-After-Free.
- [High] Mismatch between devm-managed DMA registration and manual resource teardown causes sysfs leaks and use-after-free risks in both probe error and remove paths.
--

--- Patch [17]: [PATCH 17/26] dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe() ---
commit 15a299566b3b9735b597a7ab918fd3926002aefe
Author: Pan Chuang <panchuang@vivo.com>
dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()

This commit removes driver-specific error logging for devm_request_irq()
failures in st_fdma_probe(), as the core now automatically logs detailed
error messages on failure.

> diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
> index d9547017f3bd6..05a2642601e61 100644
> --- a/drivers/dma/st_fdma.c
> +++ b/drivers/dma/st_fdma.c
> @@ -771,10 +771,8 @@ static int st_fdma_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, fdev->irq, st_fdma_irq_handler, 0,
>  			       dev_name(&pdev->dev), fdev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to request irq (%d)\n", ret);
> +	if (ret)
>  		goto err;

[Severity: High]
This is a pre-existing issue, but is it safe to request the IRQ before
allocating fdev->slim_rproc and initializing the channels?

If an interrupt fires immediately after devm_request_irq() is called,
st_fdma_irq_handler() will use fdma_read(), which dereferences
fdev->slim_rproc->peri. Since fdev->slim_rproc is still NULL, would this
cause a crash?

Also, if probe fails later (e.g., during DMA registration), the error
path destroys slim_rproc and kills tasklets:

drivers/dma/st_fdma.c:st_fdma_probe() {
    ...
err_rproc:
	st_fdma_free(fdev);
	st_slim_rproc_put(fdev->slim_rproc);
err:
	return ret;
}

Since the IRQ is managed by devres, it remains active until the devres
cleanup runs. If an interrupt fires during this window, could it access
destroyed resources?

[Severity: High]
This is also a pre-existing issue, but does the error handling in
st_fdma_probe() and the driver remove path create a mismatch with
devm-managed DMA registration?

When probe fails in of_dma_controller_register(), the err_rproc path calls
st_fdma_free() before devres cleanup:

drivers/dma/st_fdma.c:st_fdma_free() {
    ...
	list_del(&fchan->vchan.chan.device_node);
    ...
}

This manual list_del() empties fdev->dma_device.channels. Will this cause
the devres dma_async_device_unregister() to skip unregistering the sysfs
devices, leaving dangling pointers to the freed fdev?

Similarly, in the remove path:

drivers/dma/st_fdma.c:st_fdma_remove() {
	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);

	devm_free_irq(&pdev->dev, fdev->irq, fdev);
	st_slim_rproc_put(fdev->slim_rproc);
	of_dma_controller_free(pdev->dev.of_node);
}

st_fdma_remove() does not call st_fdma_free() to kill the tasklets, and it
puts slim_rproc manually. When devres later frees fdev, could any
scheduled but unkilled tasklet trigger a use-after-free, and could concurrent
clients terminating channels trigger a use-after-free on slim_rproc?

>  
>  	fdev->slim_rproc = st_slim_rproc_alloc(pdev, fdev->fw_name);
>  	if (IS_ERR(fdev->slim_rproc)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=17