Re: Preventing preemption of 'protected' symbols in GNU ld 2.26 [aka should we revert the fix for 65248]

[Jeff Law <law@redhat.com>]

On 03/30/2016 08:34 AM, Alan Modra wrote:
> OK, I'll try to list the technical issues re. pr65248 "Copy relocation
> against protected symbol doesn't work".
>
> Given a shared library that defines a variable, and a non-PIC
> executable that references that variable, the linker makes a duplicate
> of the variable in the executable .dynbss section and arranges to have
> the copy initialized by the dynamic loader with a copy relocation.
> .dynbss is a linker created section that becomes part of the
> executable bss segment.  The idea is that at run-time both the
> executable and the shared library will use the executable copy of the
> variable.  It's a rather ancient linker hack to avoid dynamic text
> relocations, invented well before symbol visibility.
Right.  Essentially we have to ensure there's one and only one copy of 
the variable and that if the shared library and executable take the 
address of the variable that the addresses compare equal.   Pretty 
standard stuff.


>
> The problem with protected visibility variables in shared libraries is
> that the shared library continues to access its own copy.  So we have
> two copies of a variable where the source only contained one variable
> definition.  (It's not really the copy relocation as such that causes
> a problem, but the fact that we have a duplicate in .dynbss.)
Which is clearly bad/wrong.  But isn't this the state we were in prior 
to HJ's change?

ie, the DSO would use non-GOT sequences to access its private copy and 
the main executable would reference the duplicate in .dynbss?  That led 
to two definitions.

After HJ's change, the DSO references via the GOT and we have a single 
instance?  Right?

The problem in that case is we've essentially allowed preemption of a 
symbol that was not supposed to be preemptable.  Worse yet, all accesses 
are going through the GOT in the DSO, which is almost certainly a 
performance penalty.

ISTM that there is a fundamental incompatibility between protected 
visibility and these copy relocations.

>
> 1) Have the linker emit an error, suggesting that the executable needs
> to be PIC to access a protected visibility variable in a shared
> library.  Pass the pain on to users.
Not ideal, but at least we'd get a nice compile/link time error rather 
than silently doing the wrong thing in one case or the other.


>
> 2) Emit dynamic text relocations for non-PIC executable access to
> protected visibility variables defined in shared libraries.  Makes
> executable pages with text relocations non-shared between processes.
> (But can be shared again after relocation if two processes have the
> same layout.)
Yea, but, ewww.


>
> 3) Teach the linker to edit non-PIC to PIC.  I've done this for ppc32,
> but it's not very easy for x86 due to the fact that you can't find the
> start of an instruction from a relocated field.  It also results in
> slow code, since PIC sequences are usually larger than non-PIC,
> requiring a branch into a patch area and a branch back.
I'm not a fan of all the rewriting we currently do in the linker; adding 
more to deal with this case doesn't seem wise.  And we'd have to 
replicate it in every target that supports copy relocations.  ugh.

>
> 4) Have the compiler always emit PIC for external variable access.
> The .dynbss hack could then die a natural death.  Coupled with linker
> editing of PIC to non-PIC, you'd get correct and reasonably fast code
> with perhaps a few nops.  Again, x86 has difficulty due to variable
> length insns, but this could be solved with marker relocs.
Maybe long term.  With the caveats about rewriting above, of course.

It seems like we'd want to stage in the linker changes first, then the 
compiler changes.


>
> 5) HJ's solution.  Make the compiler emit the same code for protected
> visibility variables in shared libraries as it does for default
> visibility variables.  ie. shared libraries access their protected
> visibility variables via the GOT, which allows the .dynbss hack to
> work.  Also, modify the dynamic loader to ensure protected visibility
> semantics are enforced.  (Consider the case of shared lib A that
> defines protected visibility var V, and shared lib B that also defines
> protected visibility var V, and an executable that references V.
> Assuming A is before B in breadth-first order, then the executable and
> A should use the .dynbss copy of V, but B should use its own V!)  I've
> got to hand it to HJ, I think he has all this working properly to
> satisfy the semantics of protected visibility.
I'm even less inclined to dump this responsibility onto the dynamic loader.

>
> However, shared library access of protected visibility variables is of
> course no quicker than default visibility variables.  Which defeats
> the main reason to use protected visibility variables in shared
> libraries!  Note also that it unnecessarily slows down shared library
> code using protected visibility variables that are not accessed by the
> executable (but perhaps accessed from some other shared library).
Right.

>
> Also, I don't believe there is anything to prevent a newly compiled
> program from running with an old glibc, which would mean protected
> visibility semantics are broken on such a system.  To be fair, they
> were probably broken on such a system before the linker complained
> about non-PIC protected visibility access, anyway.
RIght.


So in the immediate term, if we drop the problem 65248 patch, we're back 
in a state where the DSO and the executable can have two different views 
of certain objects.  In which case we really need a solution in place to 
flag that as an error, which is #1 in your list of mitigations.

Right?

jeff