Re: [PATCH 3/3] ata: pata_ep93xx: add COMPILE_TEST support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Double free of 'rx' DMA channel and invalid pointer dereference of `ERR_PTR` in the 'tx' DMA channel error path.
--

commit b636a1b70934f2b41b7dce1e54946eb6d1bed3b4
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: add COMPILE_TEST support

This commit enables COMPILE_TEST support for the Cirrus Logic EP93xx PATA
controller. This allows the buildbots to compile the driver on other
architectures to find potential issues.

> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index fff305ec1e788..28ca856ecc754 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -692,7 +692,7 @@ config PATA_EFAR
>  
>  config PATA_EP93XX
>  	tristate "Cirrus Logic EP93xx PATA support"
> -	depends on ARCH_EP93XX
> +	depends on ARCH_EP93XX || COMPILE_TEST

[Severity: High]
This isn't a bug introduced by this patch, but enabling COMPILE_TEST might
expose a pre-existing error handling issue in drivers/ata/pata_ep93xx.c.

In ep93xx_pata_dma_init(), if dma_request_chan() for "tx" fails, it stores
the ERR_PTR in drv_data->dma_tx_channel and jumps to fail_release_rx:

drivers/ata/pata_ep93xx.c:ep93xx_pata_dma_init() {
    ...
	drv_data->dma_tx_channel = dma_request_chan(&pdev->dev, "tx");
	if (IS_ERR(drv_data->dma_tx_channel)) {
		ret = dev_err_probe(dev, PTR_ERR(drv_data->dma_tx_channel),
				    "tx DMA setup failed\n");
		goto fail_release_rx;
	}
    ...
fail_release_rx:
	dma_release_channel(drv_data->dma_rx_channel);
fail_release_dma:
	ep93xx_pata_release_dma(drv_data);

	return ret;
}

Could this sequence lead to a double-free and an invalid pointer dereference?

The fail_release_rx path releases dma_rx_channel but doesn't clear the
pointer to NULL before falling through to ep93xx_pata_release_dma(). Also,
dma_tx_channel holds an ERR_PTR which is non-NULL.

When ep93xx_pata_release_dma() is called, it checks if the pointers are
non-NULL:

drivers/ata/pata_ep93xx.c:ep93xx_pata_release_dma() {
    ...
	if (drv_data->dma_rx_channel) {
		dma_release_channel(drv_data->dma_rx_channel);
		drv_data->dma_rx_channel = NULL;
	}
	if (drv_data->dma_tx_channel) {
		dma_release_channel(drv_data->dma_tx_channel);
		drv_data->dma_tx_channel = NULL;
	}
    ...
}

Would this result in dma_release_channel() being called a second time on
the rx channel, and being called with the ERR_PTR for the tx channel?

>  	select PATA_TIMINGS
>  	help
>  	  This option enables support for the PATA controller in

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530003627.42473-1-rosenp@gmail.com?part=3