Re: [alsa-devel] [PATCH] ASoC: topology: Fix not to keep a reference to tplg fw

On Thu, 26 Nov 2015 15:11:00 +0100, Subhransu S. Prusty wrote:

During element creation, the name of some of the elements point to memory referenced in tplg fw. If the tplg fw is released after tplg is parsed by framework, kernel panic happens during creation of elements while card initialization.

In which code path? When the kctl is already instantiated from snd_kcontrol_new template, we don't have to duplicate the string. The only case where the strdup() is required is to delay the instantiation, i.e. storing the kcontrol_new object itself instead of referring temporarily.

Issue is caught with id#87b5ed8ecb9fe05a696e1c0b53c7a49ea66432c1

You should put the commit subject, too.

So create a copy of the memory and assign to names instead.

And who releases these duplicated memory? It looks like another memory leak to me.

Takashi

Signed-off-by: Subhransu S. Prusty subhransu.s.prusty@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com

---

sound/soc/soc-topology.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c index 6963ba2..61eb1de 100644 --- a/sound/soc/soc-topology.c +++ b/sound/soc/soc-topology.c @@ -709,7 +709,7 @@ static int soc_tplg_dbytes_create(struct soc_tplg *tplg, unsigned int count, be->hdr.name, be->hdr.access);

memset(&kc, 0, sizeof(kc));

• kc.name = be->hdr.name;
  

• kc.name = kstrdup(be->hdr.name, GFP_KERNEL);
  

  kc.private_value = (long)sbe; kc.iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc.access = be->hdr.access;

@@ -789,7 +789,7 @@ static int soc_tplg_dmixer_create(struct soc_tplg *tplg, unsigned int count, mc->hdr.name, mc->hdr.access);

memset(&kc, 0, sizeof(kc));

• kc.name = mc->hdr.name;
  

• kc.name = kstrdup(mc->hdr.name, GFP_KERNEL);
  

  kc.private_value = (long)sm; kc.iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc.access = mc->hdr.access;

@@ -935,7 +935,7 @@ static int soc_tplg_denum_create(struct soc_tplg *tplg, unsigned int count, ec->hdr.name, ec->items);

memset(&kc, 0, sizeof(kc));

• kc.name = ec->hdr.name;
  

• kc.name = kstrdup(ec->hdr.name, GFP_KERNEL);
  

  kc.private_value = (long)se; kc.iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc.access = ec->hdr.access;

@@ -1105,8 +1105,8 @@ static int soc_tplg_dapm_graph_elems_load(struct soc_tplg *tplg, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) return -EINVAL;

• route.source = elem->source;
  

• route.sink = elem->sink;
  

• route.source = kstrdup(elem->source, GFP_KERNEL);
  

• route.sink = kstrdup(elem->sink, GFP_KERNEL);
  

  route.connected = NULL; /* set to NULL atm for tplg users */ if (strnlen(elem->control, SNDRV_CTL_ELEM_ID_NAME_MAXLEN) == 0) route.control = NULL;

@@ -1149,7 +1149,7 @@ 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);
  

  kc[i].private_value = (long)sm; kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc[i].access = mc->hdr.access;

@@ -1228,7 +1228,7 @@ 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->name = ec->hdr.name;

• kc->name = kstrdup(ec->hdr.name, GFP_KERNEL); kc->private_value = (long)se; kc->iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc->access = ec->hdr.access;

@@ -1330,7 +1330,7 @@ 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);
  

  kc[i].private_value = (long)sbe; kc[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER; kc[i].access = be->hdr.access;

-- 1.9.1

---

Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel