Re: [PATCH v4 2/5] ata: sata_fsl: Fix use-after-free of host_priv on probe() failure

[Niklas Cassel <cassel@kernel.org>]

On Tue, Sep 15, 2026 at 11:06:55AM +0700, Damien Le Moal wrote:
> On 2026/09/11 0:14, Niklas Cassel wrote:
> > sata_fsl_host_stop() releases hcr_base and host_priv:
> > 
> > 	static void sata_fsl_host_stop(struct ata_host *host)
> > 	{
> > 		struct sata_fsl_host_priv *host_priv = host->private_data;
> > 
> > 		iounmap(host_priv->hcr_base);
> > 		kfree(host_priv);
> > 	}
> > 
> > ->host_stop() is called by the driver core through the ata_host_stop()
> > devres action which ata_host_start() registers, so it is called both
> > when the device is unbound and when probe() fails after
> > ata_host_activate() has started the host.
> > 
> > The error_exit_with_cleanup label in sata_fsl_probe() releases hcr_base
> > and host_priv as well, and it is reachable from the two
> > device_create_file() calls done after ata_host_activate(). In that case
> > sata_fsl_host_stop() runs on an already freed host_priv, resulting in a
> > use-after-free and a double iounmap()/kfree().
> > 
> > sata_fsl_probe() also ignores the return value of ata_host_activate(),
> > so probe() returns success even if activating the host failed.
> > 
> > Check the return value of ata_host_activate() and, once the host has
> > been activated, leave hcr_base and host_priv to sata_fsl_host_stop().
> > 
> > Note that if ata_host_activate() fails inside ata_host_start(), e.g. if
> > sata_fsl_port_start() fails, ->host_stop() is not registered and
> > hcr_base and host_priv are leaked. Leaking them is preferable to freeing
> > them twice, and this is addressed by a later patch in this series.
> 
> I would prefer seeing the leak fixed here too so that we have a self contained
> fix in a single commit.
> 
> Or, move the leak fix patch before this one.

I took a third option:

This (temporary) leak was only added by this patch, and was later fixed
by "ata: libata: Do not leave ata_host_stop() registered when activation
fails".

The UAF happened only when the activation succeeded and device_create_file()
failed. Thus, this fix does not rely on ata_host_activate() error handling.

Thus, let this patch only fix the UAF, without adding any error handling to
ata_host_activate(). That way this patch is self contained.

Patch "ata: libata: Do not leave ata_host_stop() registered when activation
fails" will add ata_host_activate() error handling to this driver, and at
this point, ata_host_activate() internals has been fixed. Thus, we there
will never exist any intermediate step where we might leak resources.


Kind regards,
Niklas