Re: [PATCH] ata: ahci_brcm: fix refcount leak in brcm_ahci_probe()

[Niklas Cassel <cassel@kernel.org>]

On Wed, Jun 03, 2026 at 10:30:08AM +0000, Wentao Liang wrote:
> When reset_control_deassert() fails in brcm_ahci_probe(), the
> function returns without calling reset_control_rearm() on the
> previously asserted shared reset control. This leaves the
> triggered count incremented, leaking the reset control reference.
> 
> All other error paths after the reset_control_reset() call properly
> reach the out_reset label which performs the rearm. Rework the
> deassert error path to go through out_reset to restore the
> triggered count and ensure the reference is released properly.
> 
> Cc: stable@vger.kernel.org
> Fixes: 1a0600d112e3 ("ata: ahci_brcm: Perform reset after obtaining resources")
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/ata/ahci_brcm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
> index 38c63d73d210..b58343f027cf 100644
> --- a/drivers/ata/ahci_brcm.c
> +++ b/drivers/ata/ahci_brcm.c
> @@ -492,7 +492,7 @@ static int brcm_ahci_probe(struct platform_device *pdev)
>  		return ret;
>  	ret = reset_control_deassert(priv->rcdev_ahci);
>  	if (ret)
> -		return ret;
> +		goto out_reset;
>  
>  	ret = ahci_platform_enable_clks(hpriv);
>  	if (ret)
> -- 
> 2.34.1
> 

The code in brcm_ahci_probe():

        ret = reset_control_reset(priv->rcdev_rescal);
        if (ret)
                return ret;
        ret = reset_control_deassert(priv->rcdev_ahci);
        if (ret)
                return ret;

        ret = ahci_platform_enable_clks(hpriv);
        if (ret)


The code in brcm_ahci_resume():

        ret = reset_control_deassert(priv->rcdev_ahci);
        if (ret)
                return ret;
        ret = reset_control_reset(priv->rcdev_rescal);
        if (ret)
                return ret;

        ret = ahci_platform_enable_clks(hpriv);
        if (ret)


This makes no sense.

The order of the resets should be the same in both functions, since it is
different reset handles.


It is probably easier to add a new jump label, so that there is one jump
label per reset handle.

That way you can jump to the correct jump label, so that you only perform
the opposite of the reset that was actually successful.


Kind regards,
Nilklas