Re: [PATCH] [BZ #18422] elf/tst-audit tests fail without PLT entries

["Carlos O'Donell" <carlos at redhat dot com>]

On 05/24/2015 08:45 AM, H.J. Lu wrote:
> On Sat, May 23, 2015 at 8:37 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 05/23/2015 09:14 AM, H.J. Lu wrote:
>>> Since ld.so is built with -z now, there are no PLT relocations and this
>>> calloc won't be used:
>>
>> Which is bad. We always want these functions to be interposable by all
>> of the analysis tools that want and need to track memory allocations.
>> Thus it is not just the test that matters.
>>
> ...
>>> 00222fc4  00001506 R_X86_64_GLOB_DAT 00018300   malloc@@GLIBC_2.16 + 0
>>> 00222fcc  00000d06 R_X86_64_GLOB_DAT 00018310   calloc@@GLIBC_2.16 + 0
>>> 00222fd4  00000506 R_X86_64_GLOB_DAT 000184a0   realloc@@GLIBC_2.16 + 0
>>> 00222fdc  00000706 R_X86_64_GLOB_DAT 002239a0   _r_debug@@GLIBC_2.16 + 0
>>> 00222fe4  00000406 R_X86_64_GLOB_DAT 00018340   free@@GLIBC_2.16 + 0
>>
>> Doesn't this also cause check-localplt to fail for ld.so given
>> that calloc and others are no longer R_X86_64_JUMP_SLOT?
> 
> Yes.

Good.

>>> Assuming we do want to keep PLT relocations in ld.so so that malloc
>>> functions in ld.so can be overridden, ld.so should be built with -z now.
>>> There is no reason to build ld.so with -z now since ld.so is the one
>>> doing BIND_NOW.  The only thing we get with -z now on ld.so is DT tag:
>>>
>>>  0x0000000000000018 (BIND_NOW)
>>>  0x000000006ffffffb (FLAGS_1)            Flags: NOW
>>>
>>> This patch removes -Wl,-z,now from ld.so build.
>>>
>>> OK for master?
>>
>> No. I'd like to see more discussion on this.
>>
>> I don't see any other way forward, and I agree that DT_BIND_NOW seems
>> a bit silly for the linker since it itself is the component responsible
>> for that binding.
>>
>> My worry is that the missing DT tag is going to have security implications.
>> I'm including several other distro people on the TO.
>>
>> The first thing I'll have to explain is "Why doesn't ld.so meet full RELRO?"
>> Since full RELRO requires DT_BIND_NOW + RO segments. Does this mean ld.so
>> with your patch will by lazily bound and not mark it's own PLT immediately RO?
>> That seems wrong.
> 
> My patch just doesn't use -z now to build ld.so when --enable-bind-now is
> used and ld.so always gets PLT with/without  --enable-bind-now.  It is the
> same as before when building glibc with the older ld.  The only difference now
> is we no long generate DT_BIND_NOW even when --enable-bind-now is used.
> I don't believe DT_BIND_NOW should make a difference on ld.so.  If it is, it is
> a separate bug and we should fix it.

The patch is a hack.

- ld.so should continue to have DT_BIND_NOW since we are requesting that
  any symbol binding done in ld.so be done at startup and not lazily, and
  generic security tools should be able to check all binaries including
  ld.so to see that this is enabled and requested without making special
  exceptions.

- ld.so can have no PLT if that is what happens as a result of a dynamic
  loader optimization triggered by -Wl,-z,now. That seems like a useful
  optimization which would be disabled by your suggested patch?

- ld.so should still support ELF symbol interposition, which is orthogonal
  to lazy binding. Lazy binding and ELF symbol interposition should not be
  conflated.

>> The notion of -z,now means "Bind symbols now, not lazily", but that doesn't
>> give you enough information to elide the PLT and use the GOT directly since
>> the interposition is still useful and relied upon semantic in ELF.
> 
> You still get the interposition with GOT, just not lazy bind, which needs PLT
> relocation.

Good point.

> GCC 6 even has a new option, -fno-plt, to avoid PLT:
> 
> https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00231.html

GCC has many options that must be used with care and foresight.
 
> ld.so is a special case and needs PLT for the interposition unless
> ld.so re-applies GOT relocations on itself after all modules are loaded.

And that is the *real* bug. Please fix that instead of removing -Wl,-z,now.
It should be a net-zero performance chance since either you are doing
relocations for the GOT and then the PLT, or the GOT twice?

I think it is important we continue to build ld.so with -Wl,-z,now, both
for the optimizations, and for consistency among our security tooling.

I'm happy to hear input from others.

Have you checked to see how your patch to remove the PLT impacts analysis
tooling like Asan and Valgrind?

Cheers,
Carlos.