sound/alsa/soc/codec: fix memory leak and resource relaim in error path

PATCH In review Axel Lin 2 participants 14 messages Jul 23, 2010

Subsystems (5)

Sound ASoC Wolfson microelectronics drivers Dialog semiconductor drivers +1 more subsystems

lore

Share & tools

lore

Axel Lin

author

2010-07-23 05:54 UTC #0 lore raw

v2 cover sound/alsa/soc/codec: fix memory leak and resource relaim in error path

This serial of patches fixes memory leak and resource relaim in error path

I made a mistake that changed the subject line for some of the update patches. So. here is a re-send and all patches are marked as v2.

Regards,
Axel

changes for v2:
wm8940: follow Guennadi's comment to move the buffer allocate and release
at the same level to prevent the memory leak.
wm9081: follow Guennadi's comment to move the buffer allocate and release
at the same level to prevent the memory leak.

Axel Lin (12):
ad1836: fix a memory leak if another ad1836 is registered
ak4642: fix a memory leak if failed to initialise AK4642
da7210: fix a memory leak if failed to initialise da7210 audio codec
wm8523: fix resource reclaim in wm8523_register error path
wm8711: fix a memory leak if another WM8711 is registered
wm8904: fix resource reclaim in wm8904_register error path
wm8940: fix a memory leak if wm8940_register return error
wm8955: fix resource reclaim in wm8955_register error path
wm8961: fix resource reclaim in wm8961_register error path
wm8974: fix a memory leak if another WM8974 is registered
wm8978: fix a memory leak if a wm8978_register fail
wm9081: fix resource reclaim in wm9081_register error path

 sound/soc/codecs/ad1836.c |    1 +
 sound/soc/codecs/ak4642.c |    4 +++-
 sound/soc/codecs/da7210.c |    8 ++++++--
 sound/soc/codecs/wm8523.c |   10 ++++++----
 sound/soc/codecs/wm8711.c |    3 ++-
 sound/soc/codecs/wm8904.c |   13 ++++++++-----
 sound/soc/codecs/wm8940.c |    7 ++++++-
 sound/soc/codecs/wm8955.c |   10 ++++++----
 sound/soc/codecs/wm8961.c |    9 +++++----
 sound/soc/codecs/wm8974.c |    3 ++-
 sound/soc/codecs/wm8978.c |   10 +++++++---
 sound/soc/codecs/wm9081.c |   11 ++++++-----
 12 files changed, 58 insertions(+), 31 deletions(-)

Axel Lin

author

2010-07-23 05:54 UTC #1 lore raw

PATCH v2 1/12 ad1836: fix a memory leak if another ad1836 is registered

ad1836 is allocated in ad1836_spi_probe() but is not freed if ad1836_register() return -EINVAL (if another ad1836 is registered).

Signed-off-by Axel Lin <[email protected]>

Acked-by Barry Song <[email protected]>

---
 sound/soc/codecs/ad1836.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

sound/soc/codecs/ad1836.c +1 -0

index 2175384..a01006c 100644
@@ -272,6 +272,7 @@ static int ad1836_register(struct ad1836_priv *ad1836)
 
 	if (ad1836_codec) {
 		dev_err(codec->dev, "Another ad1836 is registered\n");
+		kfree(ad1836);
 		return -EINVAL;
 	}

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #2 lore raw

PATCH v2 2/12 ak4642: fix a memory leak if failed to initialise AK4642

In reply to Axel Lin

ak4642 should be kfreed if ak4642_init() return error.

Signed-off-by Axel Lin <[email protected]>

Reviewed-by Kuninori Morimoto <[email protected]>

---
 sound/soc/codecs/ak4642.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

sound/soc/codecs/ak4642.c +3 -1

index 7528a54..4feefa8 100644
@@ -491,8 +491,10 @@ static int ak4642_i2c_probe(struct i2c_client *i2c,
 	codec->control_data = i2c;
 
 	ret = ak4642_init(ak4642);
-	if (ret < 0)
+	if (ret < 0) {
 		printk(KERN_ERR "failed to initialise AK4642\n");
+		kfree(ak4642);
+	}
 
 	return ret;
 }

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:56 UTC #3 lore raw

PATCH v2 3/12 da7210: fix a memory leak if failed to initialise da7210 audio codec

In reply to Axel Lin

da7210 should be kfreed if da7210_init() return error.
This patch also fixes the error handing in the case of snd_soc_register_dai() fail by adding snd_soc_unregister_codec() in error path.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/da7210.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

sound/soc/codecs/da7210.c +6 -2

index 75af2d6..3b9a6cc 100644
@@ -488,7 +488,7 @@ static int da7210_init(struct da7210_priv *da7210)
 	ret = snd_soc_register_dai(&da7210_dai);
 	if (ret) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		goto init_err;
+		goto codec_err;
 	}
 
 	/* FIXME

@@ -574,6 +574,8 @@ static int da7210_init(struct da7210_priv *da7210)
 
 	return ret;
 
+codec_err:
+	snd_soc_unregister_codec(codec);
 init_err:
 	kfree(codec->reg_cache);
 	codec->reg_cache = NULL;

@@ -601,8 +603,10 @@ static int __devinit da7210_i2c_probe(struct i2c_client *i2c,
 	codec->control_data = i2c;
 
 	ret = da7210_init(da7210);
-	if (ret < 0)
+	if (ret < 0) {
 		pr_err("Failed to initialise da7210 audio codec\n");
+		kfree(da7210);
+	}
 
 	return ret;
 }

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #4 lore raw

PATCH v2 4/12 wm8523: fix resource reclaim in wm8523_register error path

In reply to Axel Lin

This patch includes below fixes:
1. If another WM8523 is registered, need to kfree wm8523 before return -EINVAL.
2. If snd_soc_register_codec failed, goto error path to properly free resources.
3. Instead of using mixed in-line and goto style cleanup, use goto style error handling if snd_soc_register_dai failed.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8523.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

sound/soc/codecs/wm8523.c +6 -4

index 37242a7..0ad039b 100644
@@ -482,7 +482,8 @@ static int wm8523_register(struct wm8523_priv *wm8523,
 
 	if (wm8523_codec) {
 		dev_err(codec->dev, "Another WM8523 is registered\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	mutex_init(&codec->mutex);

@@ -570,18 +571,19 @@ static int wm8523_register(struct wm8523_priv *wm8523,
 	ret = snd_soc_register_codec(codec);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		return ret;
+		goto err_enable;
 	}
 
 	ret = snd_soc_register_dai(&wm8523_dai);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		snd_soc_unregister_codec(codec);
-		return ret;
+		goto err_codec;
 	}
 
 	return 0;
 
+err_codec:
+	snd_soc_unregister_codec(codec);
 err_enable:
 	regulator_bulk_disable(ARRAY_SIZE(wm8523->supplies), wm8523->supplies);
 err_get:

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #5 lore raw

PATCH v2 5/12 wm8711: fix a memory leak if another WM8711 is registered

In reply to Axel Lin

wm8711 is allocated in either wm8711_spi_probe() or wm8711_i2c_probe() but is not freed if wm8711_register() return -EINVAL(if another ad1836 is registered).

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8711.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

sound/soc/codecs/wm8711.c +2 -1

index effb14e..e2dba07 100644
@@ -439,7 +439,8 @@ static int wm8711_register(struct wm8711_priv *wm8711,
 
 	if (wm8711_codec) {
 		dev_err(codec->dev, "Another WM8711 is registered\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	mutex_init(&codec->mutex);

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:56 UTC #6 lore raw

PATCH v2 6/12 wm8904: fix resource reclaim in wm8904_register error path

In reply to Axel Lin

This patch includes below fixes:
1. wm8904 need to be kfreed in wm8904_register() error path before return.
2. fix the error path for snd_soc_register_codec() fail and snd_soc_register_dai() fail to properly free resources.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8904.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

sound/soc/codecs/wm8904.c +8 -5

index 87f14f8..f7dcabf 100644
@@ -2433,7 +2433,8 @@ static int wm8904_register(struct wm8904_priv *wm8904,
 
 	if (wm8904_codec) {
 		dev_err(codec->dev, "Another WM8904 is registered\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	mutex_init(&codec->mutex);

@@ -2462,7 +2463,8 @@ static int wm8904_register(struct wm8904_priv *wm8904,
 	default:
 		dev_err(codec->dev, "Unknown device type %d\n",
 			wm8904->devtype);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	memcpy(codec->reg_cache, wm8904_reg, sizeof(wm8904_reg));

@@ -2566,18 +2568,19 @@ static int wm8904_register(struct wm8904_priv *wm8904,
 	ret = snd_soc_register_codec(codec);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		return ret;
+		goto err_enable;
 	}
 
 	ret = snd_soc_register_dai(&wm8904_dai);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		snd_soc_unregister_codec(codec);
-		return ret;
+		goto err_codec;
 	}
 
 	return 0;
 
+err_codec:
+	snd_soc_unregister_codec(codec);
 err_enable:
 	regulator_bulk_disable(ARRAY_SIZE(wm8904->supplies), wm8904->supplies);
 err_get:

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:55 UTC #7 lore raw

PATCH v2 7/12 wm8940: fix a memory leak if wm8940_register return error

In reply to Axel Lin

This patch adds checking for wm8940_register return value, and does kfree(wm8940) if wm8940_register() fail.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8940.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

sound/soc/codecs/wm8940.c +6 -1

index e3c4bbf..f0c1113 100644
@@ -845,6 +845,7 @@ static void wm8940_unregister(struct wm8940_priv *wm8940)
 static int wm8940_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
+	int ret;
 	struct wm8940_priv *wm8940;
 	struct snd_soc_codec *codec;

@@ -858,7 +859,11 @@ static int wm8940_i2c_probe(struct i2c_client *i2c,
 	codec->control_data = i2c;
 	codec->dev = &i2c->dev;
 
-	return wm8940_register(wm8940, SND_SOC_I2C);
+	ret = wm8940_register(wm8940, SND_SOC_I2C);
+	if (ret < 0)
+		kfree(wm8940);
+
+	return ret;
 }
 
 static int __devexit wm8940_i2c_remove(struct i2c_client *client)

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #8 lore raw

PATCH v2 8/12 wm8955: fix resource reclaim in wm8955_register error path

In reply to Axel Lin

This patch fixes the error path in wm8955_register to properly free resources.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8955.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

sound/soc/codecs/wm8955.c +6 -4

index fedb764..5f02559 100644
@@ -964,7 +964,8 @@ static int wm8955_register(struct wm8955_priv *wm8955,
 
 	if (wm8955_codec) {
 		dev_err(codec->dev, "Another WM8955 is registered\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	mutex_init(&codec->mutex);

@@ -1047,18 +1048,19 @@ static int wm8955_register(struct wm8955_priv *wm8955,
 	ret = snd_soc_register_codec(codec);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		return ret;
+		goto err_enable;
 	}
 
 	ret = snd_soc_register_dai(&wm8955_dai);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		snd_soc_unregister_codec(codec);
-		return ret;
+		goto err_codec;
 	}
 
 	return 0;
 
+err_codec:
+	snd_soc_unregister_codec(codec);
 err_enable:
 	regulator_bulk_disable(ARRAY_SIZE(wm8955->supplies), wm8955->supplies);
 err_get:

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:55 UTC #9 lore raw

PATCH v2 9/12 wm8961: fix resource reclaim in wm8961_register error path

In reply to Axel Lin

This patch fixes the error path in wm8961_register to properly free resources.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8961.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

sound/soc/codecs/wm8961.c +5 -4

index 5b9a756..2549d3a 100644
@@ -1102,7 +1102,7 @@ static int wm8961_register(struct wm8961_priv *wm8961)
 	ret = wm8961_reset(codec);
 	if (ret < 0) {
 		dev_err(codec->dev, "Failed to issue reset\n");
-		return ret;
+		goto err;
 	}
 
 	/* Enable class W */

@@ -1147,18 +1147,19 @@ static int wm8961_register(struct wm8961_priv *wm8961)
 	ret = snd_soc_register_codec(codec);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		return ret;
+		goto err;
 	}
 
 	ret = snd_soc_register_dai(&wm8961_dai);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		snd_soc_unregister_codec(codec);
-		return ret;
+		goto err_codec;
 	}
 
 	return 0;
 
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
 	kfree(wm8961);
 	return ret;

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #10 lore raw

PATCH v2 10/12 wm8974: fix a memory leak if another WM8974 is registered

In reply to Axel Lin

wm8974 is allocated in wm8974_i2c_probe() but is not freed if wm8974_register() return -EINVAL (if another WM8974 is registered).

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm8974.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

sound/soc/codecs/wm8974.c +2 -1

index a2c4b2f..1468fe1 100644
@@ -670,7 +670,8 @@ static __devinit int wm8974_register(struct wm8974_priv *wm8974)
 
 	if (wm8974_codec) {
 		dev_err(codec->dev, "Another WM8974 is registered\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err;
 	}
 
 	mutex_init(&codec->mutex);

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #11 lore raw

PATCH v2 11/12 wm8978: fix a memory leak if a wm8978_register fail

In reply to Axel Lin

There is a memory leak found if wm8978_register() fail. This patch moves the buffer allocate and release at the same level to prevent the memory leak.

Signed-off-by Axel Lin <[email protected]>

Reviewed-by Guennadi Liakhovetski <[email protected]>

---
 sound/soc/codecs/wm8978.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

sound/soc/codecs/wm8978.c +7 -3

index 51d5f43..8a1ad77 100644
@@ -1076,7 +1076,6 @@ static __devinit int wm8978_register(struct wm8978_priv *wm8978)
 err_codec:
 	snd_soc_unregister_codec(codec);
 err:
-	kfree(wm8978);
 	return ret;
 }

@@ -1085,13 +1084,13 @@ static __devexit void wm8978_unregister(struct wm8978_priv *wm8978)
 	wm8978_set_bias_level(&wm8978->codec, SND_SOC_BIAS_OFF);
 	snd_soc_unregister_dai(&wm8978_dai);
 	snd_soc_unregister_codec(&wm8978->codec);
-	kfree(wm8978);
 	wm8978_codec = NULL;
 }
 
 static __devinit int wm8978_i2c_probe(struct i2c_client *i2c,
 				      const struct i2c_device_id *id)
 {
+	int ret;
 	struct wm8978_priv *wm8978;
 	struct snd_soc_codec *codec;

@@ -1107,13 +1106,18 @@ static __devinit int wm8978_i2c_probe(struct i2c_client *i2c,
 
 	codec->dev = &i2c->dev;
 
-	return wm8978_register(wm8978);
+	ret = wm8978_register(wm8978);
+	if (ret < 0)
+		kfree(wm8978);
+
+	return ret;
 }
 
 static __devexit int wm8978_i2c_remove(struct i2c_client *client)
 {
 	struct wm8978_priv *wm8978 = i2c_get_clientdata(client);
 	wm8978_unregister(wm8978);
+	kfree(wm8978);
 	return 0;
 }

Signature · 1 line

1.7.0.4

Axel Lin

author

2010-07-23 05:54 UTC #12 lore raw

PATCH v2 12/12 wm9081: fix resource reclaim in wm9081_register error path

In reply to Axel Lin

This patch fixes the error path in wm9081_register to properly free resources.

Signed-off-by Axel Lin <[email protected]>

---
 sound/soc/codecs/wm9081.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

sound/soc/codecs/wm9081.c +6 -5

index 13186fb..76b37ff 100644
@@ -1356,7 +1356,7 @@ static int wm9081_register(struct wm9081_priv *wm9081,
 	ret = snd_soc_codec_set_cache_io(codec, 8, 16, control);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to set cache I/O: %d\n", ret);
-		return ret;
+		goto err;
 	}
 
 	reg = snd_soc_read(codec, WM9081_SOFTWARE_RESET);

@@ -1369,7 +1369,7 @@ static int wm9081_register(struct wm9081_priv *wm9081,
 	ret = wm9081_reset(codec);
 	if (ret < 0) {
 		dev_err(codec->dev, "Failed to issue reset\n");
-		return ret;
+		goto err;
 	}
 
 	wm9081_set_bias_level(codec, SND_SOC_BIAS_STANDBY);

@@ -1388,18 +1388,19 @@ static int wm9081_register(struct wm9081_priv *wm9081,
 	ret = snd_soc_register_codec(codec);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register codec: %d\n", ret);
-		return ret;
+		goto err;
 	}
 
 	ret = snd_soc_register_dai(&wm9081_dai);
 	if (ret != 0) {
 		dev_err(codec->dev, "Failed to register DAI: %d\n", ret);
-		snd_soc_unregister_codec(codec);
-		return ret;
+		goto err_codec;
 	}
 
 	return 0;
 
+err_codec:
+	snd_soc_unregister_codec(codec);
 err:
 	kfree(wm9081);
 	return ret;

Signature · 1 line

1.7.0.4

Liam Girdwood

2010-07-23 18:22 UTC #13 lore raw

In reply to Axel Lin

Quoted 5 lines · Axel Lin

This serial of patches fixes memory leak and resource relaim in error path
I made a mistake that changed the subject line for some of the update patches. So. here is a re-send and all patches are marked as v2.

Ok, we will keep this in reserve atm if multi-component is held up.

Thanks

Liam

Signature · 3 lines

Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk