* Re: [PATCH] ata: pata_pxa: Fix DMA channel leak on probe error
2026-06-25 14:18 [PATCH] ata: pata_pxa: Fix DMA channel leak on probe error Wentao Liang
@ 2026-06-25 14:27 ` sashiko-bot
2026-06-26 11:10 ` Niklas Cassel
2026-06-26 21:44 ` Damien Le Moal
2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-25 14:27 UTC (permalink / raw)
To: Wentao Liang; +Cc: cassel, linux-ide, dlemoal
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect teardown order in `pxa_ata_remove()` releases the DMA channel before detaching the ATA host, creating a use-after-free window.
- [High] Missing NULL check for platform data (`pdata`) before dereferencing it in the probe function.
- [High] Returning `AC_ERR_OK` when DMA descriptor preparation fails leads to an unrecoverable 30-second ATA command timeout.
--
commit fb8345f4cc3cb507abc9c29fac4d410d641eab22
Author: Wentao Liang <vulab@iscas.ac.cn>
Subject: ata: pata_pxa: Fix DMA channel leak on probe error
When dmaengine_slave_config() fails, the DMA channel acquired by
dma_request_chan() is not released before returning the error,
leaking the channel reference. This commit adds dma_release_channel()
in the error path.
> diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c
> index 03dbaf4a13a75..9f63bdfb8576e 100644
> --- a/drivers/ata/pata_pxa.c
> +++ b/drivers/ata/pata_pxa.c
> @@ -286,6 +286,7 @@ static int pxa_ata_probe(struct platform_device *pdev)
> ret = dmaengine_slave_config(data->dma_chan, &config);
> if (ret < 0) {
> dev_err(&pdev->dev, "dma configuration failed: %d\n", ret);
> + dma_release_channel(data->dma_chan);
> return ret;
> }
While reviewing this patch, I noticed a few pre-existing issues in the same
file.
In pxa_ata_probe():
> struct pata_pxa_pdata *pdata = dev_get_platdata(&pdev->dev);
[ ... ]
> /*
> * Adjust register offsets
> */
> ap->ioaddr.altstatus_addr = ap->ioaddr.ctl_addr;
> ap->ioaddr.data_addr = ap->ioaddr.cmd_addr +
> (ATA_REG_DATA << pdata->reg_shift);
[Severity: High]
This isn't a bug introduced by this patch, but is it possible for pdata to be
NULL here?
If the driver is bound manually via sysfs to a platform device lacking
platform data, dev_get_platdata() will return NULL, which would cause a
direct kernel panic when dereferencing pdata->reg_shift.
In pxa_qc_prep():
> tx = dmaengine_prep_slave_sg(pd->dma_chan, qc->sg, qc->n_elem, dir,
> DMA_PREP_INTERRUPT);
> if (!tx) {
> ata_dev_err(qc->dev, "prep_slave_sg() failed\n");
> return AC_ERR_OK;
> }
[Severity: High]
This isn't a bug introduced by this patch, but should this return an error
code like AC_ERR_SYSTEM instead of AC_ERR_OK?
Returning AC_ERR_OK when dmaengine_prep_slave_sg() fails incorrectly masks
the error from the ATA core. The core proceeds to issue the command thinking
it was prepared, but since no DMA descriptors were actually submitted, the
device will never complete the transfer.
This inevitably results in a 30-second block layer timeout.
In 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);
> }
[Severity: High]
This isn't a bug introduced by this patch, but is the teardown order safe
here?
ata_host_detach() unregisters the SCSI host, which can invoke the ATA error
handler (EH) to flush or abort pending commands. If an ongoing command is
aborted, the EH invokes driver callbacks like pxa_bmdma_stop(), which
attempt to call dmaengine_tx_status() and dmaengine_terminate_all() on the
already freed data->dma_chan.
Does calling dma_release_channel() before ata_host_detach() create a
use-after-free window?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625141837.62362-1-vulab@iscas.ac.cn?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] ata: pata_pxa: Fix DMA channel leak on probe error
2026-06-25 14:18 [PATCH] ata: pata_pxa: Fix DMA channel leak on probe error Wentao Liang
2026-06-25 14:27 ` sashiko-bot
@ 2026-06-26 11:10 ` Niklas Cassel
2026-06-26 21:44 ` Damien Le Moal
2 siblings, 0 replies; 4+ messages in thread
From: Niklas Cassel @ 2026-06-26 11:10 UTC (permalink / raw)
To: Wentao Liang; +Cc: dlemoal, linux-ide, linux-kernel, stable
On Thu, Jun 25, 2026 at 10:18:37PM +0800, Wentao Liang wrote:
> When dmaengine_slave_config() fails, the DMA channel acquired by
> dma_request_chan() is not released before returning the error,
> leaking the channel reference.
>
> Fix by adding dma_release_channel() in the error path.
>
> The ata_host_activate() error path already correctly releases the
> DMA channel.
>
> Cc: stable@vger.kernel.org
> Fixes: 88622d80af82 ("ata: pata_pxa: dmaengine conversion")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/ata/pata_pxa.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/ata/pata_pxa.c b/drivers/ata/pata_pxa.c
> index 03dbaf4a13a7..9f63bdfb8576 100644
> --- a/drivers/ata/pata_pxa.c
> +++ b/drivers/ata/pata_pxa.c
> @@ -286,6 +286,7 @@ static int pxa_ata_probe(struct platform_device *pdev)
> ret = dmaengine_slave_config(data->dma_chan, &config);
> if (ret < 0) {
> dev_err(&pdev->dev, "dma configuration failed: %d\n", ret);
> + dma_release_channel(data->dma_chan);
> return ret;
> }
>
> --
> 2.39.5 (Apple Git-154)
>
Reviewed-by: Niklas Cassel <cassel@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread