On Fri, Nov 28, 2014 at 10:24:46PM +0100, Mark Wielaard wrote: > We already checked this in all other cases except for the special case > of relocs in statically_linked executables. Found with afl.
Sorry, this patch is bogus. It works around the actual cause. The destshdr should not be NULL to begin with. We actually check that before processing the relocations. But when we see a STT_SECTION symbol relocation we reuse destshdr to lookup that section. The correct fix is to not trash destshdr in that case. Which the attached patch does. Cheers, Mark
>From 313799d9082964b3a391b3d50596b2562348db5f Mon Sep 17 00:00:00 2001 From: Mark Wielaard <m...@redhat.com> Date: Fri, 28 Nov 2014 22:22:16 +0100 Subject: [PATCH] readelf: Don't trash destshdr for STT_SECTION in handle_relocs_rel[a]. We might need the original destshdr for handling other relocations. Signed-off-by: Mark Wielaard <m...@redhat.com> --- src/ChangeLog | 7 +++++++ src/readelf.c | 34 ++++++++++++++++++++-------------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index a6d18b5..d3828d9 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,10 @@ +2014-11-28 Mark Wielaard <m...@redhat.com> + + * readelf.c (handle_relocs_rel): Don't reuse destshdr to store + section header of a relocation against a STT_SECTION symbol. Use + a new local variable secshdr. + (handle_relocs_rela): Likewise. + 2014-11-26 Mark Wielaard <m...@redhat.com> * readelf.c (print_debug_aranges_section): Cast Dwarf_Word length diff --git a/src/readelf.c b/src/readelf.c index cd15e4c..69ae5d0 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -1894,12 +1894,15 @@ handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr) elf_strptr (ebl->elf, symshdr->sh_link, sym->st_name)); else { - destshdr = gelf_getshdr (elf_getscn (ebl->elf, - sym->st_shndx == SHN_XINDEX - ? xndx : sym->st_shndx), - &destshdr_mem); - - if (unlikely (destshdr == NULL)) + /* This is a relocation against a STT_SECTION symbol. */ + GElf_Shdr secshdr_mem; + GElf_Shdr *secshdr; + secshdr = gelf_getshdr (elf_getscn (ebl->elf, + sym->st_shndx == SHN_XINDEX + ? xndx : sym->st_shndx), + &secshdr_mem); + + if (unlikely (secshdr == NULL)) printf (" %#0*" PRIx64 " %-20s <%s %ld>\n", class == ELFCLASS32 ? 10 : 18, rel->r_offset, ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info)) @@ -1921,7 +1924,7 @@ handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr) buf, sizeof (buf)) + 2 : gettext ("<INVALID RELOC>"), class == ELFCLASS32 ? 10 : 18, sym->st_value, - elf_strptr (ebl->elf, shstrndx, destshdr->sh_name)); + elf_strptr (ebl->elf, shstrndx, secshdr->sh_name)); } } } @@ -2085,12 +2088,15 @@ handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr) elf_strptr (ebl->elf, symshdr->sh_link, sym->st_name)); else { - destshdr = gelf_getshdr (elf_getscn (ebl->elf, - sym->st_shndx == SHN_XINDEX - ? xndx : sym->st_shndx), - &destshdr_mem); - - if (unlikely (destshdr == NULL)) + /* This is a relocation against a STT_SECTION symbol. */ + GElf_Shdr secshdr_mem; + GElf_Shdr *secshdr; + secshdr = gelf_getshdr (elf_getscn (ebl->elf, + sym->st_shndx == SHN_XINDEX + ? xndx : sym->st_shndx), + &secshdr_mem); + + if (unlikely (secshdr == NULL)) printf (" %#0*" PRIx64 " %-15s <%s %ld>\n", class == ELFCLASS32 ? 10 : 18, rel->r_offset, ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info)) @@ -2114,7 +2120,7 @@ handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr) : gettext ("<INVALID RELOC>"), class == ELFCLASS32 ? 10 : 18, sym->st_value, rel->r_addend, - elf_strptr (ebl->elf, shstrndx, destshdr->sh_name)); + elf_strptr (ebl->elf, shstrndx, secshdr->sh_name)); } } } -- 1.9.3