Re: [PATCH] edd: fix possible memory leak in edd_init() error path

From: Andrew Morton
Date: Mon Jul 12 2010 - 15:49:41 EST


On Fri, 09 Jul 2010 18:50:07 +0800
Axel Lin <axel.lin@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/