Re: [PATCH v2] elf: Remove ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA

[Szabolcs Nagy <szabolcs.nagy@arm.com>]

The 06/01/2022 00:34, Fangrui Song wrote:
> On 2022-06-01, Szabolcs Nagy wrote:
> > The 05/31/2022 21:50, Fangrui Song wrote:
> > > If an executable has copy relocations for extern protected data, that
> > > can only work if the library containing the definition is built with
> > > assumptions (a) the compiler emits GOT-generating relocations (b) the
> > > linker produces R_*_GLOB_DAT instead of R_*_RELATIVE.  Otherwise the
> > > library uses its own definition directly and the executable accesses a
> > > stale copy.  Note: the GOT relocations defeat the purpose of protected
> > > visibility as an optimization, but allow rtld to make the executable and
> > > library use the same copy when copy relocations are present, but it
> > > turns out this never worked perfectly.
> > > 
> > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA has strange semantics when both
> > > a.so and b.so define protected var and the executable copy relocates
> > > var: b.so accesses its own copy even with GLOB_DAT.  The behavior change
> > > is from commit 62da1e3b00b51383ffa7efc89d8addda0502e107 (x86) and then
> > > copied to nios2 (ae5eae7cfc9c4a8297ff82ec6b794faca1976ecc) and arc
> > > (0e7d930c4c11de896fe807f67fa1eb756c9c1e05).
> > > 
> > > Without ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, b.so accesses the copy
> > > relocated data like a.so.
> > > 
> > > It's extremely unlikely anyone relies on the
> > > ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA behavior, so let's remove it.
> > > 
> > > --
> > > Changes from v1:
> > > * Reword commit message as suggested by Szabolcs Nagy
> > 
> > Please document the interposition change or fix it.
> 
> Does this paragraph count as an explanation?
> "Without ELF_RTYPE_CLASS_EXTERN_PROTECTED_DATA, b.so accesses the copy relocated data like a.so."
> 
> Or, what other documentation on the interposition behavior do you expect?

the current commit message implies there is only behaviour
change in presence of copy relocations.

but that's not true: the exe (or a shared lib) can now
interpose a protected variable by another one with the
same name (and no copy relocs).

i originally thought that warning/rejecting copy relocs in
ld is enough to get sane behaviour for protected symbols,
but when multiple definitions are present the behaviour
will depend on ld's decision to use GOT or not.

i think the removed logic tried to ensure that GOT relocs
resolve to the definition within the same shared lib for
protected data. (i.e. ld's decision does not matter.)

if we want to allow ld to not use GOT then i think we need
to keep the logic that makes GOT behave consistently with
that future.

if we instead want to simplify ld.so and handle GOT for
protected symbols more efficiently, then that should be
explained in the commit clearly: why do we choose the
"symbol lookup returns a unique address" over "protected
symbol binds locally" rule when there are multiple
definitions? (maybe because this case should not be
relied on but then we should diagnose it, etc)