Re: [PATCH] ata: pata_pxa: use devm_platform_ioremap_resource

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

Pre-existing issues:
- [High] Teardown order bug in `pxa_ata_remove()` causes Use-After-Free
--

--- Patch [1]: [PATCH] ata: pata_pxa: use devm_platform_ioremap_resource ---
commit 5dcbe84075bf85bf6a553efa199d7ee5d2ff52be
Author: Rosen Penev <[email protected]>

ata: pata_pxa: use devm_platform_ioremap_resource

This commit replaces the open-coded platform_get_resource() and
devm_ioremap() sequences for the CMD and CTL ports with
devm_platform_ioremap_resource(). It simplifies the probe function by
consolidating resource fetching and mapping.

> diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c
> index 9f63bdfb8576e..c816997dc0a99 100644
> --- a/drivers/ata/pata_pxa.c
> +++ b/drivers/ata/pata_pxa.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does pxa_ata_remove() have a use-after-free
if an interrupt occurs during module unload?

Looking at drivers/ata/pata_pxa.c:pxa_ata_remove():

static void pxa_ata_remove(struct platform_device *pdev)
{
	struct ata_host *host = platform_get_drvdata(pdev);
	struct pata_pxa_data *data = host->ports[0]->private_data;

	dma_release_channel(data->dma_chan);

	ata_host_detach(host);
}

The DMA channel is explicitly freed via dma_release_channel() before calling
ata_host_detach(). 

Because the ATA host is still active and its interrupt handler was registered
via devres (which only unregisters the IRQ after the remove callback
completes), an active device or a stray interrupt could trigger
ata_sff_interrupt().

This interrupt handler calls pxa_bmdma_status(), which dereferences the
already freed data->dma_chan. 

Should ata_host_detach() be called before releasing the DMA channel to prevent
this race?

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