All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
@ 2016-10-12  8:14 Dan Carpenter
  2016-10-12  8:30 ` Johannes Thumshirn
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2016-10-12  8:14 UTC (permalink / raw)
  To: jthumshirn; +Cc: linux-gpio

Hello Johannes Thumshirn,

The patch e1547af8c059: "pinctrl: berlin: Don't leak memory if
krealloc() fails" from Sep 30, 2016, leads to the following static
checker warning:

	drivers/pinctrl/berlin/berlin.c:244 berlin_pinctrl_build_state()
	warn: passing devm_ allocated variable to kfree. 'pctrl->functions'

drivers/pinctrl/berlin/berlin.c
   221  
   222          /* we will reallocate later */
   223          pctrl->functions = devm_kzalloc(&pdev->dev,
   224                                          max_functions * sizeof(*pctrl->functions),
   225                                          GFP_KERNEL);
   226          if (!pctrl->functions)
   227                  return -ENOMEM;
   228  
   229          /* register all functions */
   230          for (i = 0; i < pctrl->desc->ngroups; i++) {
   231                  desc_group = pctrl->desc->groups + i;
   232                  desc_function = desc_group->functions;
   233  
   234                  while (desc_function->name) {
   235                          berlin_pinctrl_add_function(pctrl, desc_function->name);
   236                          desc_function++;
   237                  }
   238          }
   239  
   240          functions = krealloc(pctrl->functions,
   241                               pctrl->nfunctions * sizeof(*pctrl->functions),
   242                               GFP_KERNEL);
   243          if (!functions) {
   244                  kfree(pctrl->functions);

This will lead to a double free.

   245                  return -ENOMEM;
   246          }
   247          pctrl->functions = functions;

I'm really concerned about this generally.  It's like we can't tell if
pctrl->functions is a managed allocation or not, and I can't immediately
see where it is freed when it's unmanaged.

   248  

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12  8:14 [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails Dan Carpenter
@ 2016-10-12  8:30 ` Johannes Thumshirn
  2016-10-12  8:45   ` Dan Carpenter
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2016-10-12  8:30 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-gpio

On Wed, Oct 12, 2016 at 11:14:22AM +0300, Dan Carpenter wrote:
> Hello Johannes Thumshirn,
> 
> The patch e1547af8c059: "pinctrl: berlin: Don't leak memory if
> krealloc() fails" from Sep 30, 2016, leads to the following static
> checker warning:
> 
> 	drivers/pinctrl/berlin/berlin.c:244 berlin_pinctrl_build_state()
> 	warn: passing devm_ allocated variable to kfree. 'pctrl->functions'
> 
> drivers/pinctrl/berlin/berlin.c
>    221  
>    222          /* we will reallocate later */
>    223          pctrl->functions = devm_kzalloc(&pdev->dev,
>    224                                          max_functions * sizeof(*pctrl->functions),
>    225                                          GFP_KERNEL);
>    226          if (!pctrl->functions)
>    227                  return -ENOMEM;
>    228  
>    229          /* register all functions */
>    230          for (i = 0; i < pctrl->desc->ngroups; i++) {
>    231                  desc_group = pctrl->desc->groups + i;
>    232                  desc_function = desc_group->functions;
>    233  
>    234                  while (desc_function->name) {
>    235                          berlin_pinctrl_add_function(pctrl, desc_function->name);
>    236                          desc_function++;
>    237                  }
>    238          }
>    239  
>    240          functions = krealloc(pctrl->functions,
>    241                               pctrl->nfunctions * sizeof(*pctrl->functions),
>    242                               GFP_KERNEL);
>    243          if (!functions) {
>    244                  kfree(pctrl->functions);
> 
> This will lead to a double free.
> 
>    245                  return -ENOMEM;
>    246          }
>    247          pctrl->functions = functions;
> 
> I'm really concerned about this generally.  It's like we can't tell if
> pctrl->functions is a managed allocation or not, and I can't immediately
> see where it is freed when it's unmanaged.
> 
>    248  
> 
> regards,
> dan carpenter

Oh I see. Damn, missed the devm_kzalloc(). But shouldn't we avoid krealloc()
on devm_kzalloc() in general? krealloc() calls kfree() if the reallocation
succeeded and this will break the devres tracking, wouldn't it?

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12  8:30 ` Johannes Thumshirn
@ 2016-10-12  8:45   ` Dan Carpenter
  2016-10-12  9:44     ` Johannes Thumshirn
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2016-10-12  8:45 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: linux-gpio

On Wed, Oct 12, 2016 at 10:30:02AM +0200, Johannes Thumshirn wrote:
> Oh I see. Damn, missed the devm_kzalloc(). But shouldn't we avoid krealloc()
> on devm_kzalloc() in general? krealloc() calls kfree() if the reallocation
> succeeded and this will break the devres tracking, wouldn't it?

Good point.  I will update my test to check for that.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12  8:45   ` Dan Carpenter
@ 2016-10-12  9:44     ` Johannes Thumshirn
  2016-10-12 11:19       ` Dan Carpenter
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2016-10-12  9:44 UTC (permalink / raw)
  To: Dan Carpenter, Linus Walleij; +Cc: linux-gpio

On Wed, Oct 12, 2016 at 11:45:27AM +0300, Dan Carpenter wrote:
> On Wed, Oct 12, 2016 at 10:30:02AM +0200, Johannes Thumshirn wrote:
> > Oh I see. Damn, missed the devm_kzalloc(). But shouldn't we avoid krealloc()
> > on devm_kzalloc() in general? krealloc() calls kfree() if the reallocation
> > succeeded and this will break the devres tracking, wouldn't it?
> 
> Good point.  I will update my test to check for that.

Well the initial problem still stands, how do we fix the possible double
free? I think I could "just" change the devm_kzalloc() to kzalloc() (and
introduce cleanups) but I'm not sure this is a good solution. The whoule point
of these devm stuff is to apply a safety net, isn't it?

Linus how shall we proceed? Reverting the patch keeps the double-free (IMHO
but I'm not 100% sure how this devres tracking works if we do a realloc on the
pointer) and reintroduces the memory leak.

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12  9:44     ` Johannes Thumshirn
@ 2016-10-12 11:19       ` Dan Carpenter
  2016-10-12 12:36         ` Johannes Thumshirn
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2016-10-12 11:19 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Linus Walleij, linux-gpio

On Wed, Oct 12, 2016 at 11:44:36AM +0200, Johannes Thumshirn wrote:
> On Wed, Oct 12, 2016 at 11:45:27AM +0300, Dan Carpenter wrote:
> > On Wed, Oct 12, 2016 at 10:30:02AM +0200, Johannes Thumshirn wrote:
> > > Oh I see. Damn, missed the devm_kzalloc(). But shouldn't we avoid krealloc()
> > > on devm_kzalloc() in general? krealloc() calls kfree() if the reallocation
> > > succeeded and this will break the devres tracking, wouldn't it?
> > 
> > Good point.  I will update my test to check for that.
> 
> Well the initial problem still stands, how do we fix the possible double
> free? I think I could "just" change the devm_kzalloc() to kzalloc() (and
> introduce cleanups) but I'm not sure this is a good solution. The whoule point
> of these devm stuff is to apply a safety net, isn't it?

No.  It's just a convenience because we allocate so much stuff on probe.
A few people use it like that, where they try to manage their own memory
and use devm_ at the same time and it annoys me.

Just use kzalloc().  But make sure there is a free somewhere.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12 11:19       ` Dan Carpenter
@ 2016-10-12 12:36         ` Johannes Thumshirn
  2016-10-12 17:06           ` Dan Carpenter
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2016-10-12 12:36 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Linus Walleij, linux-gpio

On Wed, Oct 12, 2016 at 02:19:24PM +0300, Dan Carpenter wrote:
> On Wed, Oct 12, 2016 at 11:44:36AM +0200, Johannes Thumshirn wrote:
> > On Wed, Oct 12, 2016 at 11:45:27AM +0300, Dan Carpenter wrote:
> > > On Wed, Oct 12, 2016 at 10:30:02AM +0200, Johannes Thumshirn wrote:
> > > > Oh I see. Damn, missed the devm_kzalloc(). But shouldn't we avoid krealloc()
> > > > on devm_kzalloc() in general? krealloc() calls kfree() if the reallocation
> > > > succeeded and this will break the devres tracking, wouldn't it?
> > > 
> > > Good point.  I will update my test to check for that.
> > 
> > Well the initial problem still stands, how do we fix the possible double
> > free? I think I could "just" change the devm_kzalloc() to kzalloc() (and
> > introduce cleanups) but I'm not sure this is a good solution. The whoule point
> > of these devm stuff is to apply a safety net, isn't it?
> 
> No.  It's just a convenience because we allocate so much stuff on probe.
> A few people use it like that, where they try to manage their own memory
> and use devm_ at the same time and it annoys me.
> 
> Just use kzalloc().  But make sure there is a free somewhere.

Unfortunately changing the allocation function for pctrl->functions in
berlin_pinctrl_build_state() also ment that I had to change it for the
function group so the error handling path is not so straight forward.
That's what I came up with:

>From 41899a7bce490a49b5cebe9586a07ff7e2f1b784 Mon Sep 17 00:00:00 2001
From: Johannes Thumshirn <jthumshirn@suse.de>
Date: Wed, 12 Oct 2016 14:15:52 +0200
Subject: [PATCH] pinctrl: berlin: don't krealloc devm managed memory

Don't krealloc devm managed memory instead allocate the memory which will
likely get reallocated using the traditional unmanaged allocators.

Fixes: e1547af8c (pinctrl: berlin: Don't leak memory if krealloc() fails)
Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/pinctrl/berlin/berlin-bg2.c   |  1 +
 drivers/pinctrl/berlin/berlin-bg2cd.c |  1 +
 drivers/pinctrl/berlin/berlin-bg2q.c  |  1 +
 drivers/pinctrl/berlin/berlin-bg4ct.c |  1 +
 drivers/pinctrl/berlin/berlin.c       | 60 ++++++++++++++++++++++++-----------
 drivers/pinctrl/berlin/berlin.h       | 21 ++++++++++++
 6 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/drivers/pinctrl/berlin/berlin-bg2.c b/drivers/pinctrl/berlin/berlin-bg2.c
index fabe728..733bc62 100644
--- a/drivers/pinctrl/berlin/berlin-bg2.c
+++ b/drivers/pinctrl/berlin/berlin-bg2.c
@@ -239,6 +239,7 @@ static int berlin2_pinctrl_probe(struct platform_device *pdev)
 
 static struct platform_driver berlin2_pinctrl_driver = {
 	.probe	= berlin2_pinctrl_probe,
+	.remove = berlin_pinctrl_remove,
 	.driver	= {
 		.name = "berlin-bg2-pinctrl",
 		.of_match_table = berlin2_pinctrl_match,
diff --git a/drivers/pinctrl/berlin/berlin-bg2cd.c b/drivers/pinctrl/berlin/berlin-bg2cd.c
index ad8c758..70fcc77 100644
--- a/drivers/pinctrl/berlin/berlin-bg2cd.c
+++ b/drivers/pinctrl/berlin/berlin-bg2cd.c
@@ -184,6 +184,7 @@ static int berlin2cd_pinctrl_probe(struct platform_device *pdev)
 
 static struct platform_driver berlin2cd_pinctrl_driver = {
 	.probe	= berlin2cd_pinctrl_probe,
+	.remove = berlin_pinctrl_remove,
 	.driver	= {
 		.name = "berlin-bg2cd-pinctrl",
 		.of_match_table = berlin2cd_pinctrl_match,
diff --git a/drivers/pinctrl/berlin/berlin-bg2q.c b/drivers/pinctrl/berlin/berlin-bg2q.c
index cd171ae..a0e80f4 100644
--- a/drivers/pinctrl/berlin/berlin-bg2q.c
+++ b/drivers/pinctrl/berlin/berlin-bg2q.c
@@ -401,6 +401,7 @@ static int berlin2q_pinctrl_probe(struct platform_device *pdev)
 
 static struct platform_driver berlin2q_pinctrl_driver = {
 	.probe	= berlin2q_pinctrl_probe,
+	.remove = berlin_pinctrl_remove,
 	.driver	= {
 		.name = "berlin-bg2q-pinctrl",
 		.of_match_table = berlin2q_pinctrl_match,
diff --git a/drivers/pinctrl/berlin/berlin-bg4ct.c b/drivers/pinctrl/berlin/berlin-bg4ct.c
index 0917204..f91eaae 100644
--- a/drivers/pinctrl/berlin/berlin-bg4ct.c
+++ b/drivers/pinctrl/berlin/berlin-bg4ct.c
@@ -491,6 +491,7 @@ static int berlin4ct_pinctrl_probe(struct platform_device *pdev)
 
 static struct platform_driver berlin4ct_pinctrl_driver = {
 	.probe	= berlin4ct_pinctrl_probe,
+	.remove = berlin_pinctrl_remove,
 	.driver	= {
 		.name = "berlin4ct-pinctrl",
 		.of_match_table = berlin4ct_pinctrl_match,
diff --git a/drivers/pinctrl/berlin/berlin.c b/drivers/pinctrl/berlin/berlin.c
index 76281c8..0774973 100644
--- a/drivers/pinctrl/berlin/berlin.c
+++ b/drivers/pinctrl/berlin/berlin.c
@@ -26,15 +26,6 @@
 #include "../pinctrl-utils.h"
 #include "berlin.h"
 
-struct berlin_pinctrl {
-	struct regmap *regmap;
-	struct device *dev;
-	const struct berlin_pinctrl_desc *desc;
-	struct berlin_pinctrl_function *functions;
-	unsigned nfunctions;
-	struct pinctrl_dev *pctrl_dev;
-};
-
 static int berlin_pinctrl_get_group_count(struct pinctrl_dev *pctrl_dev)
 {
 	struct berlin_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctrl_dev);
@@ -203,6 +194,26 @@ static int berlin_pinctrl_add_function(struct berlin_pinctrl *pctrl,
 	return 0;
 }
 
+void berlin_pinctrl_free_funcgroups(struct berlin_pinctrl *pctrl)
+{
+	struct berlin_desc_group const *desc_group;
+	struct berlin_desc_function const *desc_function;
+	int i;
+
+	for (i = 0; i < pctrl->desc->ngroups; i++) {
+		desc_group = pctrl->desc->groups + i;
+		desc_function = desc_group->functions;
+
+		while (desc_function->name) {
+			struct berlin_pinctrl_function
+				*function = pctrl->functions;
+
+			kfree(function->groups);
+			desc_function++;
+		}
+	}
+}
+
 static int berlin_pinctrl_build_state(struct platform_device *pdev)
 {
 	struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev);
@@ -210,6 +221,7 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 	struct berlin_desc_function const *desc_function;
 	struct berlin_pinctrl_function *functions;
 	int i, max_functions = 0;
+	int ret;
 
 	pctrl->nfunctions = 0;
 
@@ -220,9 +232,8 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 	}
 
 	/* we will reallocate later */
-	pctrl->functions = devm_kzalloc(&pdev->dev,
-					max_functions * sizeof(*pctrl->functions),
-					GFP_KERNEL);
+	pctrl->functions = kcalloc(max_functions, sizeof(*pctrl->functions),
+				   GFP_KERNEL);
 	if (!pctrl->functions)
 		return -ENOMEM;
 
@@ -265,17 +276,21 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 				function++;
 			}
 
-			if (!found)
-				return -EINVAL;
+			if (!found) {
+				ret = -EINVAL;
+				goto free_functions;
+			}
 
 			if (!function->groups) {
 				function->groups =
-					devm_kzalloc(&pdev->dev,
-						     function->ngroups * sizeof(char *),
-						     GFP_KERNEL);
+					kcalloc(function->ngroups,
+						sizeof(char *),
+						GFP_KERNEL);
 
-				if (!function->groups)
-					return -ENOMEM;
+				if (!function->groups) {
+					ret = -ENOMEM;
+					goto free_functions;
+				}
 			}
 
 			groups = function->groups;
@@ -289,6 +304,11 @@ static int berlin_pinctrl_build_state(struct platform_device *pdev)
 	}
 
 	return 0;
+
+free_functions:
+	berlin_pinctrl_free_funcgroups(pctrl);
+	kfree(pctrl->functions);
+	return ret;
 }
 
 static struct pinctrl_desc berlin_pctrl_desc = {
@@ -326,6 +346,8 @@ int berlin_pinctrl_probe_regmap(struct platform_device *pdev,
 						 pctrl);
 	if (IS_ERR(pctrl->pctrl_dev)) {
 		dev_err(dev, "failed to register pinctrl driver\n");
+		berlin_pinctrl_free_funcgroups(pctrl);
+		kfree(pctrl->functions);
 		return PTR_ERR(pctrl->pctrl_dev);
 	}
 
diff --git a/drivers/pinctrl/berlin/berlin.h b/drivers/pinctrl/berlin/berlin.h
index e9b30f9..a50686c 100644
--- a/drivers/pinctrl/berlin/berlin.h
+++ b/drivers/pinctrl/berlin/berlin.h
@@ -13,6 +13,17 @@
 #ifndef __PINCTRL_BERLIN_H
 #define __PINCTRL_BERLIN_H
 
+#include <linux/slab.h>
+
+struct berlin_pinctrl {
+	struct regmap *regmap;
+	struct device *dev;
+	const struct berlin_pinctrl_desc *desc;
+	struct berlin_pinctrl_function *functions;
+	unsigned nfunctions;
+	struct pinctrl_dev *pctrl_dev;
+};
+
 struct berlin_desc_function {
 	const char	*name;
 	u8		muxval;
@@ -61,5 +72,15 @@ int berlin_pinctrl_probe(struct platform_device *pdev,
 int berlin_pinctrl_probe_regmap(struct platform_device *pdev,
 				const struct berlin_pinctrl_desc *desc,
 				struct regmap *regmap);
+void berlin_pinctrl_free_funcgroups(struct berlin_pinctrl *pctrl);
+
+static inline int berlin_pinctrl_remove(struct platform_device *pdev)
+{
+	struct berlin_pinctrl *pctrl = platform_get_drvdata(pdev);
+
+	berlin_pinctrl_free_funcgroups(pctrl);
+	kfree(pctrl->functions);
+	return 0;
+}
 
 #endif /* __PINCTRL_BERLIN_H */
-- 
1.8.5.6


-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12 12:36         ` Johannes Thumshirn
@ 2016-10-12 17:06           ` Dan Carpenter
  2016-10-13 13:09             ` Johannes Thumshirn
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2016-10-12 17:06 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Linus Walleij, linux-gpio

On Wed, Oct 12, 2016 at 02:36:16PM +0200, Johannes Thumshirn wrote:

> +void berlin_pinctrl_free_funcgroups(struct berlin_pinctrl *pctrl)
> +{
> +	struct berlin_desc_group const *desc_group;
> +	struct berlin_desc_function const *desc_function;
> +	int i;
> +
> +	for (i = 0; i < pctrl->desc->ngroups; i++) {
> +		desc_group = pctrl->desc->groups + i;
> +		desc_function = desc_group->functions;
> +
> +		while (desc_function->name) {
> +			struct berlin_pinctrl_function
> +				*function = pctrl->functions;
> +
> +			kfree(function->groups);

It looks like, we're just freeing pctrl->functions->groups over and
over.

Not taking advantage of managed allocations has made this stuff so much
more complicated.  I'd be tempted almost to just delete the krealloc()
and waste a little RAM...  (I have no idea how much RAM I'm talking
about here so maybe this is a bad idea).

> +			desc_function++;
> +		}
> +	}
> +}
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-12 17:06           ` Dan Carpenter
@ 2016-10-13 13:09             ` Johannes Thumshirn
  2016-10-18 12:34               ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Thumshirn @ 2016-10-13 13:09 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Linus Walleij, linux-gpio

On Wed, Oct 12, 2016 at 08:06:40PM +0300, Dan Carpenter wrote:
> On Wed, Oct 12, 2016 at 02:36:16PM +0200, Johannes Thumshirn wrote:
> 
> > +void berlin_pinctrl_free_funcgroups(struct berlin_pinctrl *pctrl)
> > +{
> > +	struct berlin_desc_group const *desc_group;
> > +	struct berlin_desc_function const *desc_function;
> > +	int i;
> > +
> > +	for (i = 0; i < pctrl->desc->ngroups; i++) {
> > +		desc_group = pctrl->desc->groups + i;
> > +		desc_function = desc_group->functions;
> > +
> > +		while (desc_function->name) {
> > +			struct berlin_pinctrl_function
> > +				*function = pctrl->functions;
> > +
> > +			kfree(function->groups);
> 
> It looks like, we're just freeing pctrl->functions->groups over and
> over.
> 
> Not taking advantage of managed allocations has made this stuff so much
> more complicated.  I'd be tempted almost to just delete the krealloc()
> and waste a little RAM...  (I have no idea how much RAM I'm talking
> about here so maybe this is a bad idea).

Hmm yes. But I think it's best to just drop the patch from
linux-pinctrl/for-next (and the sunxi one as well as it's suffering from the
same pain). Let's get it back to the less broken state it has been in.

@Linus are you ok with dropping these patches from your queue or shall I send
you reverts?

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-13 13:09             ` Johannes Thumshirn
@ 2016-10-18 12:34               ` Linus Walleij
  2016-10-18 14:12                 ` Johannes Thumshirn
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2016-10-18 12:34 UTC (permalink / raw)
  To: Johannes Thumshirn; +Cc: Dan Carpenter, linux-gpio

On Thu, Oct 13, 2016 at 3:09 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:

> @Linus are you ok with dropping these patches from your queue or shall I send
> you reverts?

Dropping them, rebasing my for-next test branch on v4.9-rc1 anyway.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails
  2016-10-18 12:34               ` Linus Walleij
@ 2016-10-18 14:12                 ` Johannes Thumshirn
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2016-10-18 14:12 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Dan Carpenter, linux-gpio

On Tue, Oct 18, 2016 at 02:34:23PM +0200, Linus Walleij wrote:
> On Thu, Oct 13, 2016 at 3:09 PM, Johannes Thumshirn <jthumshirn@suse.de> wrote:
> 
> > @Linus are you ok with dropping these patches from your queue or shall I send
> > you reverts?
> 
> Dropping them, rebasing my for-next test branch on v4.9-rc1 anyway.

Thanks,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-10-18 14:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-12  8:14 [bug report] pinctrl: berlin: Don't leak memory if krealloc() fails Dan Carpenter
2016-10-12  8:30 ` Johannes Thumshirn
2016-10-12  8:45   ` Dan Carpenter
2016-10-12  9:44     ` Johannes Thumshirn
2016-10-12 11:19       ` Dan Carpenter
2016-10-12 12:36         ` Johannes Thumshirn
2016-10-12 17:06           ` Dan Carpenter
2016-10-13 13:09             ` Johannes Thumshirn
2016-10-18 12:34               ` Linus Walleij
2016-10-18 14:12                 ` Johannes Thumshirn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.