* Re: [PATCH] readelf: Always print INVALID SECTION if destshdr is NULL for relocation
@ 2014-12-04 13:44 Mark Wielaard
0 siblings, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2014-12-04 13:44 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 955 bytes --]
On Sat, 2014-11-29 at 15:03 +0100, Mark Wielaard wrote:
> 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.
> [...]
> +2014-11-28 Mark Wielaard <mjw@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.
I pushed this fixed version of the fix to master now.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] readelf: Always print INVALID SECTION if destshdr is NULL for relocation
@ 2014-11-29 14:03 Mark Wielaard
0 siblings, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2014-11-29 14:03 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 570 bytes --]
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
[-- Attachment #2: 0001-readelf-Don-t-trash-destshdr-for-STT_SECTION-in-hand.patch --]
[-- Type: text/plain, Size: 3507 bytes --]
>From 313799d9082964b3a391b3d50596b2562348db5f Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@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 <mjw@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 <mjw@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 <mjw@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] readelf: Always print INVALID SECTION if destshdr is NULL for relocation
@ 2014-11-28 21:24 Mark Wielaard
0 siblings, 0 replies; 3+ messages in thread
From: Mark Wielaard @ 2014-11-28 21:24 UTC (permalink / raw)
To: elfutils-devel
[-- Attachment #1: Type: text/plain, Size: 4832 bytes --]
We already checked this in all other cases except for the special case
of relocs in statically_linked executables. Found with afl.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
src/ChangeLog | 6 +++++
src/readelf.c | 77 +++++++++++++++++++++++++++++++++++++++++------------------
2 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/src/ChangeLog b/src/ChangeLog
index a6d18b5..072146a 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2014-11-28 Mark Wielaard <mjw@redhat.com>
+
+ * readelf.c (handle_relocs_rel): Print INVALID SECTION if destshdr
+ is NULL also for statically_linked relocations.
+ (handle_relocs_rela): Likewise.
+
2014-11-26 Mark Wielaard <mjw@redhat.com>
* readelf.c (print_debug_aranges_section): Cast Dwarf_Word length
diff --git a/src/readelf.c b/src/readelf.c
index cd15e4c..6060851 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -1857,17 +1857,32 @@ handle_relocs_rel (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr)
}
if (is_statically_linked > 0 && shdr->sh_link == 0)
- printf ("\
- %#0*" PRIx64 " %-20s %*s %s\n",
- class == ELFCLASS32 ? 10 : 18, rel->r_offset,
- ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
- /* Avoid the leading R_ which isn't carrying any
- information. */
- ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
- buf, sizeof (buf)) + 2
- : gettext ("<INVALID RELOC>"),
- class == ELFCLASS32 ? 10 : 18, "",
- elf_strptr (ebl->elf, shstrndx, destshdr->sh_name));
+ {
+ if (unlikely (destshdr == NULL))
+ printf (" %#0*" PRIx64 " %-20s %*s <%s %ld>\n",
+ class == ELFCLASS32 ? 10 : 18, rel->r_offset,
+ ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
+ /* Avoid the leading R_ which isn't carrying any
+ information. */
+ ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
+ buf, sizeof (buf)) + 2
+ : gettext ("<INVALID RELOC>"),
+ class == ELFCLASS32 ? 10 : 18, "",
+ gettext ("INVALID SECTION"),
+ (long int) (sym->st_shndx == SHN_XINDEX
+ ? xndx : sym->st_shndx));
+ else
+ printf (" %#0*" PRIx64 " %-20s %*s %s\n",
+ class == ELFCLASS32 ? 10 : 18, rel->r_offset,
+ ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
+ /* Avoid the leading R_ which isn't carrying any
+ information. */
+ ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
+ buf, sizeof (buf)) + 2
+ : gettext ("<INVALID RELOC>"),
+ class == ELFCLASS32 ? 10 : 18, "",
+ elf_strptr (ebl->elf, shstrndx, destshdr->sh_name));
+ }
else
printf (" %#0*" PRIx64 " %-20s <%s %ld>\n",
class == ELFCLASS32 ? 10 : 18, rel->r_offset,
@@ -2045,18 +2060,34 @@ handle_relocs_rela (Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr)
}
if (is_statically_linked > 0 && shdr->sh_link == 0)
- printf ("\
- %#0*" PRIx64 " %-15s %*s %#6" PRIx64 " %s\n",
- class == ELFCLASS32 ? 10 : 18, rel->r_offset,
- ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
- /* Avoid the leading R_ which isn't carrying any
- information. */
- ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
- buf, sizeof (buf)) + 2
- : gettext ("<INVALID RELOC>"),
- class == ELFCLASS32 ? 10 : 18, "",
- rel->r_addend,
- elf_strptr (ebl->elf, shstrndx, destshdr->sh_name));
+ {
+ if (unlikely (destshdr == NULL))
+ printf (" %#0*" PRIx64 " %-15s %*s %#6" PRIx64 " <%s %ld>\n",
+ class == ELFCLASS32 ? 10 : 18, rel->r_offset,
+ ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
+ /* Avoid the leading R_ which isn't carrying any
+ information. */
+ ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
+ buf, sizeof (buf)) + 2
+ : gettext ("<INVALID RELOC>"),
+ class == ELFCLASS32 ? 10 : 18, "",
+ rel->r_addend,
+ gettext ("INVALID SECTION"),
+ (long int) (sym->st_shndx == SHN_XINDEX
+ ? xndx : sym->st_shndx));
+ else
+ printf (" %#0*" PRIx64 " %-15s %*s %#6" PRIx64 " %s\n",
+ class == ELFCLASS32 ? 10 : 18, rel->r_offset,
+ ebl_reloc_type_check (ebl, GELF_R_TYPE (rel->r_info))
+ /* Avoid the leading R_ which isn't carrying any
+ information. */
+ ? ebl_reloc_type_name (ebl, GELF_R_TYPE (rel->r_info),
+ buf, sizeof (buf)) + 2
+ : gettext ("<INVALID RELOC>"),
+ class == ELFCLASS32 ? 10 : 18, "",
+ rel->r_addend,
+ elf_strptr (ebl->elf, shstrndx, destshdr->sh_name));
+ }
else
printf (" %#0*" PRIx64 " %-15s <%s %ld>\n",
class == ELFCLASS32 ? 10 : 18, rel->r_offset,
--
1.9.3
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-12-04 13:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-04 13:44 [PATCH] readelf: Always print INVALID SECTION if destshdr is NULL for relocation Mark Wielaard
-- strict thread matches above, loose matches on Subject: below --
2014-11-29 14:03 Mark Wielaard
2014-11-28 21:24 Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).