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 <maenn...@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
