Re: [PATCH v3] ALSA: hda: cs35l56: Fix system name string leaks

[Richard Fitzgerald <rf@opensource.cirrus.com>]

On 21/05/2026 2:34 pm, Cássio Gabriel wrote:
> cs35l56_hda_read_acpi() gets an allocated ACPI _SUB string from
> acpi_get_subsystem_id(). On success, that string is used to create the
> firmware system name.
> 
> Several error paths after the _SUB lookup can return without releasing
> the allocated string. This includes speaker ID lookup errors other than
> -ENOENT, and errors after a firmware system name has been allocated.
> 
> Use scoped cleanup for the temporary _SUB string and make
> cs35l56->system_name device-managed. This releases the temporary _SUB
> string on every error path and lets devres release the firmware system
> name on probe failure and device removal.
> 
> Fixes: 6f03b446cbae ("ALSA: hda: cs35l56: Add support for speaker id")
> Fixes: 40b1c2f9b299 ("ALSA: hda/cs35l56: Workaround bad dev-index on Lenovo Yoga Book 9i GenX")
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
> ---
> Changes in v3:
> - Make cs35l56->system_name devm-managed.
> - Keep __free(kfree) only for the temporary ACPI _SUB string.
> - Remove the local cleanup-managed system_name variable.
> - Drop the probe-error cleanup patch; devres now owns system_name.
> - Remove the manual remove-time kfree for system_name.
> - Link to v2: https://patch.msgid.link/20260520-alsa-cs35l56-system-name-leak-v2-0-60f26c159023@gmail.com
> 
> Changes in v2:
> - Use __free(kfree) for the temporary ACPI _SUB string.
> - Keep the derived system name local until read_acpi() succeeds.
> - Transfer ownership with no_free_ptr() only on successful paths.
> - Link to v1: https://patch.msgid.link/20260520-alsa-cs35l56-system-name-leak-v1-0-d13b54423198@gmail.com
> ---
>   sound/hda/codecs/side-codecs/cs35l56_hda.c | 20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> index cdbc576569ef..8213d62ad74b 100644
> --- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
> +++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> @@ -1025,7 +1025,7 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
>   	u32 values[HDA_MAX_COMPONENTS];
>   	char hid_string[8];
>   	struct acpi_device *adev;
> -	const char *property, *sub;
> +	const char *property;
>   	int i, ret;
>   
>   	/*
> @@ -1047,7 +1047,8 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
>   	/* Initialize things that could be overwritten by a fixup */
>   	cs35l56->index = -1;
>   
> -	sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
> +	const char *sub __free(kfree) = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
> +
>   	ret = cs35l56_hda_apply_platform_fixups(cs35l56, sub, &id);
>   	if (ret)
>   		return ret;
> @@ -1095,15 +1096,16 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
>   		ret = cirrus_scodec_get_speaker_id(cs35l56->base.dev, cs35l56->index,
>   						   cs35l56->num_amps, -1);
>   		if (ret == -ENOENT) {
> -			cs35l56->system_name = sub;
> +			cs35l56->system_name = devm_kstrdup(cs35l56->base.dev, sub, GFP_KERNEL);
>   		} else if (ret >= 0) {
> -			cs35l56->system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
> -			kfree(sub);
> -			if (!cs35l56->system_name)
> -				return -ENOMEM;
> +			cs35l56->system_name = devm_kasprintf(cs35l56->base.dev, GFP_KERNEL,
> +							      "%s-spkid%d", sub, ret);
>   		} else {
>   			return ret;
>   		}
> +
> +		if (!cs35l56->system_name)
> +			return -ENOMEM;
>   	}
>   
>   	cs35l56->base.reset_gpio = devm_gpiod_get_index_optional(cs35l56->base.dev,
> @@ -1118,7 +1120,8 @@ static int cs35l56_hda_read_acpi(struct cs35l56_hda *cs35l56, int hid, int id)
>   		 * line and reset all the amps
>   		 */
>   		if (ret != -EBUSY)
> -			return dev_err_probe(cs35l56->base.dev, ret, "Failed to get reset GPIO\n");
> +			return dev_err_probe(cs35l56->base.dev, ret,
> +					     "Failed to get reset GPIO\n");

Spurious change. Not related to system_name.

> 
>   		dev_info(cs35l56->base.dev, "Reset GPIO busy, assume shared reset\n");
>   		cs35l56->base.reset_gpio = NULL;
> @@ -1254,7 +1257,6 @@ void cs35l56_hda_remove(struct device *dev)
>   
>   	cs_dsp_remove(&cs35l56->cs_dsp);
>   
> -	kfree(cs35l56->system_name);
>   	pm_runtime_put_noidle(cs35l56->base.dev);
>   
>   	gpiod_set_value_cansleep(cs35l56->base.reset_gpio, 0);
> 
> ---
> base-commit: 38c607c673155d6335591cdbd9c785fb2b7550e5
> change-id: 20260501-alsa-cs35l56-system-name-leak-7fd779eac391
> 
> Best regards,
> --
> Cássio Gabriel <cassiogabrielcontato@gmail.com>
>