[PATCH] libelf: decompress: ensure zlib resource cleanup

[Mark Wielaard]

Hi Matthias,

On Sun, 2020-03-15 at 23:03 +0100, Matthias Maennich via Elfutils-devel wrote:
> __libelf_decompress would only cleanup zlib resources via inflateEnd()
> in case inflating was successful, but would leak memory if not. Fix this
> by calling inflateEnd() in the error case as well.
> 
> Fixes: 272018bba1f2 ("libelf: Add elf_compress and elf_compress_gnu.")
> Signed-off-by: Matthias Maennich <maennich@google.com>
> ---
>  libelf/elf_compress.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
> index 244467b5e3ae..beb1834bbbd7 100644
> --- a/libelf/elf_compress.c
> +++ b/libelf/elf_compress.c
> @@ -257,6 +257,7 @@ __libelf_decompress (void *buf_in, size_t size_in, size_t size_out)
>    if (unlikely (zrc != Z_OK) || unlikely (z.avail_out != 0))
>      {
>        free (buf_out);
> +      inflateEnd(&z);
>        __libelf_seterrno (ELF_E_DECOMPRESS_ERROR);
>        return NULL;
>      }

This looks correct at first sight, but...

Just before this hunk we do:

  if (likely (zrc == Z_OK))
    zrc = inflateEnd (&z);

So, zrc can be !Z_OK because the earlier inflateEnd() failed, which
might cause it to call inflateEnd() twice (which might be fine, I
dunno). Should we maybe ignore the error if inflateEnd() and just call
it unconditionally before (ignoring its return code)?

So, replace:
  if (... Z_OK) zrc = inflateEnd (&z);
with unconditionally ending the stream:
  (void)inflateEnd(&z);

What do you think?

Cheers,

Mark