[PATCH v2] x86: Restore PC16 relocation overflow check

[H.J. Lu]

On Fri, May 28, 2021 at 8:52 AM Michael Matz <matz@suse.de> wrote:
>
> Hello,
>
> On Fri, 28 May 2021, Jan Beulich via Binutils wrote:
>
> > On 28.05.2021 14:22, H.J. Lu wrote:
> > > The x86-64 psABI has
> > >
> > > ---
> > > A program or object file using R_X86_64_8, R_X86_64_16, R_X86_64_PC16
> > > or R_X86_64_PC8 relocations is not conformant to this ABI, these
> > > relocations are only added for documentation purposes.
> > > ---
> > >
> > > Since x86 PC16 relocations were intended for 16-bit programs in an ELF32
> > > or ELF64 container, PC16 relocation should wrap-around in 16-bit address
> > > space.
> >
> > At the risk of stating the obvious - this wrapping around is not
> > expressed correctly by complain_overflow_bitfield. I simply can't
> > see how, in a 32- or 64-bit container, 16-bit code could ever be
> > linked correctly, suitably detecting overflow where it would
> > cause a problem at runtime. This is only possible when the linker
> > knows 16-bit segment boundaries.
> >
> > Therefore I see only two reasonable modes of operation for the
> > linker:
> > 1) Treat PC16 analogously to PC8/PC32/PC64.
>
> I think this is the right course of action.  PC16 should check that the
> signed 16bit value is the same as the computed 64 bit (or 32bit on ELF32)
> value that's supposed to go in there.
>
> The PC16 (or any of the other relocations named PC*) is not the place to
> check for the funny truncation of rIP that some instructions are doing.
> PC* has uses outside of instructions, and even within instructions the
> "normal" meaning makes sense and reflects reality the most (i.e. no rIP
> truncation).
>
> If we _really_ wanted that linkers be able to check that rIP truncations
> is correct then that would need a new relocation.  I will remark that
> checking such truncation would require the linker to know where the final
> linked runtime address is, and that in the wild, for loaders and BIOSes
> writing into ELF files for merely containing byte blobs to be later
> extracted by objcopy with the assumption that that blob then is magically
> loaded at "the right address", these final load addresses are often _not_
> written into the linker scripts correctly.  So, in the end, even if we
> were to introduce such new relocation with the rIP truncation checking, it
> often would have to be disabled anyway in practice because the checks then
> would trigger.
>
> > 2) Don't check PC16 (and then also PC8, 8, and 16) for overflow
> >    at all.
> > The default ought to be 1, while 2 ought to be enabled explicitly
> > (and at the programmer's own risk) for code caring to link sizable
> > amounts of 16-bit code (i.e. when the overflow checking gets in
> > the way).
> > This said, with the psABI saying what it says, I could see one
> > taking the position of taking "don't check overflows at all" as
> > a legitimate mode to always operate in. But then again this
> > ought to apply to all four non-conformant reloc types at the
> > same time.
>
> We can change the psABI.  But changes should make sense in the abstract as
> well as being demanded by reality.  I think we have good cause to make
> PC16 conforming now, but I don't think we have good cause to make its
> semantic anything else from the obvious.  FWIW, my patch would be
>   https://gitlab.com/x86-psABIs/x86-64-ABI/-/merge_requests/22
>
> Do you also want _8 and _16 (zero-extended I guess)?  _PC8
> (sign-extended)?
>

Since there are 16-bit applications which expect PC16 behavior in
binutils 2.36, we can't change it without breaking them.  We can
update psABI to add a new PC16 relocation and leave the current
PC16 alone.


-- 
H.J.