Re: [PATCHv2] ata: sata_dwc_460ex: fix PHY lifecycle ordering on device removal

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] dma_release_channel() and dmaengine_terminate_sync() access a destroyed DMA controller during device removal and probe failure.
--

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

ata: sata_dwc_460ex: fix PHY lifecycle ordering on device removal

This patch fixes the PHY lifecycle ordering in sata_dwc_remove() by ensuring
phy_power_off() completes before phy_exit() is called. It moves phy_exit()
into sata_dwc_port_stop() and reworks the error paths in probe accordingly.

> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 8e3fc713891af..8a1d80ac906a7 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
[ ... ]
> @@ -1171,29 +1182,26 @@ static int sata_dwc_probe(struct platform_device *ofdev)
[ ... ]
>  	err = ata_host_activate(host, irq, sata_dwc_isr, 0, &sata_dwc_sht);
> -	if (err)
> +	if (err) {
>  		dev_err(dev, "failed to activate host");
> +		goto error_out;
> +	}
>  
>  	/* Enable SATA Interrupts */
>  	sata_dwc_enable_interrupts(hsdev);
>  	return 0;
>  
>  error_out:
> -	phy_exit(hsdev->phy);
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> +	if (!device_property_present(dev, "dmas"))
> +		sata_dwc_dma_exit_old(hsdev);
> +#endif

[Severity: High]
This isn't a bug introduced by this patch, but this patch copies a broken
teardown sequence into the probe error path in drivers/ata/sata_dwc_460ex.c.

If ata_host_activate() fails, the DMA controller is explicitly destroyed here
by sata_dwc_dma_exit_old(). Devres will then later clean up the ATA host by
calling ata_host_stop(), which invokes sata_dwc_port_stop().

sata_dwc_port_stop() calls dmaengine_terminate_sync() and
dma_release_channel(). Will this result in a use-after-free and an invalid
hardware access to the already suspended and destroyed DMA controller?

>  	return err;
>  }
>  
> @@ -1205,8 +1213,6 @@ static void sata_dwc_remove(struct platform_device *ofdev)
>  
>  	ata_host_detach(host);
>  
> -	phy_exit(hsdev->phy);
> -
>  #ifdef CONFIG_SATA_DWC_OLD_DMA
>  	/* Free SATA DMA resources */
>  	sata_dwc_dma_exit_old(hsdev);

[Severity: High]
This is a pre-existing issue, but it is present in the device removal path
within sata_dwc_remove() in drivers/ata/sata_dwc_460ex.c.

During device removal, sata_dwc_remove() explicitly tears down the DMA
controller via sata_dwc_dma_exit_old(). When devres subsequently cleans up the
ATA host, it invokes sata_dwc_port_stop() which calls
dmaengine_terminate_sync() and dma_release_channel(). 

Could this cause a hardware fault and logical use-after-free by accessing the
unregistered DMA structures?

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