linux-kernel - Re: [PATCH] staging: android: fix mem leaks in __persistent_ram

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMbhsRQiT7oFu=B1_+Ygy+kX1sSs6GijwBv1ArHUO=0BMMDOaQ@mail.gmail.com>
Date:	Wed, 11 Apr 2012 13:19:24 -0700
From:	Colin Cross <ccross@...roid.com>
To:	Jesper Juhl <jj@...osbits.net>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
	John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH] staging: android: fix mem leaks in __persistent_ram_init()

On Wed, Apr 11, 2012 at 1:10 PM, Jesper Juhl <jj@...osbits.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@...osbits.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@...osbits.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@...roid.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/