Re: [PATCHv3] ata: pata_ep93xx: add missing "cc" and "memory" clobbers to inline asm

[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 and ERR_PTR dereference in DMA initialization error path.
--

--- Patch [1]: [PATCHv3] ata: pata_ep93xx: add missing "cc" and "memory" clobbers to inline asm ---
commit 05e8a0e6eff7bdb5719b3840e3f01ca22df401f7
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: add missing "cc" and "memory" clobbers to inline asm

This commit adds "cc" and "memory" clobbers to the inline assembly in
ep93xx_pata_delay(). It prevents compiler optimizations from incorrectly
reordering flag-dependent instructions or I/O accesses.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index 42a24dc51d26b..813e72425e6f7 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does the error path in
ep93xx_pata_dma_init() result in a double-free and an invalid pointer
dereference if tx DMA setup fails?

When dma_request_chan(&pdev->dev, "tx") fails, it returns an ERR_PTR
which is assigned to drv_data->dma_tx_channel. The code then 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;
}

In fail_release_rx, dma_release_channel() is called but
drv_data->dma_rx_channel is not set to NULL.

The code then falls through to ep93xx_pata_release_dma():

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;
	}
}

Will this cause dma_release_channel() to be called a second time for
dma_rx_channel since the pointer is still truthy?

Additionally, since dma_tx_channel contains an ERR_PTR rather than NULL,
won't the truthy check pass and cause an invalid pointer dereference in
dma_release_channel()?

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