2010-07-23 05:54:07

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 00/12] 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(-)


2010-07-23 05:54:09

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 01/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(-)

diff --git a/sound/soc/codecs/ad1836.c b/sound/soc/codecs/ad1836.c
index 2175384..a01006c 100644
--- a/sound/soc/codecs/ad1836.c
+++ b/sound/soc/codecs/ad1836.c
@@ -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;
}

--
1.7.0.4

2010-07-23 05:54:13

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 02/12] ak4642: fix a memory leak if failed to initialise AK4642

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(-)

diff --git a/sound/soc/codecs/ak4642.c b/sound/soc/codecs/ak4642.c
index 7528a54..4feefa8 100644
--- a/sound/soc/codecs/ak4642.c
+++ b/sound/soc/codecs/ak4642.c
@@ -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;
}
--
1.7.0.4

2010-07-23 05:54:26

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 04/12] wm8523: fix resource reclaim in wm8523_register error path

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(-)

diff --git a/sound/soc/codecs/wm8523.c b/sound/soc/codecs/wm8523.c
index 37242a7..0ad039b 100644
--- a/sound/soc/codecs/wm8523.c
+++ b/sound/soc/codecs/wm8523.c
@@ -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:
--
1.7.0.4

2010-07-23 05:54:38

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 10/12] wm8974: fix a memory leak if another WM8974 is registered

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(-)

diff --git a/sound/soc/codecs/wm8974.c b/sound/soc/codecs/wm8974.c
index a2c4b2f..1468fe1 100644
--- a/sound/soc/codecs/wm8974.c
+++ b/sound/soc/codecs/wm8974.c
@@ -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);
--
1.7.0.4

2010-07-23 05:54:31

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 08/12] wm8955: fix resource reclaim in wm8955_register error path

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(-)

diff --git a/sound/soc/codecs/wm8955.c b/sound/soc/codecs/wm8955.c
index fedb764..5f02559 100644
--- a/sound/soc/codecs/wm8955.c
+++ b/sound/soc/codecs/wm8955.c
@@ -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:
--
1.7.0.4

2010-07-23 05:54:42

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 12/12] wm9081: fix resource reclaim in wm9081_register error path

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(-)

diff --git a/sound/soc/codecs/wm9081.c b/sound/soc/codecs/wm9081.c
index 13186fb..76b37ff 100644
--- a/sound/soc/codecs/wm9081.c
+++ b/sound/soc/codecs/wm9081.c
@@ -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;
--
1.7.0.4

2010-07-23 05:54:57

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 11/12] wm8978: fix a memory leak if a wm8978_register fail

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(-)

diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
index 51d5f43..8a1ad77 100644
--- a/sound/soc/codecs/wm8978.c
+++ b/sound/soc/codecs/wm8978.c
@@ -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;
}

--
1.7.0.4

2010-07-23 05:55:15

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 09/12] wm8961: fix resource reclaim in wm8961_register error path

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(-)

diff --git a/sound/soc/codecs/wm8961.c b/sound/soc/codecs/wm8961.c
index 5b9a756..2549d3a 100644
--- a/sound/soc/codecs/wm8961.c
+++ b/sound/soc/codecs/wm8961.c
@@ -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;
--
1.7.0.4

2010-07-23 05:55:44

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 07/12] wm8940: fix a memory leak if wm8940_register return error

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(-)

diff --git a/sound/soc/codecs/wm8940.c b/sound/soc/codecs/wm8940.c
index e3c4bbf..f0c1113 100644
--- a/sound/soc/codecs/wm8940.c
+++ b/sound/soc/codecs/wm8940.c
@@ -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)
--
1.7.0.4

2010-07-23 05:54:24

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 05/12] wm8711: fix a memory leak if another WM8711 is registered

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(-)

diff --git a/sound/soc/codecs/wm8711.c b/sound/soc/codecs/wm8711.c
index effb14e..e2dba07 100644
--- a/sound/soc/codecs/wm8711.c
+++ b/sound/soc/codecs/wm8711.c
@@ -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);
--
1.7.0.4

2010-07-23 05:56:04

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 06/12] wm8904: fix resource reclaim in wm8904_register error path

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(-)

diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 87f14f8..f7dcabf 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -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:
--
1.7.0.4

2010-07-23 05:56:08

by Axel Lin

[permalink] [raw]
Subject: [PATCH v2 03/12] da7210: fix a memory leak if failed to initialise da7210 audio codec

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(-)

diff --git a/sound/soc/codecs/da7210.c b/sound/soc/codecs/da7210.c
index 75af2d6..3b9a6cc 100644
--- a/sound/soc/codecs/da7210.c
+++ b/sound/soc/codecs/da7210.c
@@ -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;
}
--
1.7.0.4

2010-07-23 18:22:04

by Liam Girdwood

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] sound/alsa/soc/codec: fix memory leak and resource relaim in error path

On Fri, 2010-07-23 at 05:53 +0000, Axel Lin wrote:
> 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
--
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk