Re: [binutils-gdb] Fix the linker so that it will not silently generate ELF binaries with invalid program headers. Fix

[Nick Clifton <nickc at redhat dot com>]

Hi Maciej,

> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   PHDR           0x000034 0x00001034 0x00000000 0x000a0 0x000a0 R E 0x4
>   INTERP         0x001000 0x00080000 0x00080000 0x00013 0x00013 R   0x1
>       [Requesting program interpreter: /usr/lib/libc.so.1]
>   LOAD           0x001000 0x00080000 0x00080000 0x00408 0x00408 R E 0x1000
>   LOAD           0x002000 0x00081000 0x00081000 0x00804 0x00c00 RW  0x1000
>   DYNAMIC        0x002000 0x00081000 0x00081000 0x00078 0x00078 RW  0x4

The problem with this layout is that the ELF specification says in its description
of the program header types:

 PT_PHDR  The array element, if present, specifies the location 
          and size of the program header table itself, both in 
          the file and in the memory image of the program. This 
          segment type may not occur more than once in a file.
          Moreover, it may occur only if the program header table 
          is part of the memory image of the program. If it is
          present, it must precede any loadable segment entry.

Hence, if a PT_PHDR segment is present in a binary it must also be part of
a PT_LOAD segment, so that it gets loaded into the runtime memory image of
the executable.

The layout above breaks this requirement by placing the PT_PHDR segment at
0x1034 but starting the first load segment at 0x80000.

> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   PHDR           0x000034 0x0007f034 0x0007f034 0x000a0 0x000a0 R E 0x4
>   INTERP         0x001000 0x00080000 0x00080000 0x00013 0x00013 R   0x1
>       [Requesting program interpreter: /usr/lib/libc.so.1]
>   LOAD           0x000000 0x0007f000 0x0007f000 0x01408 0x01408 R E 0x1000
>   LOAD           0x002000 0x00081000 0x00081000 0x00804 0x00c00 RW  0x1000
>   DYNAMIC        0x002000 0x00081000 0x00081000 0x00078 0x00078 RW  0x4

This is the linker's attempt to resolve the problem, by moving the start
address of the first LOAD segment down to 0x7f0000.  (The linker cannot just
create another LOAD segment just for the program headers as there is code
inside the linker that will discard any LOAD segments that do not have any
sections mapped into them).


> This places the first LOAD segment below the location set with the linker 
> script, which I fear may affect run-time interpretation.  The test case 
> has the addresses expected spelled out in full for a reason I suppose.

True.  You could fix the problem by adding --no-dynamic-linker to the test's
linker command line, since then the linker will not create PHDR or INTERP
segments, and so the LOAD segments will have the expected addresses.  I doubt
if you want this solution however as I assume that the whole point of this
test is to create a working dynamic executable.

The correct solution, I think, is to define the program headers in the linker
script, so that their placement can be specifically controlled by the user.
See the vxworks1.ld script attached, which does this.

But...this causes a problem for the "VxWorks executable test 2 (static)" test
as now there is a DYNAMIC segment in the program headers but no .dynamic
section in the section headers, and readelf complains.  *sigh*  So you need
a second linker script, just for static executables.

So the result is the attached patch, which I think resolves the problem.  (For
MIPS based vxworks targets anyway.  It looks like similar changes may be needed
for other vxworks architectures).  What do you think ?

Cheers
 Nick

Attachment: vxworks1.ld
Description: Text document

diff --git a/ld/testsuite/ld-mips-elf/mips-elf.exp b/ld/testsuite/ld-mips-elf/mips-elf.exp
index fd79e80..b12d1b7 100644
--- a/ld/testsuite/ld-mips-elf/mips-elf.exp
+++ b/ld/testsuite/ld-mips-elf/mips-elf.exp
@@ -37,7 +37,7 @@ if {[istarget "mips*-*-vxworks"]} {
 	 {{readelf --segments vxworks2.sd}}
 	 "vxworks2"}
 	{"VxWorks executable test 2 (static)"
-	 "-Tvxworks1.ld" ""
+	 "-Tvxworks2-static.ld" ""
 	 "-mips2" {vxworks2.s}
 	 {{readelf --segments vxworks2-static.sd}}
 	 "vxworks2"}
diff --git a/ld/testsuite/ld-mips-elf/vxworks1.ld b/ld/testsuite/ld-mips-elf/vxworks1.ld
index d9f8621..428564c 100644
--- a/ld/testsuite/ld-mips-elf/vxworks1.ld
+++ b/ld/testsuite/ld-mips-elf/vxworks1.ld
@@ -1,8 +1,19 @@
+PHDRS
+{
+  headers      PT_PHDR PHDRS;
+  header_load  PT_LOAD PHDRS;
+  interp       PT_INTERP;
+  code         PT_LOAD;
+  data         PT_LOAD;
+  dynamic      PT_DYNAMIC;
+}
+
 SECTIONS
 {
   . = 0x80000;
-  .interp : { *(.interp) }
-  .hash : { *(.hash) }
+  . = . + SIZEOF_HEADERS;
+  .interp : { *(.interp) } :interp :headers :header_load
+  .hash : { *(.hash) } :code
   .dynsym : { *(.dynsym) }
   .dynstr : { *(.dynstr) }
 
@@ -17,10 +28,10 @@ SECTIONS
   .text : { *(.text) }
 
   . = ALIGN (0x1000);
-  .dynamic : { *(.dynamic) }
+  .dynamic : { *(.dynamic) } :data :dynamic
 
   . = ALIGN (0x400);
-  .got : { *(.got.plt) *(.got) }
+  .got : { *(.got.plt) *(.got) } :data
 
   . = ALIGN (0x400);
   .data : { *(.data) }
@@ -28,5 +39,6 @@ SECTIONS
   . = ALIGN (0x400);
   .bss : { *(.bss) *(.dynbss) }
 
+
   /DISCARD/ : { *(.reginfo) *(.MIPS.abiflags) }
 }
diff --git a/ld/testsuite/ld-mips-elf/vxworks2.sd b/ld/testsuite/ld-mips-elf/vxworks2.sd
index 5ff87d3..01301ba 100644
--- a/ld/testsuite/ld-mips-elf/vxworks2.sd
+++ b/ld/testsuite/ld-mips-elf/vxworks2.sd
@@ -6,8 +6,8 @@ Program Headers:
   Type .*
   PHDR .*
 #...
-  LOAD .* 0x00080000 0x00080000 .* R E 0x1000
-  LOAD .* 0x00081000 0x00081000 .* RW  0x1000
+  LOAD .* 0x00080... 0x00080... .* R E 0x1000
+  LOAD .* 0x00081... 0x00081... .* RW  0x1000
   DYNAMIC .*
 
 #...
--- /dev/null	2016-12-08 07:35:33.885870158 +0000
+++ ld/testsuite/ld-mips-elf/vxworks2-static.ld	2016-12-08 12:04:17.091239541 +0000
@@ -0,0 +1,25 @@
+SECTIONS
+{
+  . = 0x80000;
+  .hash : { *(.hash) }
+
+  . = ALIGN (0x400);
+  .rela.plt : { *(.rela.plt) }
+
+  . = ALIGN (0x400);
+  .plt : { *(.plt) }
+
+  . = ALIGN (0x400);
+  .text : { *(.text) }
+
+  . = ALIGN (0x1000);
+  .got : { *(.got.plt) *(.got) }
+
+  . = ALIGN (0x400);
+  .data : { *(.data) }
+
+  . = ALIGN (0x400);
+  .bss : { *(.bss) *(.dynbss) }
+
+  /DISCARD/ : { *(.reginfo) *(.MIPS.abiflags) }
+}