vdso handling

vdso handling

[Metzger, Markus T]

Hello,

My name is Markus.  I work for Intel on GDB in the area of
hardware-supported execution recording.

I noticed that the BFD created for the VDSO (system-provided in-memory
DSO) does not contain any BFD sections.  Is this intentional?  Or has
there just been no need for them?

If it just hasn't been done, yet, how would I best approach this?


Here's the problem I am trying to solve with this.  May as well be
that I'm on the wrong track...

When using the btrace record target, GDB only allows access to
read-only memory during replay.  The btrace record target does not
trace data so read-write memory corresponds to the end of the trace,
not the current replay position.

The implementation uses the same check that is also used for
'trust-readonly-sections", i.e.

	    section = target_section_by_addr (ops, offset);
	    if (section != NULL)
	      {
		/* Check if the section we found is readonly.  */
		if ((bfd_get_section_flags (section->the_bfd_section->owner,
					    section->the_bfd_section)
		     & SEC_READONLY) != 0)

For the vdso, there is no target section, so the check fails.  This
prevents GDB from disassembling vdso instructions during replay.


The vdso is processed in symbol_file_add_from_memory at
gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
for the vdso and then processes it.  If there were BFD sections, the
following small patch should add the respective target sections to GDB.

diff --git a/gdb/symfile-mem.c b/gdb/symfile-mem.c
index e3230de..cf4da38 100644
--- a/gdb/symfile-mem.c
+++ b/gdb/symfile-mem.c
@@ -91,6 +91,7 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
   struct section_addr_info *sai;
   unsigned int i;
   struct cleanup *cleanup;
+  struct target_section *sections, *sections_end, *tsec;

   if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
     error (_("add-symbol-file-from-memory not supported for this target"));
@@ -113,6 +114,22 @@ symbol_file_add_from_memory (struct bfd *templ, CORE_ADDR addr, char *name,
     error (_("Got object file from memory but can't read symbols: %s."),
	   bfd_errmsg (bfd_get_error ()));

+  /* Add target sections for this bfd.  */
+  sections = NULL;
+  sections_end = NULL;
+  if (build_section_table (nbfd, &sections, &sections_end))
+    error (_("Failed to build section table"));
+
+  /* Adjust the target section addresses by the load address.  */
+  for (tsec = sections; tsec != sections_end; ++tsec)
+    {
+      tsec->addr += loadbase;
+      tsec->endaddr += loadbase;
+    }
+
+  add_target_sections (&nbfd, sections, sections_end);
+  xfree (sections);
+
   sai = alloc_section_addr_info (bfd_count_sections (nbfd));
   make_cleanup (xfree, sai);
   i = 0;


This should allow me to pass the above read-only section check and
allow GDB to disassemble vdso instructions.

thanks,
markus.


Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Alan Modra]

On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
> I noticed that the BFD created for the VDSO (system-provided in-memory
> DSO) does not contain any BFD sections.  Is this intentional?  Or has
> there just been no need for them?
[snip]
> The vdso is processed in symbol_file_add_from_memory at
> gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
> for the vdso and then processes it.

The underlying cause is that you're trying to debug an ELF binary that
only contains the execution view.  The linking view (of which the
sections are a part) is not loaded, so bfd_from_remote_memory does not
have this information.  See elfcode.h bfd_from_remote_memory.

You can see similar breakage of gdb and binutils if you zap e_shoff,
e_shnum, and e_shstrndx of your favourite hello world program.

I suppose one way to provide something that gdb and other tools expect
would be to treat the vdso like a core file, and create fake sections
corresponding to the program headers.  I'm not really keen on the idea
though, since I know that will open up a can of worms.

Can't you point gdb at a file image for the vdso?

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

On Wed 12 Mar 2014 17:47:02 Alan Modra wrote:
> On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
> > I noticed that the BFD created for the VDSO (system-provided in-memory
> > DSO) does not contain any BFD sections.  Is this intentional?  Or has
> > there just been no need for them?
> 
> [snip]
> 
> > The vdso is processed in symbol_file_add_from_memory at
> > gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
> > for the vdso and then processes it.
> 
> The underlying cause is that you're trying to debug an ELF binary that
> only contains the execution view.  The linking view (of which the
> sections are a part) is not loaded, so bfd_from_remote_memory does not
> have this information.  See elfcode.h bfd_from_remote_memory.
> 
> You can see similar breakage of gdb and binutils if you zap e_shoff,
> e_shnum, and e_shstrndx of your favourite hello world program.
> 
> I suppose one way to provide something that gdb and other tools expect
> would be to treat the vdso like a core file, and create fake sections
> corresponding to the program headers.  I'm not really keen on the idea
> though, since I know that will open up a can of worms.
> 
> Can't you point gdb at a file image for the vdso?

i don't think distros generally ship it ?  the kernel doesn't install it by 
default (e.g. into /lib/modules/$(uname -r)/vdso/).
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: vdso handling

[Doug Evans]

On Wed, Mar 12, 2014 at 12:17 AM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 10, 2014 at 01:04:33PM +0000, Metzger, Markus T wrote:
>> I noticed that the BFD created for the VDSO (system-provided in-memory
>> DSO) does not contain any BFD sections.  Is this intentional?  Or has
>> there just been no need for them?
> [snip]
>> The vdso is processed in symbol_file_add_from_memory at
>> gdb/symfile-mem.c:84.  It calls bfd_from_remote_memory to create a BFD
>> for the vdso and then processes it.
>
> The underlying cause is that you're trying to debug an ELF binary that
> only contains the execution view.  The linking view (of which the
> sections are a part) is not loaded, so bfd_from_remote_memory does not
> have this information.  See elfcode.h bfd_from_remote_memory.
>
> You can see similar breakage of gdb and binutils if you zap e_shoff,
> e_shnum, and e_shstrndx of your favourite hello world program.
>
> I suppose one way to provide something that gdb and other tools expect
> would be to treat the vdso like a core file, and create fake sections
> corresponding to the program headers.  I'm not really keen on the idea
> though, since I know that will open up a can of worms.

I think a case can be made that gdb should be able to use the
"execution view" of the program here.
As for how to achieve that ... "Discuss." :-)

Re: vdso handling

[Cary Coutant]

> I think a case can be made that gdb should be able to use the
> "execution view" of the program here.
> As for how to achieve that ... "Discuss." :-)

Add a PT_DEBUG program header entry? The PT_DEBUG segment would need
to have a small header that allows the debugger to find .debug_abbrev,
.debug_info, etc. (i.e., a mini section table). Or, just add
individual program header entries for each of the standard debug
sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.

-cary

Re: vdso handling

[Alan Modra]

On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > I think a case can be made that gdb should be able to use the
> > "execution view" of the program here.
> > As for how to achieve that ... "Discuss." :-)
> 
> Add a PT_DEBUG program header entry? The PT_DEBUG segment would need
> to have a small header that allows the debugger to find .debug_abbrev,
> .debug_info, etc. (i.e., a mini section table). Or, just add
> individual program header entries for each of the standard debug
> sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.

Debug sections are not normally loaded.  For that reason I don't think
it makes any sense to specify program headers for them.  It wouldn't
help in the vdso case anyway, since the problem there is that you only
have the loaded part of the original ELF file.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Thursday, March 13, 2014 2:02 AM
> To: Cary Coutant
> Cc: Doug Evans; Metzger, Markus T; gdb@sourceware.org;
> binutils@sourceware.org
> Subject: Re: vdso handling
> 
> On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > > I think a case can be made that gdb should be able to use the
> > > "execution view" of the program here.
> > > As for how to achieve that ... "Discuss." :-)
> >
> > Add a PT_DEBUG program header entry? The PT_DEBUG segment would
> need
> > to have a small header that allows the debugger to find .debug_abbrev,
> > .debug_info, etc. (i.e., a mini section table). Or, just add
> > individual program header entries for each of the standard debug
> > sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
> 
> Debug sections are not normally loaded.  For that reason I don't think
> it makes any sense to specify program headers for them.  It wouldn't
> help in the vdso case anyway, since the problem there is that you only
> have the loaded part of the original ELF file.

The vdso contains a section table, as well.  When I hack
bfd_from_remote_memory to create BFD sections from them similar
to what elf_object_p does, I get the target sections that I wanted in GDB.
The patch is rather big, though, and duplicating a lot of elf_object_p's code.

I have not tried generating fake sections from segments, yet.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Metzger, Markus T
> Sent: Thursday, March 13, 2014 9:24 AM


> > On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
> > > > I think a case can be made that gdb should be able to use the
> > > > "execution view" of the program here.
> > > > As for how to achieve that ... "Discuss." :-)
> > >
> > > Add a PT_DEBUG program header entry? The PT_DEBUG segment would
> > need
> > > to have a small header that allows the debugger to find .debug_abbrev,
> > > .debug_info, etc. (i.e., a mini section table). Or, just add
> > > individual program header entries for each of the standard debug
> > > sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
> >
> > Debug sections are not normally loaded.  For that reason I don't think
> > it makes any sense to specify program headers for them.  It wouldn't
> > help in the vdso case anyway, since the problem there is that you only
> > have the loaded part of the original ELF file.
> 
> The vdso contains a section table, as well.  When I hack
> bfd_from_remote_memory to create BFD sections from them similar
> to what elf_object_p does, I get the target sections that I wanted in GDB.
> The patch is rather big, though, and duplicating a lot of elf_object_p's code.
> 
> I have not tried generating fake sections from segments, yet.

This turned out to be rather simple.  Is this the right direction to go?
Should I maybe restrict this to PT_LOAD segments?


diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 20101be..22aae2a 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1771,7 +1771,6 @@ NAME(_bfd_elf,bfd_from_remote_memory)
 	    return NULL;
 	  }
       }
-  free (x_phdrs);
 
   /* If the segments visible in memory didn't include the section headers,
      then clear them from the file header.  */
@@ -1791,6 +1790,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct bfd_in_memory));
   if (bim == NULL)
     {
+      free (x_phdrs);
       free (contents);
       bfd_set_error (bfd_error_no_memory);
       return NULL;
@@ -1799,6 +1799,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   if (nbfd == NULL)
     {
       free (bim);
+      free (x_phdrs);
       free (contents);
       bfd_set_error (bfd_error_no_memory);
       return NULL;
@@ -1815,6 +1816,12 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   nbfd->mtime = time (NULL);
   nbfd->mtime_set = TRUE;
 
+  /* Add fake sections for program headers.  We ignore errors.  */
+  for (i = 0; i < i_ehdr.e_phnum; ++i)
+    (void) bfd_section_from_phdr (nbfd, &i_phdrs[i], i);
+
+  free (x_phdrs);
+
   if (loadbasep)
     *loadbasep = loadbase;
   return nbfd;


regards,
markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Mark Wielaard]

On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> It wouldn't
> help in the vdso case anyway, since the problem there is that you only
> have the loaded part of the original ELF file.

Note that the vdso is often special, compared to other ELF dsos, because
the loaded part is just the complete ELF image in memory. Since they are
very simple they will just have one PT_LOAD at offset zero and if the
image is smaller than the page size then the whole file is just simply
mapped into memory completely. So by fetching the vdso ELF image from
remote memory you should be able to get the section headers and the
not-allocated sections too.

Cheers,

Mark

Re: vdso handling

[Pedro Alves]

On 03/13/2014 08:23 AM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Alan Modra [mailto:amodra@gmail.com]
>> Sent: Thursday, March 13, 2014 2:02 AM
>> To: Cary Coutant
>> Cc: Doug Evans; Metzger, Markus T; gdb@sourceware.org;
>> binutils@sourceware.org
>> Subject: Re: vdso handling
>>
>> On Wed, Mar 12, 2014 at 01:22:58PM -0700, Cary Coutant wrote:
>>>> I think a case can be made that gdb should be able to use the
>>>> "execution view" of the program here.
>>>> As for how to achieve that ... "Discuss." :-)
>>>
>>> Add a PT_DEBUG program header entry? The PT_DEBUG segment would
>> need
>>> to have a small header that allows the debugger to find .debug_abbrev,
>>> .debug_info, etc. (i.e., a mini section table). Or, just add
>>> individual program header entries for each of the standard debug
>>> sections: PT_DEBUG_ABBREV, PT_DEBUG_INFO, etc.
>>
>> Debug sections are not normally loaded.  For that reason I don't think
>> it makes any sense to specify program headers for them.  It wouldn't
>> help in the vdso case anyway, since the problem there is that you only
>> have the loaded part of the original ELF file.
> 
> The vdso contains a section table, as well.  When I hack
> bfd_from_remote_memory to create BFD sections from them similar
> to what elf_object_p does, I get the target sections that I wanted in GDB.

I got curious.  Indeed:

$ gdb --quiet /bin/ls
Reading symbols from /usr/bin/ls...(no debugging symbols found)...done.
(gdb) catch syscall
Catchpoint 1 (any syscall)
(gdb) r
Starting program: /usr/bin/ls

Catchpoint 1 (call to syscall brk), 0x000000323d0163fa in __brk (addr=addr@entry=0x0) at ../sysdeps/unix/sysv/linux/x86_64/brk.c:32
32        __curbrk = newbrk = (void *) INLINE_SYSCALL (brk, 1, addr);
(gdb) shell cat /proc/23042/maps|grep vdso
7ffff7ffd000-7ffff7fff000 r-xp 00000000 00:00 0                          [vdso]
(gdb) dump memory /tmp/vdso.so 0x7ffff7ffd000 0x7ffff7fff000
(gdb) q

$ objdump -h /tmp/vdso.so

/tmp/vdso.so:     file format elf64-x86-64

Sections:
Idx Name          Size      VMA               LMA               File off  Algn
  0 .hash         00000040  ffffffffff700120  ffffffffff700120  00000120  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  1 .dynsym       00000108  ffffffffff700160  ffffffffff700160  00000160  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  2 .dynstr       0000005e  ffffffffff700268  ffffffffff700268  00000268  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .gnu.version  00000016  ffffffffff7002c6  ffffffffff7002c6  000002c6  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .gnu.version_d 00000038  ffffffffff7002e0  ffffffffff7002e0  000002e0  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 .note         0000003c  ffffffffff700318  ffffffffff700318  00000318  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .eh_frame_hdr 0000003c  ffffffffff700354  ffffffffff700354  00000354  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  7 .eh_frame     00000138  ffffffffff700390  ffffffffff700390  00000390  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
  8 .dynamic      000000f0  ffffffffff7004c8  ffffffffff7004c8  000004c8  2**3
                  CONTENTS, ALLOC, LOAD, DATA
  9 .rodata       00000020  ffffffffff7005b8  ffffffffff7005b8  000005b8  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 __bug_table   0000000c  ffffffffff7005d8  ffffffffff7005d8  000005d8  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 11 .discard      00000006  ffffffffff7005e4  ffffffffff7005e4  000005e4  2**0
                  CONTENTS, ALLOC, LOAD, DATA
 12 .altinstructions 00000048  ffffffffff7005ea  ffffffffff7005ea  000005ea  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA
 13 .altinstr_replacement 00000012  ffffffffff700632  ffffffffff700632  00000632  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 14 .text         0000069d  ffffffffff700700  ffffffffff700700  00000700  2**4
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .comment      0000002c  0000000000000000  0000000000000000  00000d9d  2**0
                  CONTENTS, READONLY

> The patch is rather big, though, and duplicating a lot of elf_object_p's code.

Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
a file-backed bfd internally in bfd?  It sounds to me that this should be
doable without duplication.

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 10:07 AM, Pedro Alves wrote:
> Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
> a file-backed bfd internally in bfd?  It sounds to me that this should be
> doable without duplication.

BTW, I meant that for vDSO's only.  The vsyscall page is not an elf,
and therefore bfd still needs to be passed a template elf.  For the
latter, GDB would indeed need to work with the segments.  Do we still
care for vsyscall kernels?  But for the former, bfd should just be
able to read the whole DSO as a plain elf.

Some glibc versions even include the vdso in the DSO list (*), and GDB
should be able to tell that that DSO is the vDSO (by matching addresses), and
load it completely from memory, still using a memory backed bfd, but _without_
a template.  So with that in mind, bfd should be able to read the vdso
as a bfd from memory using the same paths as a file-backed bfd, except,
well, the bfd's backing store is in memory rather than in a file.

(*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
doesn't show it, on some systems.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
> On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> > It wouldn't
> > help in the vdso case anyway, since the problem there is that you only
> > have the loaded part of the original ELF file.
> 
> Note that the vdso is often special, compared to other ELF dsos, because
> the loaded part is just the complete ELF image in memory. Since they are
> very simple they will just have one PT_LOAD at offset zero and if the
> image is smaller than the page size then the whole file is just simply
> mapped into memory completely. So by fetching the vdso ELF image from
> remote memory you should be able to get the section headers and the
> not-allocated sections too.

Yes, but if the vdso does not fit in a page (which incidentally is
inferred by program header p_align), then you may lose the section
headers.  I was assuming this was the case.

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 10:07:10AM +0000, Pedro Alves wrote:
> Why doesn't the memory-backed bfd paths take the same paths as
> a file-backed bfd internally in bfd?

I think they do.  In symfile-mem.c:symbol_file_add_from_memory

  nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, &loadbase,
					 target_read_memory_bfd);
..
  if (!bfd_check_format (nbfd, bfd_object))

That bfd_check_format is where elf_object_p is called.  *If*
bfd_from_remote_memory picked up the sections headers, you'll get a
normal bfd with sections..

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Mark Wielaard]

On Thu, 2014-03-13 at 23:33 +1030, Alan Modra wrote:
> On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
> > On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
> > > It wouldn't
> > > help in the vdso case anyway, since the problem there is that you only
> > > have the loaded part of the original ELF file.
> > 
> > Note that the vdso is often special, compared to other ELF dsos, because
> > the loaded part is just the complete ELF image in memory. Since they are
> > very simple they will just have one PT_LOAD at offset zero and if the
> > image is smaller than the page size then the whole file is just simply
> > mapped into memory completely. So by fetching the vdso ELF image from
> > remote memory you should be able to get the section headers and the
> > not-allocated sections too.
> 
> Yes, but if the vdso does not fit in a page (which incidentally is
> inferred by program header p_align), then you may lose the section
> headers.  I was assuming this was the case.

Yes, you are right. Almost everything else is bigger than a page and has
multiple PT_LOAD segments. In which case determining whether the full
shdrs were mapped in from the file is much trickier. And might have to
rely on only the phdrs.

We recently went through this for elfutils, which also contains an
elf_from_remote_memory implementation (if someone needs inspiration).
Currently that implementation just uses the phdrs and so needs some
heuristics using the program header fields to know what part of the file
was mapped where. But if you have access to the actual /proc/PID/maps
ranges you might be able to do something more accurate, or at least be
able to cross check that the program headers view is really how the ELF
image is actually laid out (*).

Cheers,

Mark

(*) Don't make the mistake we made to think p_align is what segments are
actually aligned to, the dynamic loader on GNU/Linux just uses the
actual page size of the architecture to align the mappings:
https://lists.fedorahosted.org/pipermail/elfutils-devel/2014-March/003859.html

Re: vdso handling

[Pedro Alves]

On 03/13/2014 01:03 PM, Alan Modra wrote:
> On Thu, Mar 13, 2014 at 10:52:16AM +0100, Mark Wielaard wrote:
>> On Thu, 2014-03-13 at 11:31 +1030, Alan Modra wrote:
>>> It wouldn't
>>> help in the vdso case anyway, since the problem there is that you only
>>> have the loaded part of the original ELF file.
>>
>> Note that the vdso is often special, compared to other ELF dsos, because
>> the loaded part is just the complete ELF image in memory. Since they are
>> very simple they will just have one PT_LOAD at offset zero and if the
>> image is smaller than the page size then the whole file is just simply
>> mapped into memory completely. So by fetching the vdso ELF image from
>> remote memory you should be able to get the section headers and the
>> not-allocated sections too.
> 
> Yes, but if the vdso does not fit in a page (which incidentally is
> inferred by program header p_align), then you may lose the section
> headers.  I was assuming this was the case.

Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:

        .globl vdso_start, vdso_end
        .align PAGE_SIZE
vdso_start:
        .incbin "arch/x86/vdso/vdso.so"
vdso_end:
        .align PAGE_SIZE /* extra data here leaks to userspace. */


And then arch/x86/vdso/vma.c has:

static int __init init_vdso(void)
{
        int npages = (vdso_end - vdso_start + PAGE_SIZE - 1) / PAGE_SIZE;
        int i;

        patch_vdso64(vdso_start, vdso_end - vdso_start);

        vdso_size = npages << PAGE_SHIFT;
        for (i = 0; i < npages; i++)
                vdso_pages[i] = virt_to_page(vdso_start + i*PAGE_SIZE);

#ifdef CONFIG_X86_X32_ABI
        patch_vdsox32(vdsox32_start, vdsox32_end - vdsox32_start);
        npages = (vdsox32_end - vdsox32_start + PAGE_SIZE - 1) / PAGE_SIZE;
        vdsox32_size = npages << PAGE_SHIFT;
        for (i = 0; i < npages; i++)
                vdsox32_pages[i] = virt_to_page(vdsox32_start + i*PAGE_SIZE);
#endif

        return 0;
}

And patch_vdso64 _relies_ on sections being present at runtime:

static void __init patch_vdso64(void *vdso, size_t len)
{
        Elf64_Ehdr *hdr = vdso;
        Elf64_Shdr *sechdrs, *alt_sec = 0;
        char *secstrings;
        void *alt_data;
        int i;

        BUG_ON(len < sizeof(Elf64_Ehdr));
        BUG_ON(memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0);

        sechdrs = (void *)hdr + hdr->e_shoff;
        secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;

        for (i = 1; i < hdr->e_shnum; i++) {
                Elf64_Shdr *shdr = &sechdrs[i];
                if (!strcmp(secstrings + shdr->sh_name, ".altinstructions")) {
                        alt_sec = shdr;
                        goto found;
                }
        }

        /* If we get here, it's probably a bug. */
        pr_warning("patch_vdso64: .altinstructions not found\n");
        return;  /* nothing to patch */

found:
        alt_data = (void *)hdr + alt_sec->sh_offset;
        apply_alternatives(alt_data, alt_data + alt_sec->sh_size);
}

On e.g., arch/powerpc/kernel/vdso.c, I see even a lot more
code looking at the sections of the vdso.

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 02:59 PM, Pedro Alves wrote:

> Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:

Hmm, guess I should really be looking at where the vdso is
actually mapped to a process's address space...  (no idea where
that is).

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/13/2014 03:04 PM, Pedro Alves wrote:
> On 03/13/2014 02:59 PM, Pedro Alves wrote:
> 
>> Hmm.  How so?  On x86 (arch/x86/vdso/vdso.S), the kernel just does:
> 
> Hmm, guess I should really be looking at where the vdso is
> actually mapped to a process's address space...  (no idea where
> that is).
> 

I think I found it in the same file (arch/x86/vdso/vma.c):

int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp)
{
	return setup_additional_pages(bprm, uses_interp, vdso_pages,
				      vdso_size);
}

vdso_size comes from:

static int __init init_vdso(void)
{
	int npages = (vdso_end - vdso_start + PAGE_SIZE - 1) / PAGE_SIZE;
	int i;

	patch_vdso64(vdso_start, vdso_end - vdso_start);

	vdso_size = npages << PAGE_SHIFT;


So it seems like the whole vdso should be always mapped in.

I didn't look at the whole mode compat mess, but if it behaves
differently, it'd sound like a kernel bug to me.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Thu, Mar 13, 2014 at 03:26:18PM +0000, Pedro Alves wrote:
> So it seems like the whole vdso should be always mapped in.

OK, so I think Markus should be looking at why bfd_from_remote_memory
decides to exclude the section headers.  ie. why the following code
is being executed

  /* If the segments visible in memory didn't include the section headers,
     then clear them from the file header.  */
  if ((bfd_vma) contents_size < (i_ehdr.e_shoff
				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
    {
      memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
      memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
      memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
    }

It may be that we can tweak the heuristic.  One thing I noticed is
that the following

  /* Trim the last segment so we don't bother with zeros in the last page
     that are off the end of the file.  However, if the extra bit in that
     page includes the section headers, keep them.  */

will trim off .symtab which might otherwise be available.  GNU ld places
.symtab after the section headers.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Friday, March 14, 2014 12:54 AM


> OK, so I think Markus should be looking at why bfd_from_remote_memory
> decides to exclude the section headers.  ie. why the following code
> is being executed
> 
>   /* If the segments visible in memory didn't include the section headers,
>      then clear them from the file header.  */
>   if ((bfd_vma) contents_size < (i_ehdr.e_shoff
> 				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
>     {
>       memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
>       memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
>       memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
>     }
> 
> It may be that we can tweak the heuristic.  One thing I noticed is
> that the following
> 
>   /* Trim the last segment so we don't bother with zeros in the last page
>      that are off the end of the file.  However, if the extra bit in that
>      page includes the section headers, keep them.  */
> 
> will trim off .symtab which might otherwise be available.  GNU ld places
> .symtab after the section headers.

Something like this?

diff --git a/bfd/elfcode.h b/bfd/elfcode.h
index 20101be..601d7ea 100644
--- a/bfd/elfcode.h
+++ b/bfd/elfcode.h
@@ -1616,7 +1616,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
   bfd_byte *contents;
   int err;
   unsigned int i;
-  bfd_vma loadbase;
+  bfd_vma loadbase, shdr_begin, shdr_end;
   bfd_boolean loadbase_set;
 
   /* Read in the ELF header in external format.  */
@@ -1728,20 +1728,16 @@ NAME(_bfd_elf,bfd_from_remote_memory)
     }
 
   /* Trim the last segment so we don't bother with zeros in the last page
-     that are off the end of the file.  However, if the extra bit in that
-     page includes the section headers, keep them.  */
-  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
-      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-    {
-      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
-      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
-    }
-  else
+     that are off the end of the file.  */
+  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz)
     contents_size = last_phdr->p_offset + last_phdr->p_filesz;
 
+  /* Keep the section headers.  */
+  shdr_begin = i_ehdr.e_shoff;
+  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
+  if (shdr_end > (bfd_vma) contents_size)
+    contents_size = shdr_end;
+
   /* Now we know the size of the whole image we want read in.  */
   contents = (bfd_byte *) bfd_zmalloc (contents_size);
   if (contents == NULL)
@@ -1773,18 +1769,19 @@ NAME(_bfd_elf,bfd_from_remote_memory)
       }
   free (x_phdrs);
 
-  /* If the segments visible in memory didn't include the section headers,
-     then clear them from the file header.  */
-  if ((bfd_vma) contents_size < (i_ehdr.e_shoff
-				 + i_ehdr.e_shnum * i_ehdr.e_shentsize))
-    {
-      memset (&x_ehdr.e_shoff, 0, sizeof x_ehdr.e_shoff);
-      memset (&x_ehdr.e_shnum, 0, sizeof x_ehdr.e_shnum);
-      memset (&x_ehdr.e_shstrndx, 0, sizeof x_ehdr.e_shstrndx);
-    }
+  /* Copy the section headers.  */
+  err = target_read_memory (ehdr_vma + shdr_begin,
+			    contents + shdr_begin, shdr_end - shdr_begin);
+  if (err)
+  {
+    free (contents);
+    bfd_set_error (bfd_error_system_call);
+    errno = err;
+    return NULL;
+  }
 
   /* This will normally have been in the first PT_LOAD segment.  But it
-     conceivably could be missing, and we might have just changed it.  */
+     conceivably could be missing.  */
   memcpy (contents, &x_ehdr, sizeof x_ehdr);
 
   /* Now we have a memory image of the ELF file contents.  Make a BFD.  */


Would it be OK to send this patch as part of a GDB patch series with
binutils-patches and you CC'ed?  Or do you want a separate patch
only to binutils-patches?

This patch alone runs without new fails in the gdb and binutils suite
on x86-64 Fedora 19.  For the latter I ran "make check" from
$build/binutils, where $build is my build directory from I which I called
configure.

In combination with my other GDB changes, it shows regressions in
gdb.base/break-interp.exp.  I'll send the patch once I fixed them.

thanks,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Alan Modra]

On Tue, Mar 18, 2014 at 03:09:58PM +0000, Metzger, Markus T wrote:
> diff --git a/bfd/elfcode.h b/bfd/elfcode.h
> index 20101be..601d7ea 100644
> --- a/bfd/elfcode.h
> +++ b/bfd/elfcode.h
> @@ -1616,7 +1616,7 @@ NAME(_bfd_elf,bfd_from_remote_memory)
>    bfd_byte *contents;
>    int err;
>    unsigned int i;
> -  bfd_vma loadbase;
> +  bfd_vma loadbase, shdr_begin, shdr_end;
>    bfd_boolean loadbase_set;
>  
>    /* Read in the ELF header in external format.  */
> @@ -1728,20 +1728,16 @@ NAME(_bfd_elf,bfd_from_remote_memory)
>      }
>  
>    /* Trim the last segment so we don't bother with zeros in the last page
> -     that are off the end of the file.  However, if the extra bit in that
> -     page includes the section headers, keep them.  */
> -  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz
> -      && (bfd_vma) contents_size >= (i_ehdr.e_shoff
> -				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
> -    {
> -      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
> -      if ((bfd_vma) contents_size < (i_ehdr.e_shoff
> -				     + i_ehdr.e_shnum * i_ehdr.e_shentsize))
> -	contents_size = i_ehdr.e_shoff + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> -    }
> -  else
> +     that are off the end of the file.  */
> +  if ((bfd_vma) contents_size > last_phdr->p_offset + last_phdr->p_filesz)
>      contents_size = last_phdr->p_offset + last_phdr->p_filesz;
>  
> +  /* Keep the section headers.  */
> +  shdr_begin = i_ehdr.e_shoff;
> +  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> +  if (shdr_end > (bfd_vma) contents_size)
> +    contents_size = shdr_end;
> +

I don't think this is a good idea.  If/when bfd_from_remote_memory is
used for something other than the linux kernel vdso, we can't assume
the section headers are loaded.  The original code made the assumption
that the highest address loaded from a PT_LOAD header was rounded up
to a page, and that the page size could be inferred from p_align.
Here:
	segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
		       + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
	if (segment_end > (bfd_vma) contents_size)
	  contents_size = segment_end;

Now, p_align is generally set from the page size if using GNU ld, but
I'm wondering if your vdso somehow doesn't have that property.  Can
you show us your vdso readelf -e output?  If p_align isn't set to a
page, then the change in heuristic I envision is to make use of
elf_backend_data maxpagesize to figure out which parts of the image
might be loaded.  If that isn't enough then perhaps we should add
another parameter to bfd_from_remote_memory to allow its caller to
specify the end of the image.

> Would it be OK to send this patch as part of a GDB patch series with
> binutils-patches and you CC'ed?  Or do you want a separate patch
> only to binutils-patches?

I don't mind either way.  This part of bfd belongs to gdb, so gdb
maintainers really have the final say on patch approval.  I'm just
someone who happened to become interested in the problem..

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, March 19, 2014 12:10 AM

> Now, p_align is generally set from the page size if using GNU ld, but
> I'm wondering if your vdso somehow doesn't have that property.  Can
> you show us your vdso readelf -e output?

ELF Header:
  Magic:   7f 45 4c 46 02 01 01 00 00 00 00 00 00 00 00 00 
  Class:                             ELF64
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Shared object file)
  Machine:                           Advanced Micro Devices X86-64
  Version:                           0x1
  Entry point address:               0xffffffffff700700
  Start of program headers:          64 (bytes into file)
  Start of section headers:          4320 (bytes into file)
  Flags:                             0x0
  Size of this header:               64 (bytes)
  Size of program headers:           56 (bytes)
  Number of program headers:         4
  Size of section headers:           64 (bytes)
  Number of section headers:         18
  Section header string table index: 17

Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            0000000000000000 000000 000000 00      0   0  0
  [ 1] .hash             HASH            ffffffffff700120 000120 000040 04   A  2   0  8
  [ 2] .dynsym           DYNSYM          ffffffffff700160 000160 000108 18   A  3   2  8
  [ 3] .dynstr           STRTAB          ffffffffff700268 000268 00005e 00   A  0   0  1
  [ 4] .gnu.version      VERSYM          ffffffffff7002c6 0002c6 000016 02   A  2   0  2
  [ 5] .gnu.version_d    VERDEF          ffffffffff7002e0 0002e0 000038 00   A  3   2  8
  [ 6] .note             NOTE            ffffffffff700318 000318 00003c 00   A  0   0  4
  [ 7] .eh_frame_hdr     PROGBITS        ffffffffff700354 000354 00002c 00   A  0   0  4
  [ 8] .eh_frame         PROGBITS        ffffffffff700380 000380 0000e8 00   A  0   0  8
  [ 9] .dynamic          DYNAMIC         ffffffffff700468 000468 0000f0 10  WA  3   0  8
  [10] .rodata           PROGBITS        ffffffffff700558 000558 000020 01 AMS  0   0  8
  [11] __bug_table       PROGBITS        ffffffffff700578 000578 000018 00   A  0   0  1
  [12] .discard          PROGBITS        ffffffffff700590 000590 000012 00  WA  0   0  1
  [13] .altinstructions  PROGBITS        ffffffffff7005a2 0005a2 0000d8 00   A  0   0  1
  [14] .altinstr_replacement PROGBITS        ffffffffff70067a 00067a 000036 00  AX  0   0  1
  [15] .text             PROGBITS        ffffffffff700700 000700 0008fd 00  AX  0   0 16
  [16] .comment          PROGBITS        0000000000000000 000ffd 00002c 01  MS  0   0  1
  [17] .shstrtab         STRTAB          0000000000000000 001029 0000b7 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), l (large)
  I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x000000 0xffffffffff700000 0xffffffffff700000 0x000ffd 0x000ffd R E 0x1000
  DYNAMIC        0x000468 0xffffffffff700468 0xffffffffff700468 0x0000f0 0x0000f0 R   0x8
  NOTE           0x000318 0xffffffffff700318 0xffffffffff700318 0x00003c 0x00003c R   0x4
  GNU_EH_FRAME   0x000354 0xffffffffff700354 0xffffffffff700354 0x00002c 0x00002c R   0x4

 Section to Segment mapping:
  Segment Sections...
   00     .hash .dynsym .dynstr .gnu.version .gnu.version_d .note .eh_frame_hdr .eh_frame .dynamic .rodata __bug_table .discard .altinstructions .altinstr_replacement .text 
   01     .dynamic 
   02     .note 
   03     .eh_frame_hdr

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, March 19, 2014 12:10 AM


> > +  /* Keep the section headers.  */
> > +  shdr_begin = i_ehdr.e_shoff;
> > +  shdr_end = shdr_begin + i_ehdr.e_shnum * i_ehdr.e_shentsize;
> > +  if (shdr_end > (bfd_vma) contents_size)
> > +    contents_size = shdr_end;
> > +
> 
> I don't think this is a good idea.  If/when bfd_from_remote_memory is
> used for something other than the linux kernel vdso, we can't assume
> the section headers are loaded.

Shouldn't the ehdr indicate that there are no sections in this case?


> The original code made the assumption
> that the highest address loaded from a PT_LOAD header was rounded up
> to a page, and that the page size could be inferred from p_align.
> Here:
> 	segment_end = (i_phdrs[i].p_offset + i_phdrs[i].p_filesz
> 		       + i_phdrs[i].p_align - 1) & -i_phdrs[i].p_align;
> 	if (segment_end > (bfd_vma) contents_size)
> 	  contents_size = segment_end;
> 
> Now, p_align is generally set from the page size if using GNU ld, but
> I'm wondering if your vdso somehow doesn't have that property.  Can
> you show us your vdso readelf -e output?  If p_align isn't set to a
> page, then the change in heuristic I envision is to make use of
> elf_backend_data maxpagesize to figure out which parts of the image
> might be loaded.  If that isn't enough then perhaps we should add
> another parameter to bfd_from_remote_memory to allow its caller to
> specify the end of the image.

I'm not sure the caller knows.  GDB gets the base address from AUXV's
AT_SYSINFO_EHDR.

If we can't trust the image to contain everything that the ELF header
describes, would it be safer to generate fake sections based on the
program header?  We already assume that the program header is
contained in the image.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/18/2014 11:09 PM, Alan Modra wrote:
> I don't think this is a good idea.  If/when bfd_from_remote_memory is
> used for something other than the linux kernel vdso, we can't assume
> the section headers are loaded.  

I wonder what use cases are these though.  It'd be odd to me to
load the elf headers, but not all that the headers point at.
Sounds like we should just make that a requirement?

I looked at the history of this whole code, and here's what I found.

Roland originally added this bfd function for reading the Linux
vsyscall dso, in 8d6337fe:

 https://sourceware.org/ml/binutils/2003-05/msg00542.html

As far as I can judge from http://lwn.net/Articles/30258/ , and
from looking at the early days of the vsyscall dso in the Linux
tree, it looks like the vsyscall dso was always included complete
in memory (e.g., at v2.6.12-rc2, the initial git import):

...
/* 32bit VDSOs mapped into user space. */
asm(".section \".init.data\",\"aw\"\n"
    "syscall32_syscall:\n"
    ".incbin \"arch/x86_64/ia32/vsyscall-syscall.so\"\n"
    "syscall32_syscall_end:\n"
    "syscall32_sysenter:\n"
    ".incbin \"arch/x86_64/ia32/vsyscall-sysenter.so\"\n"
    "syscall32_sysenter_end:\n"
    ".previous");
...

# The DSO images are built using a special linker script
quiet_cmd_syscall = SYSCALL $@
      cmd_syscall = $(CC) -m32 -nostdlib -shared -s \
                           -Wl,-soname=linux-gate.so.1 -o $@ \
                           -Wl,-T,$(filter-out FORCE,$^)

...

I found no sign of strip or of any special elf munging.

GDB only uses bfd_elf_bfd_from_remote_memory for the vdso,
and for "add-symbol-file-from-memory".

Roland himself added the "add-symbol-file-from-memory"
command (5417f6dc) to GDB too, at:

 https://www.sourceware.org/ml/gdb-patches/2003-10/msg00045.html

 "This command may not really be worth having, but it serves to exercise the
 underlying function symbol_file_add_from_memory.  That function does the
 work of reading symbols and unwind info from the Linux vsyscall DSO."

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/19/2014 08:29 AM, Metzger, Markus T wrote:
> If we can't trust the image to contain everything that the ELF header
> describes, would it be safer to generate fake sections based on the
> program header?  We already assume that the program header is
> contained in the image.

Please let's try to make this work before giving up and going
in that direction.

-- 
Pedro Alves

Re: vdso handling

[Alan Modra]

On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
> On 03/18/2014 11:09 PM, Alan Modra wrote:
> > I don't think this is a good idea.  If/when bfd_from_remote_memory is
> > used for something other than the linux kernel vdso, we can't assume
> > the section headers are loaded.  
> 
> I wonder what use cases are these though.  It'd be odd to me to
> load the elf headers, but not all that the headers point at.

Well, no, the section headers are part of the linking view.  The ELF
file header points at their *file offset*.  Thinking it odd that they
are not loaded is exactly the same as thinking it odd that .comment is
not loaded!

> Sounds like we should just make that a requirement?

You indeed could make that a requirement, but it'd mean that object
files loaded by glibc ld.so would not satisfy the requirement.

Taking Markus' vdso as an example, the PT_LOAD header covers file
offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
loading a similar image, it would just load in 0 to 0xfff (assuming
ld.so's dl_pagesize is also 0x1000).  The section headers are at file
offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
since they are completely extraneous to executing the image it would
be foolish to waste another page to load them.

>  "This command may not really be worth having, but it serves to exercise the
>  underlying function symbol_file_add_from_memory.  That function does the
>  work of reading symbols and unwind info from the Linux vsyscall DSO."

Heh, OK, so don't let my comments about more general uses of
bfd_from_remote_memory get in your way of fixing this problem.

Hmm, if you only want to read vdsos then bfd_from_remote_memory is way
overengineered.  In fact, I think it could disappear entirely.
If you can find the extent of the vdso in gdb, then all of the ELF
version of bfd_from_remote_memory prior to allocating the bim is
unnecessary, because all that code really does is find the extent.
So the tail of bfd_from_remote_memory moves to bfd/opncls.c as
(totally untested):

bfd_boolean
bfd_openr_bim (bfd *templ, void *contents, size_t contents_size)
{
  struct bfd_in_memory *bim;

  bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct bfd_in_memory));
  if (bim == NULL)
    return NULL;

  nbfd = _bfd_new_bfd ();
  if (nbfd == NULL)
    {
      free (bim);
      return NULL;
    }
  nbfd->filename = "<in-memory>";
  if (templ != NULL)
    nbfd->xvec = templ->xvec;
  bim->size = contents_size;
  bim->buffer = (bfd_byte *) contents;
  nbfd->iostream = bim;
  nbfd->flags = BFD_IN_MEMORY;
  nbfd->iovec = &_bfd_memory_iovec;
  nbfd->origin = 0;
  nbfd->direction = read_direction;
  nbfd->mtime = time (NULL);
  nbfd->mtime_set = TRUE;
  return nbfd;
}

gdb is then responsible for filling in "contents" and determining
"contents_size", which presumably can be done in the same way as
you did in https://sourceware.org/ml/binutils/2014-03/msg00130.html
The loadbase calculation also moves to gdb, which shouldn't be too
hard.  Note that "templ" above is optional, which allows you to get
rid of
       /* FIXME: cagney/2004-05-06: Should not require an existing
         BFD when trying to create a run-time BFD of the VSYSCALL

-- 
Alan Modra
Australia Development Lab, IBM

Re: vdso handling

[Alan Modra]

On Wed, Mar 19, 2014 at 08:29:47AM +0000, Metzger, Markus T wrote:
> Shouldn't the ehdr indicate that there are no sections in this case?

Nope.  See my other email to Pedro.

> If we can't trust the image to contain everything that the ELF header
> describes, would it be safer to generate fake sections based on the
> program header?  We already assume that the program header is
> contained in the image.

Yes, you're correct that it is wrong to assume program headers are
loaded.  Even worse, the in-memory image doesn't even need to contain
the ELF file header.

-- 
Alan Modra
Australia Development Lab, IBM

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Thursday, March 20, 2014 2:33 AM


> On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
> > On 03/18/2014 11:09 PM, Alan Modra wrote:
> > > I don't think this is a good idea.  If/when bfd_from_remote_memory is
> > > used for something other than the linux kernel vdso, we can't assume
> > > the section headers are loaded.
> >
> > I wonder what use cases are these though.  It'd be odd to me to
> > load the elf headers, but not all that the headers point at.
> 
> Well, no, the section headers are part of the linking view.  The ELF
> file header points at their *file offset*.  Thinking it odd that they
> are not loaded is exactly the same as thinking it odd that .comment is
> not loaded!
> 
> > Sounds like we should just make that a requirement?
> 
> You indeed could make that a requirement, but it'd mean that object
> files loaded by glibc ld.so would not satisfy the requirement.
> 
> Taking Markus' vdso as an example, the PT_LOAD header covers file
> offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
> loading a similar image, it would just load in 0 to 0xfff (assuming
> ld.so's dl_pagesize is also 0x1000).  The section headers are at file
> offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
> since they are completely extraneous to executing the image it would
> be foolish to waste another page to load them.
> 
> >  "This command may not really be worth having, but it serves to exercise
> the
> >  underlying function symbol_file_add_from_memory.  That function does
> the
> >  work of reading symbols and unwind info from the Linux vsyscall DSO."
> 
> Heh, OK, so don't let my comments about more general uses of
> bfd_from_remote_memory get in your way of fixing this problem.
> 
> Hmm, if you only want to read vdsos then bfd_from_remote_memory is way
> overengineered.  In fact, I think it could disappear entirely.
> If you can find the extent of the vdso in gdb, then all of the ELF
> version of bfd_from_remote_memory prior to allocating the bim is
> unnecessary, because all that code really does is find the extent.
> So the tail of bfd_from_remote_memory moves to bfd/opncls.c as
> (totally untested):
> 
> bfd_boolean
> bfd_openr_bim (bfd *templ, void *contents, size_t contents_size)
> {
>   struct bfd_in_memory *bim;
> 
>   bim = (struct bfd_in_memory *) bfd_malloc (sizeof (struct
> bfd_in_memory));
>   if (bim == NULL)
>     return NULL;
> 
>   nbfd = _bfd_new_bfd ();
>   if (nbfd == NULL)
>     {
>       free (bim);
>       return NULL;
>     }
>   nbfd->filename = "<in-memory>";
>   if (templ != NULL)
>     nbfd->xvec = templ->xvec;
>   bim->size = contents_size;
>   bim->buffer = (bfd_byte *) contents;
>   nbfd->iostream = bim;
>   nbfd->flags = BFD_IN_MEMORY;
>   nbfd->iovec = &_bfd_memory_iovec;
>   nbfd->origin = 0;
>   nbfd->direction = read_direction;
>   nbfd->mtime = time (NULL);
>   nbfd->mtime_set = TRUE;
>   return nbfd;
> }
> 
> gdb is then responsible for filling in "contents" and determining
> "contents_size", which presumably can be done in the same way as
> you did in https://sourceware.org/ml/binutils/2014-03/msg00130.html
> The loadbase calculation also moves to gdb, which shouldn't be too
> hard.  Note that "templ" above is optional, which allows you to get
> rid of
>        /* FIXME: cagney/2004-05-06: Should not require an existing
>          BFD when trying to create a run-time BFD of the VSYSCALL

I was waiting for Pedro to reply but he didn't.

This would move the ELF processing itself into GDB.  That sounds a bit
odd to me but it seems GDB already does a fair amount of ELF reading.

Pedro, are you OK with this?  Will you accept a patch that goes into
the direction that Alan described above?

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Pedro Alves]

On 03/20/2014 01:33 AM, Alan Modra wrote:
> On Wed, Mar 19, 2014 at 12:03:40PM +0000, Pedro Alves wrote:
>> On 03/18/2014 11:09 PM, Alan Modra wrote:
>>> I don't think this is a good idea.  If/when bfd_from_remote_memory is
>>> used for something other than the linux kernel vdso, we can't assume
>>> the section headers are loaded.  
>>
>> I wonder what use cases are these though.  It'd be odd to me to
>> load the elf headers, but not all that the headers point at.
> 
> Well, no, the section headers are part of the linking view.  The ELF
> file header points at their *file offset*.  Thinking it odd that they
> are not loaded is exactly the same as thinking it odd that .comment is
> not loaded!

:-)  Sorry, I get that, but I was somehow assuming the elf header
wasn't itself covered by a PT_LOAD.  Rookie mistake...

>> Sounds like we should just make that a requirement?
> 
> You indeed could make that a requirement, but it'd mean that object
> files loaded by glibc ld.so would not satisfy the requirement.

Indeed.  So this mechanism could be further extended in future
to be able to read DSOs out of memory, and be able to retrieve
the dynamic symbol table, etc., which would allow getting at
unwind info for already-deleted libraries.  I guess that's what
the elfutils folks had been talking about, but I somehow managed
to not connect the dots.  :-)

I just tried pointing add-symbol-file-from-memory at an already
mapped DSO's elf header, but it doesn't work as is unfortunately:

 (gdb) info shared curses
 0x000000324d006d20  0x000000324d01df58  Yes         /lib64/libncurses.so.5
 (gdb) x /4b 0x000000324d000000
 0x324d000000:   127     69      76      70
 (gdb) add-symbol-file-from-memory 0x000000324d000000
 Failed to read a valid object file image from memory.

I single stepped a little through
bfd_elf_bfd_from_remote_memory - something goes wrong with the
reading of the load segment contents, probably something wrong
with the address computations.

> 
> Taking Markus' vdso as an example, the PT_LOAD header covers file
> offsets from 0 to 0xffd and page size is 0x1000.  If ld.so were
> loading a similar image, it would just load in 0 to 0xfff (assuming
> ld.so's dl_pagesize is also 0x1000).  The section headers are at file
> offset 0x10e0 so would miss being loaded.  Nor should they be loaded,
> since they are completely extraneous to executing the image it would
> be foolish to waste another page to load them.

*nod*

>>  "This command may not really be worth having, but it serves to exercise the
>>  underlying function symbol_file_add_from_memory.  That function does the
>>  work of reading symbols and unwind info from the Linux vsyscall DSO."
> 
> Heh, OK, so don't let my comments about more general uses of
> bfd_from_remote_memory get in your way of fixing this problem.

Hmm.  Thinking further ahead, it would be good to make GDB be able
to read deleted DSOs off of memory like elfutils is now doing.
With that in mind, it does seem to make sense to have this
bfd_elf_bfd_from_remote_memory code in place as is, but extend it to
treat section-less elfs just like cores, creating pseudo bfd
sections from segments...

-- 
Pedro Alves

Re: vdso handling

[Pedro Alves]

On 03/20/2014 01:59 AM, Alan Modra wrote:
> On Wed, Mar 19, 2014 at 08:29:47AM +0000, Metzger, Markus T wrote:
>> Shouldn't the ehdr indicate that there are no sections in this case?
> 
> Nope.  See my other email to Pedro.
> 
>> If we can't trust the image to contain everything that the ELF header
>> describes, would it be safer to generate fake sections based on the
>> program header?  We already assume that the program header is
>> contained in the image.
> 
> Yes, you're correct that it is wrong to assume program headers are
> loaded.  Even worse, the in-memory image doesn't even need to contain
> the ELF file header.

Yeah, and I was just assuming it didn't, hence my "just trust the
headers" push before.

I'm now thinking that we'll need pseudo-sections from program
headers anyway, so I'd suggest going in that direction, leaving
the add-symbol-file-from-memory command's intention generic,
and leave revisiting how gdb retrieves the vdso itself off of
memory for another day.

-- 
Pedro Alves

RE: vdso handling

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Friday, March 21, 2014 4:55 PM


> >> If we can't trust the image to contain everything that the ELF header
> >> describes, would it be safer to generate fake sections based on the
> >> program header?  We already assume that the program header is
> >> contained in the image.
> >
> > Yes, you're correct that it is wrong to assume program headers are
> > loaded.  Even worse, the in-memory image doesn't even need to contain
> > the ELF file header.
> 
> Yeah, and I was just assuming it didn't, hence my "just trust the
> headers" push before.
> 
> I'm now thinking that we'll need pseudo-sections from program
> headers anyway, so I'd suggest going in that direction, leaving
> the add-symbol-file-from-memory command's intention generic,
> and leave revisiting how gdb retrieves the vdso itself off of
> memory for another day.

That would be something like the patch in one of the previous emails
in this thread: https://sourceware.org/ml/gdb/2014-03/msg00028.html,
wouldn't it?

Alan, would you be OK with this, as well?

Thanks,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: vdso handling

[Samuel Bronson]

Pedro Alves <palves@redhat.com> writes:

> On 03/13/2014 10:07 AM, Pedro Alves wrote:
>> Why's that?  Why doesn't the memory-backed bfd paths take the same paths as
>> a file-backed bfd internally in bfd?  It sounds to me that this should be
>> doable without duplication.
>
> BTW, I meant that for vDSO's only.  The vsyscall page is not an elf,
> and therefore bfd still needs to be passed a template elf.  For the
> latter, GDB would indeed need to work with the segments.  Do we still
> care for vsyscall kernels?  But for the former, bfd should just be
> able to read the whole DSO as a plain elf.
>
> Some glibc versions even include the vdso in the DSO list (*), and GDB
> should be able to tell that that DSO is the vDSO (by matching addresses), and
                                                    ^^^^^^^^^^^^^^^^^^^^^
Hmm, why don't we already do that? It's bound to be easier than meeting
the conditions to get glibc to stop falsely cliaming that the vDSO comes
from a file <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c5>.

That'd be enough to take two bugs off of <http://bugs.debian.org/gdb>
right there.

> load it completely from memory, still using a memory backed bfd, but _without_
> a template.  So with that in mind, bfd should be able to read the vdso
> as a bfd from memory using the same paths as a file-backed bfd, except,
> well, the bfd's backing store is in memory rather than in a file.
>
> (*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
> doesn't show it, on some systems.

What versions don't list the vdso under some name or other?  (Mine calls
it linux-gate.so.1 for some reason.)

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature to help me spread!

Re: vdso handling

[Pedro Alves]

On 06/01/2014 09:31 PM, Samuel Bronson wrote:
> Pedro Alves <palves@redhat.com> writes:

>> Some glibc versions even include the vdso in the DSO list (*), and GDB
>> should be able to tell that that DSO is the vDSO (by matching addresses), and
>                                                     ^^^^^^^^^^^^^^^^^^^^^
> Hmm, why don't we already do that? It's bound to be easier than meeting
> the conditions to get glibc to stop falsely cliaming that the vDSO comes
> from a file <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c5>.

Dunno.  Because nobody has done it?

I suppose that's what Ulrich meant in
<https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c1>.

>> (*) note how linux-vdso.so.1 is listed by ldd, even if "info shared" in gdb
>> doesn't show it, on some systems.
> 
> What versions don't list the vdso under some name or other?  (Mine calls
> it linux-gate.so.1 for some reason.)

I don't know versions numbers, but all before the glibc commit
mentioned in <https://sourceware.org/bugzilla/show_bug.cgi?id=13097#c1>
I guess, and also, see the rest of the discussion there, indicating
that Fedora carries a reversion of the offending patch.

-- 
Pedro Alves