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

[Alan Modra]

On Fri, Dec 09, 2016 at 12:17:31AM +0000, Maciej W. Rozycki wrote:
> On Thu, 8 Dec 2016, Alan Modra wrote:
> 
> > > 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 gABI says:
> > 
> > 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.
> > 
> > The above clearly violates this part of the spec because PT_PHDR is
> > present yet is not part of the memory image.
> 
>  Well, TBH I think it's all but clear, given that nowhere in the ELF gABI 
> that I can find there is an actual definition of what "the memory image" 
> is.

I'm not going to argue this point except to note that all specs I've
ever read rely on some background knowledge and require
interpretation.  Those that try to specify endless detail might
satisfy lawyers, but often become so unwieldy that they aren't as
useful as they should be to engineers.

> > Nick's patch forced the first PT_LOAD to cover the program headers.  I
> > think an equally valid and somewhat better fix would have been to not
> > emit PT_PHDR when no PT_LOAD header covers the program headers.  The
> > reason I say that is because PT_PHDR is optional.  A loader can read
> > the program headers itself from file using info in the ELF header.
> 
>  There may be no loader available to load the program headers or the ELF 
> file header if a bare-metal ELF image has been externally loaded according 
> to some interpretation of the headers, and the binary wants to access its 
> own structures at run time.  Existing environments may rely on our current 
> semantics even if it is not strictly compliant.

This is true but I was arguing about the validity of the ELF file.  I
mentioned a loader only to say what might be done.

We do know that on reasonably up to date Linux with glibc, that
linking a simple program with a script that leaves no room for headers
results in a segfault before Nick's patch, but runs after Nick's
patch.  My suggestion to remove PHDR (patch attached) results in the
same ld.so segfault.  It would certainly be useful to know how vxworks
behaves with similar tests.

We've also found that Nick's patch broke linux kernel builds, which is
why I was exploring alternative fixes.

-- 
Alan Modra
Australia Development Lab, IBM
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dropphdr.diff
Type: text/x-diff
Size: 1572 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20161209/d4575cd0/attachment.bin>