| Message ID | alpine.LNX.2.00.1208072014140.3227@swampdragon.chaosbits.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 7 August 2012 23:46, Jesper Juhl <jj@chaosbits.net> wrote: > If omap4_keypad_parse_dt() does not return 0 (zero) in > omap4_keypad_probe() we will leak the memory we allocated for > 'keypad_data' with kzalloc() when we return and the variable goes out > of scope. How about using devm_kzalloc() instead which will take care of freeing the memory on detach? > Fix the leak by jumping to the 'err_free_keypad' label where we > properly free the allocated memory, instead of returning directly. > > Signed-off-by: Jesper Juhl <jj@chaosbits.net> > --- > drivers/input/keyboard/omap4-keypad.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > Compile tested only. > > diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c > index c05f98c..9c2ac87 100644 > --- a/drivers/input/keyboard/omap4-keypad.c > +++ b/drivers/input/keyboard/omap4-keypad.c > @@ -281,7 +281,7 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev) > } else { > error = omap4_keypad_parse_dt(&pdev->dev, keypad_data); > if (error) > - return error; > + goto err_free_keypad; > } > > res = request_mem_region(res->start, resource_size(res), pdev->name); > -- > 1.7.11.4 > > > -- > Jesper Juhl <jj@chaosbits.net> http://www.chaosbits.net/ > Don't top-post http://www.catb.org/jargon/html/T/top-post.html > Plain text mails only, please. > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
On Wed, 8 Aug 2012, Sachin Kamat wrote: > On 7 August 2012 23:46, Jesper Juhl <jj@chaosbits.net> wrote: > > If omap4_keypad_parse_dt() does not return 0 (zero) in > > omap4_keypad_probe() we will leak the memory we allocated for > > 'keypad_data' with kzalloc() when we return and the variable goes out > > of scope. > > How about using devm_kzalloc() instead which will take care of freeing > the memory on detach? > Perhaps. I'm not (yet) familiar with how that function works, so I had not considered it. I'll look into it.
On Wednesday, August 08, 2012 09:36:51 PM Jesper Juhl wrote: > On Wed, 8 Aug 2012, Sachin Kamat wrote: > > On 7 August 2012 23:46, Jesper Juhl <jj@chaosbits.net> wrote: > > > If omap4_keypad_parse_dt() does not return 0 (zero) in > > > omap4_keypad_probe() we will leak the memory we allocated for > > > 'keypad_data' with kzalloc() when we return and the variable goes out > > > of scope. > > > > How about using devm_kzalloc() instead which will take care of freeing > > the memory on detach? > > Perhaps. I'm not (yet) familiar with how that function works, so I had > not considered it. I'll look into it. Actually please not yet - I guess at some point I'll have to add devm_* variants for input_device_* operations but for now I prefer not to mix the 2 styles of managing resources. BTW, I think I need to redo a few patches so I plan on folding this fix into the original change. Thanks.
On Wed, 8 Aug 2012, Dmitry Torokhov wrote: > On Wednesday, August 08, 2012 09:36:51 PM Jesper Juhl wrote: > > On Wed, 8 Aug 2012, Sachin Kamat wrote: > > > On 7 August 2012 23:46, Jesper Juhl <jj@chaosbits.net> wrote: > > > > If omap4_keypad_parse_dt() does not return 0 (zero) in > > > > omap4_keypad_probe() we will leak the memory we allocated for > > > > 'keypad_data' with kzalloc() when we return and the variable goes out > > > > of scope. > > > > > > How about using devm_kzalloc() instead which will take care of freeing > > > the memory on detach? > > > > Perhaps. I'm not (yet) familiar with how that function works, so I had > > not considered it. I'll look into it. > > Actually please not yet - I guess at some point I'll have to add devm_* > variants for input_device_* operations but for now I prefer not to mix > the 2 styles of managing resources. > > BTW, I think I need to redo a few patches so I plan on folding this fix > into the original change. > Ok. I'll just leave this alone for now then and let you do whatever you feel is best :-) .
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c index c05f98c..9c2ac87 100644 --- a/drivers/input/keyboard/omap4-keypad.c +++ b/drivers/input/keyboard/omap4-keypad.c @@ -281,7 +281,7 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev) } else { error = omap4_keypad_parse_dt(&pdev->dev, keypad_data); if (error) - return error; + goto err_free_keypad; } res = request_mem_region(res->start, resource_size(res), pdev->name);
If omap4_keypad_parse_dt() does not return 0 (zero) in omap4_keypad_probe() we will leak the memory we allocated for 'keypad_data' with kzalloc() when we return and the variable goes out of scope. Fix the leak by jumping to the 'err_free_keypad' label where we properly free the allocated memory, instead of returning directly. Signed-off-by: Jesper Juhl <jj@chaosbits.net> --- drivers/input/keyboard/omap4-keypad.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Compile tested only.