Skip to content

Fix potential memory leak in libelf#1337

Merged
gianlucaborello merged 1 commit intodevfrom
libelf-leak
Mar 21, 2019
Merged

Fix potential memory leak in libelf#1337
gianlucaborello merged 1 commit intodevfrom
libelf-leak

Conversation

@gianlucaborello
Copy link
Contributor

Some old versions of libelf (such as the one used in CentOS 6) are prone to
memory leaks when elf_strptr() is called, under some circumstances the
memory is not freed even upon correct invocation of elf_end().

Eventually we should ship our own updated version of libelf, but since
people are currently welcome to build without using our bundled
dependencies, it's worth fixing anyway.

The fix consists in opening the file via mmap rather than reading it via
allocated buffers, which is something I honestly thought libelf was already
doing by default, so it was my intention to begin with.

The fix went upstream in:

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.

valgrind memory leak:

==44437== 213,655 bytes in 2 blocks are definitely lost in loss record 3 of 3
==44437==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==44437==    by 0x403812: __libelf_set_rawdata_wrlock (elf_getdata.c:262)
==44437==    by 0x40355E: elf_strptr (elf_strptr.c:128)

Some old versions of libelf (such as the one used in CentOS 6) are prone to
memory leaks when elf_strptr() is called, under some circumstances the
memory is not freed even upon correct invocation of elf_end().

Eventually we should ship our own updated version of libelf, but since
people are currently welcome to build without using our bundled
dependencies, it's worth fixing anyway.

The fix consists in opening the file via mmap rather than reading it via
allocated buffers, which is something I honestly thought libelf was already
doing by default, so it was my intention to begin with.

The fix went upstream in:

```
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.
```

valgrind memory leak:

```
==44437== 213,655 bytes in 2 blocks are definitely lost in loss record 3 of 3
==44437==    at 0x483577F: malloc (vg_replace_malloc.c:299)
==44437==    by 0x403812: __libelf_set_rawdata_wrlock (elf_getdata.c:262)
==44437==    by 0x40355E: elf_strptr (elf_strptr.c:128)
```
@nathan-b
Copy link
Contributor

LGTM. Good catch!

@gianlucaborello
Copy link
Contributor Author

Thanks for the review.

@gianlucaborello gianlucaborello merged commit 49e814d into dev Mar 21, 2019
@gianlucaborello gianlucaborello deleted the libelf-leak branch March 21, 2019 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants