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

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

Posted by Cássio Gabriel 3 days, 5 hours ago

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");
 
 		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>

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

Posted by Richard Fitzgerald 2 days, 7 hours ago

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>
>

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

Posted by Cássio Gabriel Monteiro Pires 2 days, 7 hours ago

On 5/22/26 08:48, Richard Fitzgerald wrote:
>> @@ -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.

Ok, I'll drop that in v4.

-- 
Thanks,
Cássio