[PATCH v2 0/2] ALSA: hda: cs35l56: fix system name leaks

[PATCH v2 0/2] ALSA: hda: cs35l56: fix system name leaks

[Cássio Gabriel]

cs35l56_hda_read_acpi() reads an allocated ACPI _SUB string to build
the firmware system name. Current error paths can return after _SUB has
been allocated, or after it has been transferred to cs35l56->system_name,
without releasing that memory.

Fix the local read_acpi() unwind paths first, then release the stored
system name from the common probe error path.

Signed-off-by: Cássio Gabriel <cassiogabrielcontato@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

---
Cássio Gabriel (2):
      ALSA: hda: cs35l56: Free ACPI _SUB string on errors
      ALSA: hda: cs35l56: Free system name on probe errors

 sound/hda/codecs/side-codecs/cs35l56_hda.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)
---
base-commit: 38c607c673155d6335591cdbd9c785fb2b7550e5
change-id: 20260501-alsa-cs35l56-system-name-leak-7fd779eac391

Best regards,
--  
Cássio Gabriel <cassiogabrielcontato@gmail.com>

[PATCH v2 1/2] ALSA: hda: cs35l56: Free ACPI _SUB string on errors

[Cássio Gabriel]

cs35l56_hda_read_acpi() gets an allocated ACPI _SUB string from
acpi_get_subsystem_id(). On success, that string is either used directly
as the firmware system name or used to construct a speaker-ID-specific
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 derived but
before ownership is transferred to cs35l56->system_name.

Use scoped cleanup for the temporary _SUB string and the derived system
name, and transfer ownership to cs35l56->system_name only after the
remaining failure paths in cs35l56_hda_read_acpi() have completed.

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>
---
 sound/hda/codecs/side-codecs/cs35l56_hda.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
index cdbc576569ef..7c9c98436757 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,9 @@ 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));
+	const char *system_name __free(kfree) = NULL;
+
 	ret = cs35l56_hda_apply_platform_fixups(cs35l56, sub, &id);
 	if (ret)
 		return ret;
@@ -1095,11 +1097,10 @@ 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;
+			system_name = no_free_ptr(sub);
 		} else if (ret >= 0) {
-			cs35l56->system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
-			kfree(sub);
-			if (!cs35l56->system_name)
+			system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
+			if (!system_name)
 				return -ENOMEM;
 		} else {
 			return ret;
@@ -1118,12 +1119,14 @@ 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;
 	}
 
+	cs35l56->system_name = no_free_ptr(system_name);
 	return 0;
 
 err:

-- 
2.54.0

Re: [PATCH v2 1/2] ALSA: hda: cs35l56: Free ACPI _SUB string on errors

[Richard Fitzgerald]

On 20/05/2026 3:50 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 either used directly
> as the firmware system name or used to construct a speaker-ID-specific
> 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 derived but
> before ownership is transferred to cs35l56->system_name.
> 
> Use scoped cleanup for the temporary _SUB string and the derived system
> name, and transfer ownership to cs35l56->system_name only after the
> remaining failure paths in cs35l56_hda_read_acpi() have completed.
> 
> 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>
> ---
>   sound/hda/codecs/side-codecs/cs35l56_hda.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
> index cdbc576569ef..7c9c98436757 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,9 @@ 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));
> +	const char *system_name __free(kfree) = NULL;
> +

Initializing a __free(kfree) to NULL is controversial. The kerneldoc
warns against it. Though I think your code is totally fine it might be
best to avoid.

I can't see any reason why we didn't use devm cleanup for
cs35l56->system_name.

So

    		if (ret == -ENOENT) {
			cs35l56->system_name = devm_kstrdup(dev, sub,
GFP_KERNEL);
   		} else if (ret >= 0) {
			cs35l56->system_name = devm_kasprintf(dev, ...);
			...


and delete the manual kfree(cs35l56->system_name) in
cs35l56_hda_remove().

>   	ret = cs35l56_hda_apply_platform_fixups(cs35l56, sub, &id);
>   	if (ret)
>   		return ret;
> @@ -1095,11 +1097,10 @@ 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;
> +			system_name = no_free_ptr(sub);
>   		} else if (ret >= 0) {
> -			cs35l56->system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
> -			kfree(sub);
> -			if (!cs35l56->system_name)
> +			system_name = kasprintf(GFP_KERNEL, "%s-spkid%d", sub, ret);
> +			if (!system_name)
>   				return -ENOMEM;
>   		} else {
>   			return ret;
> @@ -1118,12 +1119,14 @@ 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;
>   	}
>   
> +	cs35l56->system_name = no_free_ptr(system_name);
>   	return 0;
>   
>   err:
> 

Re: [PATCH v2 1/2] ALSA: hda: cs35l56: Free ACPI _SUB string on errors

[Cássio Gabriel Monteiro Pires]

[-- Attachment #1.1: Type: text/plain, Size: 1210 bytes --]

On 5/21/26 09:14, Richard Fitzgerald wrote:
>>   -    sub = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
>> +    const char *sub __free(kfree) = acpi_get_subsystem_id(ACPI_HANDLE(cs35l56->base.dev));
>> +    const char *system_name __free(kfree) = NULL;
>> +
> 
> Initializing a __free(kfree) to NULL is controversial. The kerneldoc
> warns against it. Though I think your code is totally fine it might be
> best to avoid.

I used __free(kfree) for the derived system_name to keep
ownership transfer local until read_acpi() had passed the reset GPIO error
path, but I agree that initializing a cleanup variable to NULL is best
avoided.

> 
> I can't see any reason why we didn't use devm cleanup for
> cs35l56->system_name.
> 
> So
> 
>            if (ret == -ENOENT) {
>             cs35l56->system_name = devm_kstrdup(dev, sub,
> GFP_KERNEL);
>           } else if (ret >= 0) {
>             cs35l56->system_name = devm_kasprintf(dev, ...);
>             ...
> 
> 
> and delete the manual kfree(cs35l56->system_name) in
> cs35l56_hda_remove(). 

I'll send a v3 with that approach.


Thanks,
Cássio

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

[PATCH v2 2/2] ALSA: hda: cs35l56: Free system name on probe errors

[Cássio Gabriel]

cs35l56_hda_read_acpi() stores an allocated firmware system name in
cs35l56->system_name. The normal remove path frees it, but remove is
not called if cs35l56_hda_common_probe() fails after read_acpi() has
succeeded.

Free cs35l56->system_name from the common probe error path and clear
the pointer.

Fixes: 73cfbfa9caea ("ALSA: hda/cs35l56: Add driver for Cirrus Logic CS35L56 amplifier")
Signed-off-by: Cássio Gabriel <cassiogabrielcontato@gmail.com>
---
 sound/hda/codecs/side-codecs/cs35l56_hda.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/hda/codecs/side-codecs/cs35l56_hda.c b/sound/hda/codecs/side-codecs/cs35l56_hda.c
index 7c9c98436757..8130ec271165 100644
--- a/sound/hda/codecs/side-codecs/cs35l56_hda.c
+++ b/sound/hda/codecs/side-codecs/cs35l56_hda.c
@@ -1240,6 +1240,8 @@ int cs35l56_hda_common_probe(struct cs35l56_hda *cs35l56, int hid, int id)
 	cs_dsp_remove(&cs35l56->cs_dsp);
 err:
 	gpiod_set_value_cansleep(cs35l56->base.reset_gpio, 0);
+	kfree(cs35l56->system_name);
+	cs35l56->system_name = NULL;
 
 	return ret;
 }

-- 
2.54.0