[PATCH] libelf: Fix memory leak in elf_compress for mmapped ELF files.

[PATCH] libelf: Fix memory leak in elf_compress for mmapped ELF files.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 4416 bytes --]

The testcase added to run-strip-reloc.sh for strip-compressed.o showed
a memory leak when ran under valgrind (configure --enable-valgrind).

For a mmapped ELF file when existing section data was compressed
elf_end would fail to release the new compressed data buffer assigned
to rawdata_base. For non-mapped files rawdata_base is always freed.
For decompressed data rawdata_base is released together with zdata_base.

Use the Elf_Scn flag ELF_T_MALLOCED to track whether rawdata_base
points to malloced memory and free it in elf_end even for mmapped
ELF files.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libelf/ChangeLog      |  8 ++++++++
 libelf/elf_compress.c |  7 +++++--
 libelf/elf_end.c      |  5 +++--
 libelf/libelfP.h      | 10 ++++++++--
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 3df8970..7799144 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,11 @@
+2016-08-07  Mark Wielaard  <mjw@redhat.com>
+
+	* elf_compress.c (__libelf_reset_rawdata): Check scn->flags and
+	free rawdata_base when malloced. Set ELF_F_MALLOCED for scn->flags.
+	* elf_end.c (elf_end): Check scn->flags and free rawdata_base if
+	malloced.
+	* libelfP.h (struct Elf_Scn): Document flags ELF_F_MALLOCED usage.
+
 2016-07-06  Mark Wielaard  <mjw@redhat.com>
 
 	* elf-knowledge.h (SH_FLAGS_COMBINE): Removed.
diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index 10574ea..3aebe82 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -1,5 +1,5 @@
 /* Compress or decompress a section.
-   Copyright (C) 2015 Red Hat, Inc.
+   Copyright (C) 2015, 2016 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -295,6 +295,7 @@ __libelf_decompress_elf (Elf_Scn *scn, size_t *size_out, size_t *addralign)
   return buf_out;
 }
 
+/* Assumes buf is a malloced buffer.  */
 void
 internal_function
 __libelf_reset_rawdata (Elf_Scn *scn, void *buf, size_t size, size_t align,
@@ -314,10 +315,12 @@ __libelf_reset_rawdata (Elf_Scn *scn, void *buf, size_t size, size_t align,
     free (scn->data_base);
   scn->data_base = NULL;
   if (scn->elf->map_address == NULL
-      || scn->rawdata_base == scn->zdata_base)
+      || scn->rawdata_base == scn->zdata_base
+      || (scn->flags & ELF_F_MALLOCED) != 0)
     free (scn->rawdata_base);
 
   scn->rawdata_base = buf;
+  scn->flags |= ELF_F_MALLOCED;
 }
 
 int
diff --git a/libelf/elf_end.c b/libelf/elf_end.c
index fde17b5..160f0b8 100644
--- a/libelf/elf_end.c
+++ b/libelf/elf_end.c
@@ -1,5 +1,5 @@
 /* Free resources associated with Elf descriptor.
-   Copyright (C) 1998,1999,2000,2001,2002,2004,2005,2007,2015 Red Hat, Inc.
+   Copyright (C) 1998,1999,2000,2001,2002,2004,2005,2007,2015,2016 Red Hat, Inc.
    This file is part of elfutils.
    Written by Ulrich Drepper <drepper@redhat.com>, 1998.
 
@@ -166,7 +166,8 @@ elf_end (Elf *elf)
 		/* The section data is allocated if we couldn't mmap
 		   the file.  Or if we had to decompress.  */
 		if (elf->map_address == NULL
-		    || scn->rawdata_base == scn->zdata_base)
+		    || scn->rawdata_base == scn->zdata_base
+		    || (scn->flags & ELF_F_MALLOCED) != 0)
 		  free (scn->rawdata_base);
 
 		/* Free the list of data buffers for the section.
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 57ccbce..4459982 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -1,5 +1,5 @@
 /* Internal interfaces for libelf.
-   Copyright (C) 1998-2010, 2015 Red Hat, Inc.
+   Copyright (C) 1998-2010, 2015, 2016 Red Hat, Inc.
    This file is part of elfutils.
    Contributed by Ulrich Drepper <drepper@redhat.com>, 1998.
 
@@ -233,7 +233,13 @@ struct Elf_Scn
   } shdr;
 
   unsigned int shdr_flags;	/* Section header modified?  */
-  unsigned int flags;		/* Section changed in size?  */
+  unsigned int flags;		/* Section changed in size?
+				   ELF_F_MALLOCED for a Elf_Data_Chunk
+				   dummy_scn means the rawchunks
+				   data.d.d_buf was malloced. For normal
+				   sections it means rawdata_base was
+				   malloced (by elf_compress) even if
+				   the Elf was mmapped.  */
 
   char *rawdata_base;		/* The unmodified data of the section.  */
   char *data_base;		/* The converted data of the section.  */
-- 
2.7.4

Re: [PATCH] libelf: Fix memory leak in elf_compress for mmapped ELF files.

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Sun, Aug 07, 2016 at 10:38:58PM +0200, Mark Wielaard wrote:
> The testcase added to run-strip-reloc.sh for strip-compressed.o showed
> a memory leak when ran under valgrind (configure --enable-valgrind).
> 
> For a mmapped ELF file when existing section data was compressed
> elf_end would fail to release the new compressed data buffer assigned
> to rawdata_base. For non-mapped files rawdata_base is always freed.
> For decompressed data rawdata_base is released together with zdata_base.
> 
> Use the Elf_Scn flag ELF_T_MALLOCED to track whether rawdata_base
> points to malloced memory and free it in elf_end even for mmapped
> ELF files.

I pushed this to master.

Cheers,

Mark