Skip to contentSkip to conversation

ASoC: some trivial fixups

PATCH Applied Daniel Mack 2 participants 7 messages Oct 7, 2014

Subsystems (2)
Sound ASoC
lore
Share & tools
lore

Daniel Mack

author
#0 lore raw

cover ASoC: some trivial fixups

Here are some trivial fixes for minor issues that were spotted by Coverity. Only compile-tested, but the issues are pretty obvious, and the fixes are simple.

Applicable to the for-next branch of the asoc tree.

Thanks,
Daniel

Daniel Mack (3):
ASoC: core: fix use after free in snd_soc_remove_platform()
ASoC: soc-dapm: fix use after free
ASoC: 88pm860x-codec: Fix possibly missing string termination

 sound/soc/codecs/88pm860x-codec.c |  2 +-
 sound/soc/soc-core.c              |  4 ++--
 sound/soc/soc-dapm.c              | 25 ++++++++++++++-----------
 3 files changed, 17 insertions(+), 14 deletions(-)
Signature · 1 line
2.1.0
author
#1 lore raw

PATCH 1/3 ASoC: core: fix use after free in snd_soc_remove_platform()

Coverity spotted an use-after-free condition in snd_soc_remove_platform(). Fix this by moving snd_soc_component_cleanup() after the debug print statement which uses the component's string.

Signed-off-by Daniel Mack <[email protected]>
---
 sound/soc/soc-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
sound/soc/soc-core.c +2 -2
index 3d8cff6..4c8f8a2 100644
@@ -4192,10 +4192,10 @@ void snd_soc_remove_platform(struct snd_soc_platform *platform)
 	snd_soc_component_del_unlocked(&platform->component);
 	mutex_unlock(&client_mutex);
 
-	snd_soc_component_cleanup(&platform->component);
-
 	dev_dbg(platform->dev, "ASoC: Unregistered platform '%s'\n",
 		platform->component.name);
+
+	snd_soc_component_cleanup(&platform->component);
 }
 EXPORT_SYMBOL_GPL(snd_soc_remove_platform);
Signature · 1 line
2.1.0
maintainer Applied
#2 lore raw

Re: [PATCH 1/3] ASoC: core: fix use after free in snd_soc_remove_platform()

Quoted 3 lines · Daniel Mack

Coverity spotted an use-after-free condition in snd_soc_remove_platform(). Fix this by moving snd_soc_component_cleanup() after the debug print statement which uses the component's string.

Applied, thanks.

author
#3 lore raw

PATCH 2/3 ASoC: soc-dapm: fix use after free

In reply to Daniel Mack

Coverity spotted the following possible use-after-free condition in dapm_create_or_share_mixmux_kcontrol():

If kcontrol is NULL, and (wname_in_long_name && kcname_in_long_name) validates to true, 'name' will be set to an allocated string, and be freed a few lines later via the 'long_name' alias. 'name', however, is used by dev_err() in case snd_ctl_add() fails.

Fix this by adding a jump label that frees 'long_name' at the end of the function.

Signed-off-by Daniel Mack <[email protected]>
---
 sound/soc/soc-dapm.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)
sound/soc/soc-dapm.c +14 -11
index 2c456a3..c61cb9c 100644
@@ -592,9 +592,9 @@ static int dapm_create_or_share_mixmux_kcontrol(struct snd_soc_dapm_widget *w,
 	int shared;
 	struct snd_kcontrol *kcontrol;
 	bool wname_in_long_name, kcname_in_long_name;
-	char *long_name;
+	char *long_name = NULL;
 	const char *name;
-	int ret;
+	int ret = 0;
 
 	prefix = soc_dapm_prefix(dapm);
 	if (prefix)

@@ -653,15 +653,17 @@ static int dapm_create_or_share_mixmux_kcontrol(struct snd_soc_dapm_widget *w,
 
 		kcontrol = snd_soc_cnew(&w->kcontrol_news[kci], NULL, name,
 					prefix);
-		kfree(long_name);
-		if (!kcontrol)
-			return -ENOMEM;
+		if (!kcontrol) {
+			ret = -ENOMEM;
+			goto exit_free;
+		}
+
 		kcontrol->private_free = dapm_kcontrol_free;
 
 		ret = dapm_kcontrol_data_alloc(w, kcontrol);
 		if (ret) {
 			snd_ctl_free_one(kcontrol);
-			return ret;
+			goto exit_free;
 		}
 
 		ret = snd_ctl_add(card, kcontrol);

@@ -669,17 +671,18 @@ static int dapm_create_or_share_mixmux_kcontrol(struct snd_soc_dapm_widget *w,
 			dev_err(dapm->dev,
 				"ASoC: failed to add widget %s dapm kcontrol %s: %d\n",
 				w->name, name, ret);
-			return ret;
+			goto exit_free;
 		}
 	}
 
 	ret = dapm_kcontrol_add_widget(kcontrol, w);
-	if (ret)
-		return ret;
+	if (ret == 0)
+		w->kcontrols[kci] = kcontrol;
 
-	w->kcontrols[kci] = kcontrol;
+exit_free:
+	kfree(long_name);
 
-	return 0;
+	return ret;
 }
 
 /* create new dapm mixer control */
Signature · 1 line
2.1.0
maintainer Applied
#4 lore raw

Re: [PATCH 2/3] ASoC: soc-dapm: fix use after free

Quoted 2 lines · Daniel Mack

Coverity spotted the following possible use-after-free condition in dapm_create_or_share_mixmux_kcontrol():

Applied, thanks.

author
#5 lore raw

PATCH 3/3 ASoC: 88pm860x-codec: Fix possibly missing string termination

In reply to Daniel Mack

Coverity spotted an issue with strncpy() in pm860x_codec_probe() which does not take the \0 termination byte into account. Fix this by making the buffers one byte larger so the can really accommodate MAX_NAME_LEN bytes long strings.

Signed-off-by Daniel Mack <[email protected]>
---
 sound/soc/codecs/88pm860x-codec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/88pm860x-codec.c +1 -1
index 4c3b0af..e88a6b6 100644
@@ -146,7 +146,7 @@ struct pm860x_priv {
 	struct pm860x_det	det;
 
 	int			irq[4];
-	unsigned char		name[4][MAX_NAME_LEN];
+	unsigned char		name[4][MAX_NAME_LEN+1];
 };
 
 /* -9450dB to 0dB in 150dB steps ( mute instead of -9450dB) */
Signature · 1 line
2.1.0
maintainer Applied
#6 lore raw

Re: [PATCH 3/3] ASoC: 88pm860x-codec: Fix possibly missing string termination

Quoted 4 lines · Daniel Mack

Coverity spotted an issue with strncpy() in pm860x_codec_probe() which does not take the \0 termination byte into account. Fix this by making the buffers one byte larger so the can really accommodate MAX_NAME_LEN bytes long strings.

Applied, thanks.