[PATCH] libelf: Don't leak memory on failure paths in elf_updatefile.
by Mark Wielaard
When something goes wrong during the update make sure to always free any
temporary allocated memory (shdr_data and/or scns).
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libelf/ChangeLog | 5 +++++
libelf/elf32_updatefile.c | 29 +++++++++++++++++------------
2 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 2d24007..52004f7 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
+ * elf32_updatefile.c (updatefile): Always free shdr_data and scns
+ when allocated on failure paths.
+
2015-06-16 Mark Wielaard <mjw(a)redhat.com>
* elf_update.c (write_file): Always also use ftruncate before
diff --git a/libelf/elf32_updatefile.c b/libelf/elf32_updatefile.c
index 913c3e4..832f852 100644
--- a/libelf/elf32_updatefile.c
+++ b/libelf/elf32_updatefile.c
@@ -655,16 +655,18 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
#endif
ElfW2(LIBELFBITS,Shdr) *shdr_data;
+ ElfW2(LIBELFBITS,Shdr) *shdr_data_mem = NULL;
if (change_bo || elf->state.ELFW(elf,LIBELFBITS).shdr == NULL
|| (elf->flags & ELF_F_DIRTY))
{
- shdr_data = (ElfW2(LIBELFBITS,Shdr) *)
+ shdr_data_mem = (ElfW2(LIBELFBITS,Shdr) *)
malloc (shnum * sizeof (ElfW2(LIBELFBITS,Shdr)));
- if (unlikely (shdr_data == NULL))
+ if (unlikely (shdr_data_mem == NULL))
{
__libelf_seterrno (ELF_E_NOMEM);
return -1;
}
+ shdr_data = shdr_data_mem;
}
else
shdr_data = elf->state.ELFW(elf,LIBELFBITS).shdr;
@@ -675,6 +677,7 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
Elf_Scn **scns = (Elf_Scn **) malloc (shnum * sizeof (Elf_Scn *));
if (unlikely (scns == NULL))
{
+ free (shdr_data_mem);
__libelf_seterrno (ELF_E_NOMEM);
return -1;
}
@@ -712,7 +715,12 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
(scn_start + dl->data.d.d_off)
- last_offset, fillbuf,
&filled) != 0))
- return 1;
+ {
+ fail_free:
+ free (shdr_data_mem);
+ free (scns);
+ return 1;
+ }
}
if ((scn->flags | dl->flags | elf->flags) & ELF_F_DIRTY)
@@ -744,7 +752,7 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
if (unlikely (buf == NULL))
{
__libelf_seterrno (ELF_E_NOMEM);
- return 1;
+ goto fail_free;
}
}
@@ -761,7 +769,7 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
free (buf);
__libelf_seterrno (ELF_E_WRITE_ERROR);
- return 1;
+ goto fail_free;
}
if (buf != dl->data.d.d_buf && buf != tmpbuf)
@@ -786,7 +794,7 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
if (unlikely (fill (elf->fildes, last_offset,
scn_start - last_offset, fillbuf,
&filled) != 0))
- return 1;
+ goto fail_free;
}
last_offset = scn_start + shdr->sh_size;
@@ -814,7 +822,7 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
&& unlikely (fill (elf->fildes, last_offset,
shdr_offset - last_offset,
fillbuf, &filled) != 0))
- return 1;
+ goto fail_free;
/* Write out the section header table. */
if (shdr_flags & ELF_F_DIRTY
@@ -824,13 +832,10 @@ __elfw2(LIBELFBITS,updatefile) (Elf *elf, int change_bo, size_t shnum)
!= sizeof (ElfW2(LIBELFBITS,Shdr)) * shnum))
{
__libelf_seterrno (ELF_E_WRITE_ERROR);
- return 1;
+ goto fail_free;
}
- if (change_bo || elf->state.ELFW(elf,LIBELFBITS).shdr == NULL
- || (elf->flags & ELF_F_DIRTY))
- free (shdr_data);
-
+ free (shdr_data_mem);
free (scns);
}
--
1.8.3.1
8 years, 10 months
[PATCH] elflint: Only check the PT_TLS phdr if it actually exists.
by Mark Wielaard
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
src/ChangeLog | 5 +++++
src/elflint.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 1b4f1d6..06d67f9 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * elflint.c (check_symtab): Only check the PT_TLS phdr if it actually
+ exists.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* nm.c (show_symbols): Check sizeof (GElf_SymX), not GElf_Sym to
known whether or not we stack allocated memory.
diff --git a/src/elflint.c b/src/elflint.c
index 1c6a55a..6b5a6a9 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -858,7 +858,7 @@ section [%2d] '%s': symbol %zu does not fit completely in referenced section [%2
section [%2d] '%s': symbol %zu: TLS symbol but no TLS program header entry\n"),
idx, section_name (ebl, idx), cnt);
}
- else
+ else if (phdr != NULL)
{
if (st_value
< destshdr->sh_offset - phdr->p_offset)
--
1.8.3.1
8 years, 10 months
[PATCH] libdw: Make sure the default page size is big enough to hold a Dwarf.
by Mark Wielaard
Just assert early that the page size isn't unreasonable small instead of
corrupting memory later.
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libdw/ChangeLog | 5 +++++
libdw/dwarf_begin_elf.c | 1 +
2 files changed, 6 insertions(+)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 8970ff6..753bcae 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,5 +1,10 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * dwarf_begin_elf.c (dwarf_begin_elf): Assert page size is big enough
+ to hold a Dwarf.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* dwarf_getpubnames.c (get_offsets): Always free mem on error.
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c
index 4e0d590..6d38946 100644
--- a/libdw/dwarf_begin_elf.c
+++ b/libdw/dwarf_begin_elf.c
@@ -365,6 +365,7 @@ dwarf_begin_elf (elf, cmd, scngrp)
/* Default memory allocation size. */
size_t mem_default_size = sysconf (_SC_PAGESIZE) - 4 * sizeof (void *);
+ assert (sizeof (struct Dwarf) < mem_default_size);
/* Allocate the data structure. */
Dwarf *result = (Dwarf *) calloc (1, sizeof (Dwarf) + mem_default_size);
--
1.8.3.1
8 years, 10 months
[PATCH] nm: Fix typo in size check to determine whether we stack allocated memory.
by Mark Wielaard
We allocate GElf_SymX entries, which are larger than plain GElf_Sym structs.
The check to see whether we could use stack allocation used the correct
sizeof (GElf_SymX), but the check to see if we needed to free was using
the incorrect sizeof (GElf_Sym). Which could cause us to leak memory.
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
src/ChangeLog | 5 +++++
src/nm.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index 7c47079..1b4f1d6 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * nm.c (show_symbols): Check sizeof (GElf_SymX), not GElf_Sym to
+ known whether or not we stack allocated memory.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* strings.c (readelf): Use "<unknown>" if we cannot retrieve section
name.
diff --git a/src/nm.c b/src/nm.c
index 8d19715..d89bdc7 100644
--- a/src/nm.c
+++ b/src/nm.c
@@ -1383,7 +1383,7 @@ show_symbols (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, Elf_Scn *xndxscn,
}
/* Free all memory. */
- if (nentries * sizeof (GElf_Sym) >= MAX_STACK_ALLOC)
+ if (nentries * sizeof (GElf_SymX) >= MAX_STACK_ALLOC)
free (sym_mem);
obstack_free (&whereob, NULL);
--
1.8.3.1
8 years, 10 months
[PATCH] strings: Handle failure of getting section name on bogus section data.
by Mark Wielaard
If there is something wrong with getting the section data it is likely
we won't be able to get the actual section name because the file is
somehow corrupted. Try to get the name, but handle failure gracefully.
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
src/ChangeLog | 5 +++++
src/strings.c | 8 ++++++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index b548851..7c47079 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,5 +1,10 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * strings.c (readelf): Use "<unknown>" if we cannot retrieve section
+ name.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* readelf.c (handle_gnu_hash): Free lengths on invalid_data.
2015-06-09 Mark Wielaard <mjw(a)redhat.com>
diff --git a/src/strings.c b/src/strings.c
index b2bce7b..88a3c2f 100644
--- a/src/strings.c
+++ b/src/strings.c
@@ -730,10 +730,14 @@ read_elf (Elf *elf, int fd, const char *fname, off64_t fdlen)
|| fdlen - shdr->sh_offset < shdr->sh_size)
{
size_t strndx = 0;
- elf_getshdrstrndx (elf, &strndx);
+ const char *sname;
+ if (unlikely (elf_getshdrstrndx (elf, &strndx) < 0))
+ sname = "<unknown>";
+ else
+ sname = elf_strptr (elf, strndx, shdr->sh_name) ?: "<unknown>";
error (0, 0,
gettext ("Skipping section %zd '%s' data outside file"),
- elf_ndxscn (scn), elf_strptr (elf, strndx, shdr->sh_name));
+ elf_ndxscn (scn), sname);
result = 1;
}
else
--
1.8.3.1
8 years, 10 months
[PATCH] libelf: Check symbol section header exists before use in nlist.
by Mark Wielaard
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libelf/ChangeLog | 4 ++++
libelf/nlist.c | 8 ++++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 52004f7..82540f6 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,5 +1,9 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * nlist.c (nlist): Check symscn shdr exists before use.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* elf32_updatefile.c (updatefile): Always free shdr_data and scns
when allocated on failure paths.
diff --git a/libelf/nlist.c b/libelf/nlist.c
index 89fd081..c72f420 100644
--- a/libelf/nlist.c
+++ b/libelf/nlist.c
@@ -1,5 +1,5 @@
/* Extract symbol list from binary.
- Copyright (C) 1998, 1999, 2000, 2001, 2002, 2005, 2007 Red Hat, Inc.
+ Copyright (C) 1998, 1999, 2000, 2001, 2002, 2005, 2007, 2015 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper(a)redhat.com>, 1998.
@@ -116,7 +116,11 @@ nlist (const char *filename, struct nlist *nl)
/* Re-get the section header in case we found only the dynamic symbol
table. */
if (scn == NULL)
- shdr = INTUSE(gelf_getshdr) (symscn, &shdr_mem);
+ {
+ shdr = INTUSE(gelf_getshdr) (symscn, &shdr_mem);
+ if (shdr == NULL)
+ goto fail_close;
+ }
/* SHDR->SH_LINK now contains the index of the string section. */
/* Get the data for the symbol section. */
--
1.8.3.1
8 years, 10 months
[PATCH] libdwfl: Check symbol section exists and has a sane entsize in find_symtab.
by Mark Wielaard
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libdwfl/ChangeLog | 5 +++++
libdwfl/dwfl_module_getdwarf.c | 3 ++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index a5253e2..ca98783 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
+ * dwfl_module_getdwarf.c (find_symtab): Check shdr is not NULL and
+ sh_entsize is not zero.
+
2015-06-06 Mark Wielaard <mjw(a)redhat.com>
* find-debuginfo.c (find_debuginfo_in_path): Always free localpath,
diff --git a/libdwfl/dwfl_module_getdwarf.c b/libdwfl/dwfl_module_getdwarf.c
index 908a9ee..dba9d66 100644
--- a/libdwfl/dwfl_module_getdwarf.c
+++ b/libdwfl/dwfl_module_getdwarf.c
@@ -1135,7 +1135,8 @@ find_symtab (Dwfl_Module *mod)
// Sanity check number of symbols.
GElf_Shdr shdr_mem, *shdr = gelf_getshdr (symscn, &shdr_mem);
- if (mod->syments > mod->symdata->d_size / shdr->sh_entsize
+ if (shdr == NULL || shdr->sh_entsize == 0
+ || mod->syments > mod->symdata->d_size / shdr->sh_entsize
|| (size_t) mod->first_global > mod->syments)
goto elferr;
--
1.8.3.1
8 years, 10 months
[PATCH] libdw: Not leak memory on error in dwarf_getpubnames.
by Mark Wielaard
When there is an error with the first entry we might already have allocated
the memory but not yet set cnt to 1. Just always free the memory on error.
free (NULL) is a nop anyway and doesn't matter on a failure path.
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libdw/ChangeLog | 4 ++++
libdw/dwarf_getpubnames.c | 1 +
2 files changed, 5 insertions(+)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 577de92..8970ff6 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,5 +1,9 @@
2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+ * dwarf_getpubnames.c (get_offsets): Always free mem on error.
+
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
* dwarf_getmacros.c (get_macinfo_table): Return NULL when
dwarf_formudata reports an error.
(get_table_for_offset): Likewise.
diff --git a/libdw/dwarf_getpubnames.c b/libdw/dwarf_getpubnames.c
index 19f4eae..41b2407 100644
--- a/libdw/dwarf_getpubnames.c
+++ b/libdw/dwarf_getpubnames.c
@@ -127,6 +127,7 @@ get_offsets (Dwarf *dbg)
if (mem == NULL || cnt == 0)
{
+ free (mem);
__libdw_seterrno (DWARF_E_NO_ENTRY);
return -1;
}
--
1.8.3.1
8 years, 10 months
[PATCH] libdw: Detect dwarf_formudata errors in dwarf_getmacros.
by Mark Wielaard
dwarf_formudata can return an error for bad DWARF. Don't ignore it.
Signed-off-by: Mark Wielaard <mjw(a)redhat.com>
---
libdw/ChangeLog | 6 ++++++
libdw/dwarf_getmacros.c | 6 ++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 487e34a..577de92 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,9 @@
+2015-06-18 Mark Wielaard <mjw(a)redhat.com>
+
+ * dwarf_getmacros.c (get_macinfo_table): Return NULL when
+ dwarf_formudata reports an error.
+ (get_table_for_offset): Likewise.
+
2015-06-08 Mark Wielaard <mjw(a)redhat.com>
* dwarf_getsrclines.c (read_srclines): Initialize dirarray early.
diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index 740368e..a326e58 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -125,7 +125,8 @@ get_macinfo_table (Dwarf *dbg, Dwarf_Word macoff, Dwarf_Die *cudie)
= INTUSE(dwarf_attr) (cudie, DW_AT_stmt_list, &attr_mem);
Dwarf_Off line_offset = (Dwarf_Off) -1;
if (attr != NULL)
- INTUSE(dwarf_formudata) (attr, &line_offset);
+ if (unlikely (INTUSE(dwarf_formudata) (attr, &line_offset) != 0))
+ return NULL;
Dwarf_Macro_Op_Table *table = libdw_alloc (dbg, Dwarf_Macro_Op_Table,
macinfo_data_size, 1);
@@ -178,7 +179,8 @@ get_table_for_offset (Dwarf *dbg, Dwarf_Word macoff,
Dwarf_Attribute attr_mem, *attr
= INTUSE(dwarf_attr) (cudie, DW_AT_stmt_list, &attr_mem);
if (attr != NULL)
- INTUSE(dwarf_formudata) (attr, &line_offset);
+ if (unlikely (INTUSE(dwarf_formudata) (attr, &line_offset) != 0))
+ return NULL;
}
/* """The macinfo entry types defined in this standard may, but
--
1.8.3.1
8 years, 10 months
Re: sizeof - kernel modules
by Josh Stone
CC elfutils-devel, this appears to be a regression. I'm top-posting my
own findings, but I'll leave the kernel report below...
With elfutils-0.162, it appears that "strip" doesn't reduce the file
size when operating in-place (without -o). This isn't particular to
funny kernel ELF -- I reproduced it on a local stap binary.
My full stap is 31MB. With strip -o, I get a new 2.5MB file. With
strip in-place, stap remains 31MB even though the ELF section data does
indicate all the DWARF sections are gone.
I bisected it to commit 77482c4bf63a.
https://git.fedorahosted.org/cgit/elfutils.git/commit/?id=77482c4bf63a921...
if (elf->parent == NULL
&& (elf->maximum_size == ~((size_t) 0)
|| (size_t) size > elf->maximum_size)
- && unlikely (ftruncate (elf->fildes, size) != 0))
+ && unlikely (posix_fallocate (elf->fildes, 0, size) != 0))
In elf_begin() w/ ELF_C_RDWR, we get maximum_size == ~0, but now this
posix_fallocate can only increase the file size. The later part of
write_file() to reduce the size is only called for maximum_size != ~0.
On 06/15/2015 11:32 AM, Josh Boyer wrote:
> On Mon, Jun 15, 2015 at 1:45 PM, drago01 <drago01(a)gmail.com> wrote:
>> On Mon, Jun 15, 2015 at 7:38 PM, poma <pomidorabelisima(a)gmail.com> wrote:
>>>
>>> # du -hs /lib/modules/4.0.5-300.fc22.i686/ | awk '{print $1}'
>>> 42M
>>>
>>> kernel-modules-4.0.5-300.fc22.i686.rpm ... 17M
>>>
>>> http://dl.fedoraproject.org/pub/fedora/linux/updates/testing/22/i386/k/
>>> ~~~~~~~~~~~ ~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~ ~~~~~~~ ~~~~~~ ~~~~~ ~~~~ ~~~
>>>
>>> # du -hs /lib/modules/4.1.0-0.rc7.git0.1.fc23.x86_64/ | awk '{print $1}'
>>> 53M
>>>
>>> kernel-modules-4.1.0-0.rc7.git0.1.fc23.x86_64.rpm ... 18M
>>>
>>> https://kojipkgs.fedoraproject.org/packages/kernel/4.1.0/0.rc7.git0.1.fc2...
>>> ~~~~~~~~~~~ ~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~ ~~~~~~~ ~~~~~~ ~~~~~ ~~~~ ~~~ ~~ ~ ~~ ~~~
>>>
>>> # du -hs /lib/modules/4.1.0-0.rc7.git1.1.fc23.x86_64/ | awk '{print $1}'
>>> 439M
>>>
>>> kernel-modules-4.1.0-0.rc7.git1.1.fc23.x86_64.rpm ... 218M
>>>
>>> http://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/x86_64/o...
>>> ~~~~~~~~~~~ ~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~ ~~~~~~~ ~~~~~~ ~~~~~ ~~~~ ~~~ ~~ ~ ~~ ~~~ ~~~~
>>>
>>> # du -hs /lib/modules/4.1.0-0.rc8.git0.1.fc23.x86_64/ | awk '{print $1}'
>>> 438M
>>>
>>> kernel-modules-4.1.0-0.rc8.git0.1.fc23.x86_64.rpm ... 218M
>>>
>>> https://kojipkgs.fedoraproject.org/work/tasks/9420/10059420/
>>> ~~~~~~~~~~~ ~~~~~~~~~~ ~~~~~~~~~ ~~~~~~~~ ~~~~~~~ ~~~~~~ ~~~~~
>>>
>>>
>>> 218 ÷ 18 = 12.111111111
>>> 439 ÷ 53 = 8.283018868
>>>
>>>> = 4.1.0-0.rc7.git1.1 cca. 10 x sizeof
>>
>> Those are debug builds.
>
> They aren't actually.
>
> It looks like something in the buildroot stopped stripping the kernel
> modules between rc7.git0 and rc7.git1. Both elf-utils and binutils
> changed, so maybe something in there. The same SRPM build on F22 has
> the smaller size.
>
> I haven't looked into it more than that. If someone wants to poke at
> this and figure out what broke, that would be excellent. From a
> kernel package perspective, there isn't anything that we did that
> should cause this change in behavior that I'm aware of.
>
> josh
> _______________________________________________
> kernel mailing list
> kernel(a)lists.fedoraproject.org
> https://admin.fedoraproject.org/mailman/listinfo/kernel
>
8 years, 10 months