Re: [PATCH] ALSA: hda: cs35l41: Fix ACPI device reference leak on probe failure

[Takashi Iwai <tiwai@suse.de>]

On Sun, 11 Oct 2026 08:28:01 +0200,
Ma Ke wrote:
> 
> cs35l41_hda_probe() takes an acpi_device through
> cs35l41_hda_read_acpi() and stores it in cs35l41->dacpi. It is
> released in the err: label, which is reached from every later failure.
> 
> One failure is not routed there:
> 
>         ret = cs35l41_hda_read_acpi(cs35l41, device_name, id);
>         if (ret)
>                 return dev_err_probe(cs35l41->dev, ret, ...);
> 
> cs35l41->dacpi is assigned inside cs35l41_hda_read_acpi(), before the
> point where it can fail:
> 
>         adev = acpi_dev_get_first_match_dev(hid, NULL, -1);
>         ...
>         cs35l41->dacpi = adev;
>         ...
>         ret = cs35l41_hda_parse_acpi(cs35l41, physdev, id);
>         if (ret)
>                 return ret;
> 
> so on that path the reference taken by acpi_dev_get_first_match_dev()
> is never dropped, and cs35l41->acpi_subsystem_id is leaked as well.
> This is reachable on any system whose ACPI description does not match
> what the driver expects.
> 
> Route the failure through a new err_acpi label that releases both.
> 
> cs35l41_hda_read_acpi() also drops the reference on its own !physdev
> path while leaving cs35l41->dacpi pointing at the released device.
> Clear the pointer there so the reference is not dropped a second time
> from either cleanup path.
> 
> Found by code review.
> 
> Fixes: 447106e92a0c ("ALSA: hda: cs35l41: Support mute notifications for CS35L41 HDA")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ma Ke <make_ruc2021@163.com>

While the error path you're correcting looks right, but could you
check the issue reported by Sashiko?
  https://sashiko.dev/#/patchset/20261011062801.3163066-1-make_ruc2021%40163.com
IIUC, the suggestion there looks correct, and we have already the
unreference call in the error path of cs35l41_hda_parse_acpi(),
indeed.

The refcount management there is really messy, admittedly...


thanks,

Takashi

> ---
>  sound/hda/codecs/side-codecs/cs35l41_hda.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/cs35l41_hda.c b/sound/hda/codecs/side-codecs/cs35l41_hda.c
> index 62296be45c55..1088065f1fff 100644
> --- a/sound/hda/codecs/side-codecs/cs35l41_hda.c
> +++ b/sound/hda/codecs/side-codecs/cs35l41_hda.c
> @@ -1929,6 +1929,7 @@ static int cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, const char *hid, i
>  	struct device *physdev __free(put_device) =
>  		get_device(acpi_get_first_physical_node(adev));
>  	if (!physdev) {
> +		cs35l41->dacpi = NULL;
>  		acpi_dev_put(adev);
>  		return -ENODEV;
>  	}
> @@ -1986,8 +1987,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i
>  	dev_set_drvdata(dev, cs35l41);
>  
>  	ret = cs35l41_hda_read_acpi(cs35l41, device_name, id);
> -	if (ret)
> -		return dev_err_probe(cs35l41->dev, ret, "Platform not supported\n");
> +	if (ret) {
> +		dev_err_probe(cs35l41->dev, ret, "Platform not supported\n");
> +		goto err_acpi;
> +	}
>  
>  	if (IS_ERR(cs35l41->reset_gpio)) {
>  		ret = PTR_ERR(cs35l41->reset_gpio);
> @@ -2066,6 +2069,12 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i
>  
>  	return 0;
>  
> +err_acpi:
> +	acpi_dev_put(cs35l41->dacpi);
> +	kfree(cs35l41->acpi_subsystem_id);
> +
> +	return ret;
> +
>  err_pm:
>  	pm_runtime_dont_use_autosuspend(cs35l41->dev);
>  	pm_runtime_disable(cs35l41->dev);
> -- 
> 2.43.0
>