This is the mail archive of the mailing list for the binutils 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: [binutils-gdb] Fix the linker so that it will not silently generate ELF binaries with invalid program headers. Fix

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/]
> >   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:
>     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" 

 It might be possible to prove it by contradiction from the "Base Address" 
section, where it is specified that it is `p_vaddr' of the lowest PT_LOAD 
segment that is used to calculate the base address or the relocation 
amount for the whole image, that no other segment type is a part of the 
memory image without a corresponding PT_LOAD shadow.  But I wouldn't be 
that convinced about it as arguably the definition does not actually 
*require* that there is no mapped area below the lowest PT_LOAD segment -- 
it merely specifies the calculation algorithm which needs a point of 
reference, and which may well apply to any other segment types, even if 
mapped below the lowest PT_LOAD segment.

 It may also be important to note that based on the change bars in the 
latest 4.1 PDF version of the gABI it can be inferred that the "Base 
Address" section was updated relatively late in the course of the 
document's processing and it cannot be completely ruled out that parts 
elsewhere were missed out and not revised accordingly.

 Contrariwise, it is specified that `p_memsz' gives "the number of bytes 
in the memory image of the segment" without limiting it to PT_LOAD 
segments, so one could argue all segments whose this value is non-zero 
comprise the memory image, regardless of whether they are shadowed by a 
PT_LOAD segment or not.

 Intuitively I think that you are correct in that it was the intent of the 
gABI authors that only PT_LOAD segments comprise the memory image, but I 
also think that intuition and guessing the intent is not the right way of 
interpreting a technical specification, as intuition varies among people 
and some implementers may have understood the document differently and 
still be formally right in their interpretation.

> 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.

 I think my inability to actually decide which way is correct for MIPS 
VxWorks (for which I have no environment available to verify) just shows 
that changing semantics which we had since forever just to match a piece 
of (virtual) paper from 20 years ago may not be the greatest idea, and 
certainly needs a lot of care and consideration if it is given a go-ahead.  
There may well be environments out there which have relied on our 
technically broken semantics for an equally long period of time and we may 
well have thus established the de-facto standard.  There may be parts of 
the environments we have no control over and which may be difficult to 
correct to match the written standard.

 I mean if there is a case of actual breakage that our implementation 
causes, as it has been in this case, then we certainly do ought to address 
it, one way or another.  So we found a way to fix Linux, but are we 
confident it didn't break anything else?  Maybe we need a different fix.  
Or at least a linker script or command-line option to chicken out and 
resort to the old semantics?

 But perhaps I'm just being overly cautious.  So is anyone out there who 
could actually regression-test say GCC with current binutils on MIPS 
VxWorks, or run whatever is usually done for toolchain validation with 
that target?  After all what matters is whether the environment continues 
operating correctly and not whether a cryptic test case from 10 years ago 
passes unchanged or not.  It might be that the test case is too strict or 
genuinely wrong in some way.


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