Re: [PATCH v2] ata: sata_dwc_460ex: Don't enable interrupt if libATA core registration fails

[sashiko-bot@kernel.org]

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

New issues:
- [High] Inverted teardown sequence for the PHY on probe error path, calling phy_power_off() after phy_exit().

Pre-existing issues:
- [Critical] Missing cleanup of DMA resources on probe error paths leads to Use-After-Free of DW DMA controller structures.
--

commit fb665db1a3c329dd76a0b668e1948fec56aec15e
Author: Nikolay Borisov <nik.borisov@suse.com>

ata: sata_dwc_460ex: Don't enable interrupt if libATA core registration fails

Recent 4bbc16a353a9 commit changed the timing when interrupts are enabled.
However it didn't account for the libATA registration step. Resolve the
issue by failing the entire registration process in case of failure to
register with libATA.

> diff --git a/drivers/ata/sata_dwc_460ex.c b/drivers/ata/sata_dwc_460ex.c
> index 8e3fc713891af..db39843c5f0fc 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -1185,8 +1185,10 @@ static int sata_dwc_probe(struct platform_device *ofdev)
>  	 * error_handler() to execute a dummy Softreset EH session
>  	 */
>  	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;

[Severity: High]
If ata_host_activate() fails, does this new failure path invert the PHY
teardown sequence?

When ata_host_activate() fails, it has already called ata_host_start(),
which powers on the PHY and registers a devres cleanup action for
ata_host_release(). Jumping to the error_out block executes a manual
phy_exit():

error_out:
	phy_exit(hsdev->phy);
	return err;

When devres later handles the cleanup, it cascades to sata_dwc_port_stop()
and executes phy_power_off(hsdev->phy) on the already-exited PHY. Could
this sequence violate the PHY API and potentially cause system lockups or
synchronous external aborts?

[Severity: Critical]
This is a pre-existing issue, but does this error path expose a missing
cleanup of legacy DMA resources?

When sata_dwc_probe() fails and jumps to error_out, the error handling
block skips calling sata_dwc_dma_exit_old(). Because
sata_dwc_dma_init_old() uses non-devm functions like request_irq() and
dma_async_device_register(), these globally registered resources remain
active and hold pointers to the freed devm-allocated structures.

If the shared IRQ fires or the DMA subsystem accesses the controller after
sata_dwc_probe() fails, could this lead to a use-after-free?

> +	}
>  
>  	/* Enable SATA Interrupts */
>  	sata_dwc_enable_interrupts(hsdev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909112426.811456-1-nik.borisov@suse.com?part=1