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

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

On 22/05/2026 1:49 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 v4:
> - Drop unrelated line-wrap change in reset GPIO error path.
> - Link to v3: https://patch.msgid.link/20260521-alsa-cs35l56-system-name-leak-v3-1-9dd4e2dcb473@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 | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> index cdbc576569ef..a0ea08eb96a9 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,
> @@ -1254,7 +1256,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: ef807cc07dec16edc7863d437e9250e20cb73741
> change-id: 20260501-alsa-cs35l56-system-name-leak-7fd779eac391
> 
> Best regards,
> --
> Cássio Gabriel <cassiogabrielcontato@gmail.com>
> 

Reviewed-by: Richard Fitzgerald <rf@opensource.cirrus.com>