edd: fix possible memory leak in edd_init() error path
in [Kernel]
|
Prev: Hacks allowing -rt to run on POWER7 / Powerpc.
Next: [patch v2.6 1/4] netfilter: xt_ipvs (netfilter matcher for IPVS)
From: Pekka Enberg on 11 Jul 2010 05:10 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(a)gmail.com> Looks good to me. Reviewed-by: Pekka Enberg <penberg(a)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; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 12 Jul 2010 15:50 On Fri, 09 Jul 2010 18:50:07 +0800 Axel Lin <axel.lin(a)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(a)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; > } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Axel Lin on 12 Jul 2010 21:20 2010/7/13 Andrew Morton <akpm(a)linux-foundation.org>: > On Fri, 09 Jul 2010 18:50:07 +0800 > Axel Lin <axel.lin(a)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(a)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; >> �} > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on 12 Jul 2010 21:30
On Tue, 13 Jul 2010 09:17:58 +0800 Axel Lin <axel.lin(a)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; } _ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |