[PATCH] ASoC: topology: Fix kcontrol name string handling.

[PATCH] ASoC: topology: Fix kcontrol name string handling.

[Liam Girdwood]

Fix the topology kcontrol string handling so that string pointer
references are strdup()ed instead of being copied. This fixes issues
with kcontrol templates on the stack or ones that are freed. Remember
and free the strings too when topology is unloaded.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
---
 sound/soc/soc-topology.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index 78197e82fc4f..dd2b118a35b3 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -530,6 +530,7 @@ static void remove_widget(struct snd_soc_component *comp,
 				kfree(se->dobj.control.dtexts[j]);
 
 			kfree(se);
+			kfree(w->kcontrol_news[i].name);
 		}
 	} else {
 		/* volume mixer or bytes controls */
@@ -546,6 +547,7 @@ static void remove_widget(struct snd_soc_component *comp,
 			 */
 			kfree((void *)kcontrol->private_value);
 			snd_ctl_remove(card, kcontrol);
+			kfree(w->kcontrol_news[i].name);
 		}
 	}
 
@@ -1256,7 +1258,9 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 		dev_dbg(tplg->dev, " adding DAPM widget mixer control %s at %d\n",
 			mc->hdr.name, i);
 
-		kc[i].name = mc->hdr.name;
+		kc[i].name = kstrdup(mc->hdr.name, GFP_KERNEL);
+		if (kc[i].name == NULL)
+			goto err_str;
 		kc[i].private_value = (long)sm;
 		kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 		kc[i].access = mc->hdr.access;
@@ -1304,8 +1308,10 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dmixer_create(
 err_str:
 	kfree(sm);
 err:
-	for (--i; i >= 0; i--)
+	for (--i; i >= 0; i--) {
 		kfree((void *)kc[i].private_value);
+		kfree(kc[i].name);
+	}
 	kfree(kc);
 	return NULL;
 }
@@ -1336,7 +1342,9 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_denum_create(
 		dev_dbg(tplg->dev, " adding DAPM widget enum control %s\n",
 			ec->hdr.name);
 
-		kc[i].name = ec->hdr.name;
+		kc[i].name = kstrdup(ec->hdr.name, GFP_KERNEL);
+		if (kc[i].name == NULL)
+			goto err_se;
 		kc[i].private_value = (long)se;
 		kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 		kc[i].access = ec->hdr.access;
@@ -1412,6 +1420,7 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_denum_create(
 			kfree(se->dobj.control.dtexts[j]);
 
 		kfree(se);
+		kfree(kc[i].name);
 	}
 err:
 	kfree(kc);
@@ -1450,7 +1459,9 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dbytes_create(
 			"ASoC: adding bytes kcontrol %s with access 0x%x\n",
 			be->hdr.name, be->hdr.access);
 
-		kc[i].name = be->hdr.name;
+		kc[i].name = kstrdup(be->hdr.name, GFP_KERNEL);
+		if (kc[i].name == NULL)
+			goto err;
 		kc[i].private_value = (long)sbe;
 		kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 		kc[i].access = be->hdr.access;
@@ -1480,8 +1491,10 @@ static struct snd_kcontrol_new *soc_tplg_dapm_widget_dbytes_create(
 	return kc;
 
 err:
-	for (--i; i >= 0; i--)
+	for (--i; i >= 0; i--) {
 		kfree((void *)kc[i].private_value);
+		kfree(kc[i].name);
+	}
 
 	kfree(kc);
 	return NULL;
-- 
2.14.1

Re: [PATCH] ASoC: topology: Fix kcontrol name string handling.

[Mark Brown]

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

On Wed, Mar 14, 2018 at 08:43:19PM +0000, Liam Girdwood wrote:
> Fix the topology kcontrol string handling so that string pointer
> references are strdup()ed instead of being copied. This fixes issues
> with kcontrol templates on the stack or ones that are freed. Remember
> and free the strings too when topology is unloaded.

This doesn't seem to apply against current code (I was going to apply it
as a fix but it doesn't seem to apply against -next either).

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH] ASoC: topology: Fix kcontrol name string handling.

[Liam Girdwood]

On Mon, 2018-03-19 at 15:06 +0800, Mark Brown wrote:
> On Wed, Mar 14, 2018 at 08:43:19PM +0000, Liam Girdwood wrote:
> > Fix the topology kcontrol string handling so that string pointer
> > references are strdup()ed instead of being copied. This fixes issues
> > with kcontrol templates on the stack or ones that are freed. Remember
> > and free the strings too when topology is unloaded.
> 
> This doesn't seem to apply against current code (I was going to apply it
> as a fix but it doesn't seem to apply against -next either).
> _______________________________________________
> 

Ok will do, Fwiw I did them against -next when you were at ELC, I guess there
have been some updates between now and then.

Thanks

Liam

Re: [PATCH] ASoC: topology: Fix kcontrol name string handling.

[Mark Brown]

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

On Mon, Mar 19, 2018 at 07:32:24PM +0000, Liam Girdwood wrote:
> On Mon, 2018-03-19 at 15:06 +0800, Mark Brown wrote:

> > This doesn't seem to apply against current code (I was going to apply it
> > as a fix but it doesn't seem to apply against -next either).

> Ok will do, Fwiw I did them against -next when you were at ELC, I guess there
> have been some updates between now and then.

It's also possible you have interdependencies between the patches - you
didn't send them as a series so any ordering requirements got lost.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]