[PATCH] staging: android: fix mem leaks in __persistent_ram_init()

[Colin Cross]

On Wed, Apr 11, 2012 at 1:10 PM, Jesper Juhl <jj at chaosbits.net> wrote:
> If, in __persistent_ram_init(), the call to
> persistent_ram_buffer_init() fails or the call to
> persistent_ram_init_ecc() fails then we fail to free the memory we
> allocated to 'prz' with kzalloc() - thus leaking it.
>
> To prevent the leaks I consolidated all error exits from the function
> at a 'err:' label at the end and made all error cases jump to that
> label where we can then make sure we always free 'prz'. This is safe
> since all the situations where the code bails out happen before 'prz'
> has been stored anywhere and although we'll do a redundant kfree(NULL)
> call in the case of kzalloc() itself failing that's OK since kfree()
> deals gracefully with NULL pointers and I felt it was more important
> to keep all error exits at a single location than to avoid that one
> harmless/redundant kfree() on a error path.
>
> Signed-off-by: Jesper Juhl <jj at chaosbits.net>
> ---
>  drivers/staging/android/persistent_ram.c |   11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/android/persistent_ram.c b/drivers/staging/android/persistent_ram.c
> index e08f257..8d8c1e3 100644
> --- a/drivers/staging/android/persistent_ram.c
> +++ b/drivers/staging/android/persistent_ram.c
> @@ -399,12 +399,12 @@ static  __init
>  struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
>  {
>        struct persistent_ram_zone *prz;
> -       int ret;
> +       int ret = -ENOMEM;
>
>        prz = kzalloc(sizeof(struct persistent_ram_zone), GFP_KERNEL);
>        if (!prz) {
>                pr_err("persistent_ram: failed to allocate persistent ram zone\n");
> -               return ERR_PTR(-ENOMEM);
> +               goto err;
>        }
>
>        INIT_LIST_HEAD(&prz->node);
> @@ -412,13 +412,13 @@ struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
>        ret = persistent_ram_buffer_init(dev_name(dev), prz);
This adds virtual mappings, which should get unmapped on error.

>        if (ret) {
>                pr_err("persistent_ram: failed to initialize buffer\n");
> -               return ERR_PTR(ret);
> +               goto err;
>        }
>
>        prz->ecc = ecc;
>        ret = persistent_ram_init_ecc(prz, prz->buffer_size);
>        if (ret)
> -               return ERR_PTR(ret);
> +               goto err;
>
>        if (prz->buffer->sig == PERSISTENT_RAM_SIG) {
>                if (buffer_size(prz) > prz->buffer_size ||
> @@ -442,6 +442,9 @@ struct persistent_ram_zone *__persistent_ram_init(struct device *dev, bool ecc)
>        atomic_set(&prz->buffer->size, 0);
>
>        return prz;
> +err:
> +       kfree(prz);
> +       return ERR_PTR(ret);
>  }
>
>  struct persistent_ram_zone * __init
> --
> 1.7.10
>
>
> --
> Jesper Juhl <jj at 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.
>

Acked-by: Colin Cross <ccross at android.com>