* [PATCH] edd: fix possible memory leak in edd_init() error path
@ 2010-07-09 10:50 Axel Lin
2010-07-11 9:01 ` Pekka Enberg
2010-07-12 19:49 ` Andrew Morton
0 siblings, 2 replies; 5+ messages in thread
From: Axel Lin @ 2010-07-09 10:50 UTC (permalink / raw)
To: linux-kernel
Cc: Stephen Hemminger, Jens Axboe, David Teigland, Pekka Enberg,
Hans J. Koch
The error may happen at any iteration of the for loop,
this patch properly unregisters already registed edd_devices in error path.
Signed-off-by: Axel Lin <axel.lin@gmail.com>
---
drivers/firmware/edd.c | 23 ++++++++++++++++-------
1 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
index 110e24e..b75082c 100644
--- a/drivers/firmware/edd.c
+++ b/drivers/firmware/edd.c
@@ -744,7 +744,7 @@ static inline int edd_num_devices(void)
static int __init
edd_init(void)
{
- unsigned int i;
+ int i;
int rc=0;
struct edd_device *edev;
@@ -760,21 +760,30 @@ edd_init(void)
if (!edd_kset)
return -ENOMEM;
- for (i = 0; i < edd_num_devices() && !rc; i++) {
+ for (i = 0; i < edd_num_devices(); i++) {
edev = kzalloc(sizeof (*edev), GFP_KERNEL);
- if (!edev)
- return -ENOMEM;
+ if (!edev) {
+ rc = -ENOMEM;
+ goto out;
+ }
rc = edd_device_register(edev, i);
if (rc) {
kfree(edev);
- break;
+ goto out;
}
edd_devices[i] = edev;
}
- if (rc)
- kset_unregister(edd_kset);
+ return 0;
+
+out:
+ while (--i >= 0) {
+ edev = edd_devices[i];
+ if (edev)
+ edd_device_unregister(edev);
+ }
+ kset_unregister(edd_kset);
return rc;
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] edd: fix possible memory leak in edd_init() error path
2010-07-09 10:50 [PATCH] edd: fix possible memory leak in edd_init() error path Axel Lin
@ 2010-07-11 9:01 ` Pekka Enberg
2010-07-12 19:49 ` Andrew Morton
1 sibling, 0 replies; 5+ messages in thread
From: Pekka Enberg @ 2010-07-11 9:01 UTC (permalink / raw)
To: Axel Lin
Cc: linux-kernel, Stephen Hemminger, Jens Axboe, David Teigland,
Hans J. Koch
Axel Lin wrote:
> The error may happen at any iteration of the for loop,
> this patch properly unregisters already registed edd_devices in error path.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
Looks good to me.
Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
> drivers/firmware/edd.c | 23 ++++++++++++++++-------
> 1 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> index 110e24e..b75082c 100644
> --- a/drivers/firmware/edd.c
> +++ b/drivers/firmware/edd.c
> @@ -744,7 +744,7 @@ static inline int edd_num_devices(void)
> static int __init
> edd_init(void)
> {
> - unsigned int i;
> + int i;
> int rc=0;
> struct edd_device *edev;
>
> @@ -760,21 +760,30 @@ edd_init(void)
> if (!edd_kset)
> return -ENOMEM;
>
> - for (i = 0; i < edd_num_devices() && !rc; i++) {
> + for (i = 0; i < edd_num_devices(); i++) {
> edev = kzalloc(sizeof (*edev), GFP_KERNEL);
> - if (!edev)
> - return -ENOMEM;
> + if (!edev) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> rc = edd_device_register(edev, i);
> if (rc) {
> kfree(edev);
> - break;
> + goto out;
> }
> edd_devices[i] = edev;
> }
>
> - if (rc)
> - kset_unregister(edd_kset);
> + return 0;
> +
> +out:
> + while (--i >= 0) {
> + edev = edd_devices[i];
> + if (edev)
> + edd_device_unregister(edev);
> + }
> + kset_unregister(edd_kset);
> return rc;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] edd: fix possible memory leak in edd_init() error path
2010-07-09 10:50 [PATCH] edd: fix possible memory leak in edd_init() error path Axel Lin
2010-07-11 9:01 ` Pekka Enberg
@ 2010-07-12 19:49 ` Andrew Morton
2010-07-13 1:17 ` Axel Lin
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2010-07-12 19:49 UTC (permalink / raw)
To: Axel Lin
Cc: linux-kernel, Stephen Hemminger, Jens Axboe, David Teigland,
Pekka Enberg, Hans J. Koch
On Fri, 09 Jul 2010 18:50:07 +0800
Axel Lin <axel.lin@gmail.com> wrote:
> The error may happen at any iteration of the for loop,
> this patch properly unregisters already registed edd_devices in error path.
>
> Signed-off-by: Axel Lin <axel.lin@gmail.com>
> ---
> drivers/firmware/edd.c | 23 ++++++++++++++++-------
> 1 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
> index 110e24e..b75082c 100644
> --- a/drivers/firmware/edd.c
> +++ b/drivers/firmware/edd.c
> @@ -744,7 +744,7 @@ static inline int edd_num_devices(void)
> static int __init
> edd_init(void)
> {
> - unsigned int i;
> + int i;
> int rc=0;
> struct edd_device *edev;
>
> @@ -760,21 +760,30 @@ edd_init(void)
> if (!edd_kset)
> return -ENOMEM;
>
> - for (i = 0; i < edd_num_devices() && !rc; i++) {
> + for (i = 0; i < edd_num_devices(); i++) {
> edev = kzalloc(sizeof (*edev), GFP_KERNEL);
> - if (!edev)
> - return -ENOMEM;
> + if (!edev) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> rc = edd_device_register(edev, i);
> if (rc) {
> kfree(edev);
> - break;
> + goto out;
> }
> edd_devices[i] = edev;
> }
>
> - if (rc)
> - kset_unregister(edd_kset);
> + return 0;
> +
> +out:
> + while (--i >= 0) {
> + edev = edd_devices[i];
> + if (edev)
This test is unneeded?
> + edd_device_unregister(edev);
> + }
> + kset_unregister(edd_kset);
> return rc;
> }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] edd: fix possible memory leak in edd_init() error path
2010-07-12 19:49 ` Andrew Morton
@ 2010-07-13 1:17 ` Axel Lin
2010-07-13 1:29 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Axel Lin @ 2010-07-13 1:17 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, Stephen Hemminger, Jens Axboe, David Teigland,
Pekka Enberg, Hans J. Koch
2010/7/13 Andrew Morton <akpm@linux-foundation.org>:
> On Fri, 09 Jul 2010 18:50:07 +0800
> Axel Lin <axel.lin@gmail.com> wrote:
>
>> The error may happen at any iteration of the for loop,
>> this patch properly unregisters already registed edd_devices in error path.
>>
>> Signed-off-by: Axel Lin <axel.lin@gmail.com>
>> ---
>> drivers/firmware/edd.c | 23 ++++++++++++++++-------
>> 1 files changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
>> index 110e24e..b75082c 100644
>> --- a/drivers/firmware/edd.c
>> +++ b/drivers/firmware/edd.c
>> @@ -744,7 +744,7 @@ static inline int edd_num_devices(void)
>> static int __init
>> edd_init(void)
>> {
>> - unsigned int i;
>> + int i;
>> int rc=0;
>> struct edd_device *edev;
>>
>> @@ -760,21 +760,30 @@ edd_init(void)
>> if (!edd_kset)
>> return -ENOMEM;
>>
>> - for (i = 0; i < edd_num_devices() && !rc; i++) {
>> + for (i = 0; i < edd_num_devices(); i++) {
>> edev = kzalloc(sizeof (*edev), GFP_KERNEL);
>> - if (!edev)
>> - return -ENOMEM;
>> + if (!edev) {
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>>
>> rc = edd_device_register(edev, i);
>> if (rc) {
>> kfree(edev);
>> - break;
>> + goto out;
>> }
>> edd_devices[i] = edev;
>> }
>>
>> - if (rc)
>> - kset_unregister(edd_kset);
>> + return 0;
>> +
>> +out:
>> + while (--i >= 0) {
>> + edev = edd_devices[i];
>> + if (edev)
>
> This test is unneeded?
right. a revised version is on the way.
Regards,
Axel
>
>> + edd_device_unregister(edev);
>> + }
>> + kset_unregister(edd_kset);
>> return rc;
>> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] edd: fix possible memory leak in edd_init() error path
2010-07-13 1:17 ` Axel Lin
@ 2010-07-13 1:29 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2010-07-13 1:29 UTC (permalink / raw)
To: axel.lin
Cc: linux-kernel, Stephen Hemminger, Jens Axboe, David Teigland,
Pekka Enberg, Hans J. Koch
On Tue, 13 Jul 2010 09:17:58 +0800 Axel Lin <axel.lin@gmail.com> wrote:
> >> +
> >> +out:
> >> + __ __ while (--i >= 0) {
> >> + __ __ __ __ __ __ edev = edd_devices[i];
> >> + __ __ __ __ __ __ if (edev)
> >
> > This test is unneeded?
>
> right. a revised version is on the way.
I already did the below, which you were cc'ed on?
--- a/drivers/firmware/edd.c~edd-fix-possible-memory-leak-in-edd_init-error-path-fix
+++ a/drivers/firmware/edd.c
@@ -778,11 +778,8 @@ edd_init(void)
return 0;
out:
- while (--i >= 0) {
- edev = edd_devices[i];
- if (edev)
- edd_device_unregister(edev);
- }
+ while (--i >= 0)
+ edd_device_unregister(edd_devices[i]);
kset_unregister(edd_kset);
return rc;
}
_
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-07-13 1:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-09 10:50 [PATCH] edd: fix possible memory leak in edd_init() error path Axel Lin
2010-07-11 9:01 ` Pekka Enberg
2010-07-12 19:49 ` Andrew Morton
2010-07-13 1:17 ` Axel Lin
2010-07-13 1:29 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox