Re: [PATCH] dmaengine: mediatek: mtk-uart-apdma: Return -ENOMEM on memory allocation failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Resource leak due to incorrect cleanup order in `mtk_uart_apdma_remove`. `mtk_uart_apdma_free` empties the channel list before `dma_async_device_unregister` is called.
- [High] Unbalanced runtime PM usage count due to incorrect reference dropping in `mtk_uart_apdma_alloc_chan_resources`.
--

commit c96187ec35bc4ba2bbf2baf929c0cd8ec646f5d8
Author: Vladimir Zapolskiy <vz@kernel.org>
dmaengine: mediatek: mtk-uart-apdma: Return -ENOMEM on memory allocation failure

If dynamic memory allocation in driver's probe function execution fails, it
should be reported to the driver's framework with -ENOMEM error code.

> diff --git a/drivers/dma/mediatek/mtk-uart-apdma.c b/drivers/dma/mediatek/mtk-uart-apdma.c
> index c269d84d7bd2b..f74e9a3285887 100644
> --- a/drivers/dma/mediatek/mtk-uart-apdma.c
> +++ b/drivers/dma/mediatek/mtk-uart-apdma.c
> @@ -531,7 +531,7 @@ static int mtk_uart_apdma_probe(struct platform_device *pdev)
>  	for (i = 0; i < mtkd->dma_requests; i++) {
>  		c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
>  		if (!c) {
> -			rc = -ENODEV;
> +			rc = -ENOMEM;
>  			goto err_no_dma;
>  		}

[Severity: High]
This is a pre-existing issue, but does mtk_uart_apdma_remove() have a cleanup
ordering bug?

mtk_uart_apdma_remove() calls mtk_uart_apdma_free() before calling
dma_async_device_unregister():

drivers/dma/mediatek/mtk-uart-apdma.c:mtk_uart_apdma_remove() {
	...
	mtk_uart_apdma_free(mtkd);

	dma_async_device_unregister(&mtkd->ddev);
	...
}

However, mtk_uart_apdma_free() deletes the channels from the device's list:

drivers/dma/mediatek/mtk-uart-apdma.c:mtk_uart_apdma_free() {
	...
		list_del(&c->vc.chan.device_node);
	...
}

Will this cause dma_async_device_unregister() to find an empty channel list
and skip unregistering the channels, leading to a resource leak?

[Severity: High]
This is also a pre-existing issue, but does mtk_uart_apdma_alloc_chan_resources()
have an unbalanced runtime PM usage count?

If pm_runtime_resume_and_get() fails, it drops the reference it never
acquired, but the error path calls pm_runtime_put_noidle() anyway:

drivers/dma/mediatek/mtk-uart-apdma.c:mtk_uart_apdma_alloc_chan_resources() {
	...
	ret = pm_runtime_resume_and_get(mtkd->ddev.dev);
	if (ret < 0) {
		pm_runtime_put_noidle(chan->device->dev);
		return ret;
	}
	...
}

Additionally, if all initialization succeeds, the function appears to fall
through to the err_pm label, dropping the acquired PM reference while
returning success:

	...
err_pm:
	pm_runtime_put_noidle(mtkd->ddev.dev);
	return ret;
}

Later, mtk_uart_apdma_free_chan_resources() unconditionally drops the
reference again via pm_runtime_put_sync().

Could this cause the PM usage count to drop below zero?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701200703.117929-1-vz@kernel.org?part=1