[PATCH 1/5] regmap: return ERR_PTR instead of NULL in regmap_init

[PATCH 1/5] regmap: return ERR_PTR instead of NULL in regmap_init

[Lars-Peter Clausen]

The regmap_init documentation states that it will either return a pointer to a
valid regmap structure or a ERR_PTR in case of an error. Currently it returns a
NULL pointer in case no bus or no config was given. Since NULL is not a
ERR_PTR a caller might assume that it is a pointer to a valid regmap structure,
so return a ERR_PTR(-EINVAL) instead.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/regmap.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
index b08df85..b84ebf9 100644
--- a/drivers/base/regmap/regmap.c
+++ b/drivers/base/regmap/regmap.c
@@ -139,7 +139,7 @@ struct regmap *regmap_init(struct device *dev,
 	int ret = -EINVAL;
 
 	if (!bus || !config)
-		return NULL;
+		goto err;
 
 	map = kzalloc(sizeof(*map), GFP_KERNEL);
 	if (map == NULL) {
-- 
1.7.7.1

[PATCH 2/5] regmap: Fix memory leak in regcache_hw_init error path

[Lars-Peter Clausen]

Make sure reg_defaults_raw gets freed in case of an error.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/regcache.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 6ab9f03..7944626 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -61,8 +61,10 @@ static int regcache_hw_init(struct regmap *map)
 
 	map->reg_defaults = kmalloc(count * sizeof(struct reg_default),
 				      GFP_KERNEL);
-	if (!map->reg_defaults)
-		return -ENOMEM;
+	if (!map->reg_defaults) {
+		ret = -ENOMEM;
+		goto err_free;
+	}
 
 	/* fill the reg_defaults */
 	map->num_reg_defaults = count;
@@ -77,6 +79,12 @@ static int regcache_hw_init(struct regmap *map)
 	}
 
 	return 0;
+
+err_free:
+	if (map->cache_free)
+		kfree(map->reg_defaults_raw);
+
+	return ret;
 }
 
 int regcache_init(struct regmap *map)
-- 
1.7.7.1

Re: [PATCH 2/5] regmap: Fix memory leak in regcache_hw_init error path

[Mark Brown]

On Mon, Nov 14, 2011 at 10:40:16AM +0100, Lars-Peter Clausen wrote:
> Make sure reg_defaults_raw gets freed in case of an error.

Applied, thanks.

[PATCH 3/5] regmap: Fix memory leak in regcache_init error path

[Lars-Peter Clausen]

Make sure all allocated memory gets freed again in case initializing the cache
failed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/regcache.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache.c b/drivers/base/regmap/regcache.c
index 7944626..27fae58 100644
--- a/drivers/base/regmap/regcache.c
+++ b/drivers/base/regmap/regcache.c
@@ -144,9 +144,18 @@ int regcache_init(struct regmap *map)
 	if (map->cache_ops->init) {
 		dev_dbg(map->dev, "Initializing %s cache\n",
 			map->cache_ops->name);
-		return map->cache_ops->init(map);
+		ret = map->cache_ops->init(map);
+		if (ret)
+			goto err_free;
 	}
 	return 0;
+
+err_free:
+	kfree(map->reg_defaults);
+	if (map->cache_free)
+		kfree(map->reg_defaults_raw);
+
+	return ret;
 }
 
 void regcache_exit(struct regmap *map)
-- 
1.7.7.1

Re: [PATCH 3/5] regmap: Fix memory leak in regcache_init error path

[Mark Brown]

On Mon, Nov 14, 2011 at 10:40:17AM +0100, Lars-Peter Clausen wrote:
> Make sure all allocated memory gets freed again in case initializing the cache
> failed.

Applied, thanks.

[PATCH 4/5] regmap: Do not call regcache_exit from regcache_rbtree_init error path

[Lars-Peter Clausen]

Calling regcache_exit from regcache_rbtree_init is a layering violation and
might cause double frees. In case of an error only free those structures which
have been allocated in this function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/regcache-rbtree.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index e314984..e71320f 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -17,6 +17,7 @@
 
 static int regcache_rbtree_write(struct regmap *map, unsigned int reg,
 				 unsigned int value);
+static int regcache_rbtree_exit(struct regmap *map);
 
 struct regcache_rbtree_node {
 	/* the actual rbtree node holding this block */
@@ -149,7 +150,7 @@ static int regcache_rbtree_init(struct regmap *map)
 	return 0;
 
 err:
-	regcache_exit(map);
+	regcache_rbtree_exit(map);
 	return ret;
 }
 
-- 
1.7.7.1

[PATCH 5/5] regmap: Do not call regcache_exit from regcache_lzo_init error path

[Lars-Peter Clausen]

Calling regcache_exit from regcache_lzo_init is a layering violation and
might cause double frees. In case of an error only free those structures which
have been allocated in this function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/base/regmap/regcache-lzo.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
index 226ffc1..b7d1614 100644
--- a/drivers/base/regmap/regcache-lzo.c
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -15,6 +15,8 @@
 
 #include "internal.h"
 
+static int regcache_lzo_exit(struct regmap *map);
+
 struct regcache_lzo_ctx {
 	void *wmem;
 	void *dst;
@@ -193,7 +195,7 @@ static int regcache_lzo_init(struct regmap *map)
 
 	return 0;
 err:
-	regcache_exit(map);
+	regcache_lzo_exit(map);
 	return ret;
 }
 
-- 
1.7.7.1

Re: [PATCH 5/5] regmap: Do not call regcache_exit from regcache_lzo_init error path

[Dimitris Papastamos]

On Mon, Nov 14, 2011 at 10:40:19AM +0100, Lars-Peter Clausen wrote:
> Calling regcache_exit from regcache_lzo_init is a layering violation and
> might cause double frees. In case of an error only free those structures which
> have been allocated in this function.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/base/regmap/regcache-lzo.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
> index 226ffc1..b7d1614 100644
> --- a/drivers/base/regmap/regcache-lzo.c
> +++ b/drivers/base/regmap/regcache-lzo.c
> @@ -15,6 +15,8 @@
>  
>  #include "internal.h"
>  
> +static int regcache_lzo_exit(struct regmap *map);
> +
>  struct regcache_lzo_ctx {
>  	void *wmem;
>  	void *dst;
> @@ -193,7 +195,7 @@ static int regcache_lzo_init(struct regmap *map)
>  
>  	return 0;
>  err:
> -	regcache_exit(map);
> +	regcache_lzo_exit(map);
>  	return ret;
>  }
>  
> -- 
> 1.7.7.1
> 
> 

All of them look good.

Acked-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Re: [PATCH 5/5] regmap: Do not call regcache_exit from regcache_lzo_init error path

[Mark Brown]

On Mon, Nov 14, 2011 at 10:40:19AM +0100, Lars-Peter Clausen wrote:
> Calling regcache_exit from regcache_lzo_init is a layering violation and
> might cause double frees. In case of an error only free those structures which
> have been allocated in this function.

You should also be explaining here why the cleanup done the core exit
function is still being done when the code is changed this way.  The
fact that it may be a layering violation doesn't mean that the cleanup
is actually being done anywhere else...

[PATCH 1/2] regmap: Do not call regcache_exit from regcache_rbtree_init error path

[Lars-Peter Clausen]

Calling regcache_exit from regcache_rbtree_init is first of all a layering
violation and secondly will cause double frees. regcache_exit will free buffers
allocated by the core, but the core will also free the same buffers when the
cacheops init callback returns an error. Thus we end up with a double free.
Fix this by not calling regcache_exit but only free those buffers which, have
been allocated in this function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache-rbtree.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache-rbtree.c b/drivers/base/regmap/regcache-rbtree.c
index e314984..e71320f 100644
--- a/drivers/base/regmap/regcache-rbtree.c
+++ b/drivers/base/regmap/regcache-rbtree.c
@@ -17,6 +17,7 @@
 
 static int regcache_rbtree_write(struct regmap *map, unsigned int reg,
 				 unsigned int value);
+static int regcache_rbtree_exit(struct regmap *map);
 
 struct regcache_rbtree_node {
 	/* the actual rbtree node holding this block */
@@ -149,7 +150,7 @@ static int regcache_rbtree_init(struct regmap *map)
 	return 0;
 
 err:
-	regcache_exit(map);
+	regcache_rbtree_exit(map);
 	return ret;
 }
 
-- 
1.7.7.1

[PATCH 2/2] regmap: Do not call regcache_exit from regcache_lzo_init error path

[Lars-Peter Clausen]

Calling regcache_exit from regcache_lzo_init is first of all a layering
violation and secondly will cause double frees. regcache_exit will free buffers
allocated by the core, but the core will also free the same buffers when the
cacheops init callback returns an error. Thus we end up with a double free.
Fix this by not calling regcache_exit but only free those buffers which, have
been allocated in this function.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Acked-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>
---
 drivers/base/regmap/regcache-lzo.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/base/regmap/regcache-lzo.c b/drivers/base/regmap/regcache-lzo.c
index 226ffc1..b7d1614 100644
--- a/drivers/base/regmap/regcache-lzo.c
+++ b/drivers/base/regmap/regcache-lzo.c
@@ -15,6 +15,8 @@
 
 #include "internal.h"
 
+static int regcache_lzo_exit(struct regmap *map);
+
 struct regcache_lzo_ctx {
 	void *wmem;
 	void *dst;
@@ -193,7 +195,7 @@ static int regcache_lzo_init(struct regmap *map)
 
 	return 0;
 err:
-	regcache_exit(map);
+	regcache_lzo_exit(map);
 	return ret;
 }
 
-- 
1.7.7.1

Re: [PATCH 1/2] regmap: Do not call regcache_exit from regcache_rbtree_init error path

[Mark Brown]

On Tue, Nov 15, 2011 at 01:34:40PM +0100, Lars-Peter Clausen wrote:
> Calling regcache_exit from regcache_rbtree_init is first of all a layering
> violation and secondly will cause double frees. regcache_exit will free buffers

Applied, thanks.

Re: [PATCH 1/5] regmap: return ERR_PTR instead of NULL in regmap_init

[Dimitris Papastamos]

On Mon, Nov 14, 2011 at 10:40:15AM +0100, Lars-Peter Clausen wrote:
> The regmap_init documentation states that it will either return a pointer to a
> valid regmap structure or a ERR_PTR in case of an error. Currently it returns a
> NULL pointer in case no bus or no config was given. Since NULL is not a
> ERR_PTR a caller might assume that it is a pointer to a valid regmap structure,
> so return a ERR_PTR(-EINVAL) instead.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
>  drivers/base/regmap/regmap.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c
> index b08df85..b84ebf9 100644
> --- a/drivers/base/regmap/regmap.c
> +++ b/drivers/base/regmap/regmap.c
> @@ -139,7 +139,7 @@ struct regmap *regmap_init(struct device *dev,
>  	int ret = -EINVAL;
>  
>  	if (!bus || !config)
> -		return NULL;
> +		goto err;
>  
>  	map = kzalloc(sizeof(*map), GFP_KERNEL);
>  	if (map == NULL) {
> -- 
> 1.7.7.1
> 
> 

Acked-by: Dimitris Papastamos <dp@opensource.wolfsonmicro.com>

Re: [PATCH 1/5] regmap: return ERR_PTR instead of NULL in regmap_init

[Mark Brown]

On Mon, Nov 14, 2011 at 10:40:15AM +0100, Lars-Peter Clausen wrote:
> The regmap_init documentation states that it will either return a pointer to a
> valid regmap structure or a ERR_PTR in case of an error. Currently it returns a
> NULL pointer in case no bus or no config was given. Since NULL is not a
> ERR_PTR a caller might assume that it is a pointer to a valid regmap structure,
> so return a ERR_PTR(-EINVAL) instead.

Applied, thanks.