This is the mail archive of the elfutils-devel@sourceware.org mailing list for the elfutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Malformed phdrs in separated debuginfo


On Wed, 2014-02-12 at 14:19 +0100, Florian Weimer wrote:
> On 02/12/2014 01:36 PM, Mark Wielaard wrote:
> > On Wed, 2014-02-12 at 13:14 +0100, Florian Weimer wrote:
> >> Here's the original issue I ran into: Separate debugging information,
> >> for example the file /usr/lib/debug/usr/lib64/libdl-2.18.so.debug in
> >> package glibc-debuginfo-2.18-12.fc20.x86_64, often contains garbage as
> >> the program interpreter:
> >>
> >> $ eu-readelf -a /usr/lib/debug/usr/lib64/libdl-2.18.so.debug | grep
> >> Requesting | xxd
> >> 0000000: 095b 5265 7175 6573 7469 6e67 2070 726f  .[Requesting pro
> >> 0000010: 6772 616d 2069 6e74 6572 7072 6574 6572  gram interpreter
> >> 0000020: 3a20 0608 bf14 5d0a                      : ....].
> >>
> >> Is this a known issue with debuginfo separation?  Should I report this
> >> somewhere?
> >
> > Aha. Yeah, the issue is that the phdr doesn't really make sense for a
> > separate debuginfo file. It isn't really a loadable ELF file. Only the
> > shdrs make sense, since the debuginfo sections are accessed through. In
> > theory [eu]-strip could just remove the phdrs, but some tools do expect
> > them to be there and we do sometimes use them to make sure the original
> > (stripped) ELF file and the debug file match (eu-unstrip will only
> > reconstruct the original ELF file, putting the debuginfo back in, if the
> > phdrs match up for example).
> >
> > If you look with eu-readelf -l you'll see the mapping form phdr segments
> > (which would be how an ELF file is mapped into memory) to the shdr
> > sections. e.g.
> >
> > Program Headers:
> >    Type           Offset   VirtAddr           PhysAddr           FileSiz
> > MemSiz   Flg Align
> >    PHDR           0x000040 0x00000033c8e00040 0x00000033c8e00040 0x0001f8
> > 0x0001f8 R E 0x8
> >    INTERP         0x001a50 0x00000033c8e01a50 0x00000033c8e01a50 0x00001c
> > 0x00001c R   0x10
> > 	[Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
> > [...]
> >   Section to Segment mapping:
> >    Segment Sections...
> >     00
> >     01      [RO: .interp]
> > [...]
> >
> > The problem in the .debug file is that the sections on disk that map to
> > loadable segments have been replaced with NOBITS. eu-readelf -S on the
> > debug file will give:
> >
> > [16] .interp              NOBITS       0000000000001a50 00000280
> > 0000001c  0 A      0   0 16
> >
> > But when eu-readelf tries to display the content of the PT_INTERP
> > segment it doesn't know that, so it tries to print the content anyway.
> >
> > Separate debuginfo files stretch the ELF concept a little. I'll have to
> > think about how to handle this nicely and/or how we can detect that the
> > phdrs aren't to be trusted in this case.
> 
> Is there a reliable way to recognize separate debugging information? 
> Then I could avoid extracting data from phdrs.

I looked a bit into this and I don't see any way to detect that the
phdrs are invalid, unless you already know this is a separate debuginfo
file and not a real/valid ELF file. This is indeed somewhat unfortunate.
The best I can do for eu-readelf is to mimic the behavior of trying to
display the PT_DYNAMIC segment. That is to check there is a matching elf
section at the given offset in the file with the right sh_type.
Unfortunately there is no unique SH type for the ".interp" section. So
we have to match against SHT_PROGBITS. This does seem to work in
practice though.

I wish there was something more robust, but I don't know of anything.

Cheers,

Mark
>From cb7b2d64b6fdbbb6f18ce07294b2315f60d843bc Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Mon, 24 Feb 2014 17:44:42 +0100
Subject: [PATCH] readelf: More sanity checks before trying to display interpreter string.

Check there is a SHT_PROGBITS section at the offset given by p_offsets for
a PT_INTERP segment before trying to display the interpreter string.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 src/ChangeLog |    6 ++++++
 src/readelf.c |   18 ++++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index ad3b2b1..80be466 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,9 @@
+2014-02-24  Mark Wielaard  <mjw@redhat.com>
+
+	* readelf (print_phdr): Check there is a SHT_PROGBITS section at the
+	offset given by p_offsets for a PT_INTERP segment before trying to
+	display the interpreter string.
+
 2014-02-07  Mark Wielaard  <mjw@redhat.com>
 
 	* readelf.c (print_phdr): Check phdr->p_filesz and make sure
diff --git a/src/readelf.c b/src/readelf.c
index fb95463..63675c6 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -1187,11 +1187,25 @@ print_phdr (Ebl *ebl, GElf_Ehdr *ehdr)
 
       if (phdr->p_type == PT_INTERP)
 	{
-	  /* We can show the user the name of the interpreter.  */
+	  /* If we are sure the file offset is valid then we can show
+	     the user the name of the interpreter.  We check whether
+	     there is a section at the file offset.  Normally there
+	     would be a section called ".interp".  But in separate
+	     .debug files it is a NOBITS section (and so doesn't match
+	     with gelf_offscn).  Which probably means the offset is
+	     not valid another reason could be because the ELF file
+	     just doesn't contain any section headers, in that case
+	     just play it safe and don't display anything.  */
+
+	  Elf_Scn *scn = gelf_offscn (ebl->elf, phdr->p_offset);
+	  GElf_Shdr shdr_mem;
+	  GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_mem);
+
 	  size_t maxsize;
 	  char *filedata = elf_rawfile (ebl->elf, &maxsize);
 
-	  if (filedata != NULL && phdr->p_offset < maxsize
+	  if (shdr != NULL && shdr->sh_type == SHT_PROGBITS
+	      && filedata != NULL && phdr->p_offset < maxsize
 	      && phdr->p_filesz <= maxsize - phdr->p_offset
 	      && memchr (filedata + phdr->p_offset, '\0',
 			 phdr->p_filesz) != NULL)
-- 
1.7.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]