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

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

On 05/25/2015 02:54 PM, H.J. Lu wrote:
> On Sun, May 24, 2015 at 7:06 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> > 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 did a closer look.  Nothing is wrong, except for that tst-audit2 expects
> certain order in ld.so.  With JUMP_SLOT relocation, the GOTPLT entry of
> calloc is update to calloc defined in tst-audit2:

Thank you very much for looking into this.

I'm glad to see that it does work, but that it is an ordering issue
between JUMP_SLOT and GOTPLT processing.

I have one question, please see below.

> 
> (gdb) bt
> #0  0xf7fe56bd in elf_machine_rel (reloc=<optimized out>,
>     skip_ifunc=<optimized out>, reloc_addr_arg=<optimized out>,
>     version=<optimized out>, sym=<optimized out>, map=<optimized out>)
>     at ../sysdeps/i386/dl-machine.h:329
> #1  elf_dynamic_do_Rel (skip_ifunc=<optimized out>, lazy=<optimized out>,
>     nrelative=<optimized out>, relsize=<optimized out>,
>     reladdr=<optimized out>, map=<optimized out>) at do-rel.h:137
> #2  _dl_relocate_object (scope=0xf7ffdab8, reloc_mode=reloc_mode@entry=0,
>     consider_profiling=1, consider_profiling@entry=0) at dl-reloc.c:258
> #3  0xf7fdc648 in dl_main (phdr=<optimized out>, phnum=<optimized out>,
>     user_entry=0xffffcf1c, auxv=0xffffd0a8) at rtld.c:2133
> #4  0xf7ff0de7 in _dl_sysdep_start (
>     start_argptr=start_argptr@entry=0xffffcfb0,
>     dl_main=dl_main@entry=0xf7fda6f0 <dl_main>) at ../elf/dl-sysdep.c:249
> #5  0xf7fddd05 in _dl_start_final (arg=0xffffcfb0) at rtld.c:308
> #6  _dl_start (arg=0xffffcfb0) at rtld.c:414
> #7  0xf7fd9a87 in _start ()
>    from /export/build/gnu/glibc-32bit/build-i686-linux/elf/ld.so
> (gdb)
> 
> and then calloc is called:
> 
> (gdb) c
> Continuing.
> 
> Breakpoint 4, calloc (n=n@entry=20, m=4) at tst-audit2.c:18
> 18 {
> (gdb) bt
> #0  calloc (n=n@entry=20, m=4) at tst-audit2.c:18
> #1  0xf7fe668d in _dl_relocate_object (scope=0xf7ffdab8,
>     reloc_mode=reloc_mode@entry=0, consider_profiling=1,
>     consider_profiling@entry=0) at dl-reloc.c:272
> #2  0xf7fdc648 in dl_main (phdr=<optimized out>, phnum=<optimized out>,
>     user_entry=0xffffcf1c, auxv=0xffffd0a8) at rtld.c:2133
> #3  0xf7ff0de7 in _dl_sysdep_start (
>     start_argptr=start_argptr@entry=0xffffcfb0,
>     dl_main=dl_main@entry=0xf7fda6f0 <dl_main>) at ../elf/dl-sysdep.c:249
> #4  0xf7fddd05 in _dl_start_final (arg=0xffffcfb0) at rtld.c:308
> #5  _dl_start (arg=0xffffcfb0) at rtld.c:414
> #6  0xf7fd9a87 in _start ()
>    from /export/build/gnu/glibc-32bit/build-i686-linux/elf/ld.so
> (gdb)
> 
> With GLOB_DAT relocation, calloc in ld.so is called first:

This results in one more calloc from dl-minimal.c which has to be tracked
and not freed.

> (gdb) bt
> #0  calloc (nmemb=20, size=4) at dl-minimal.c:102
> #1  0xf7fe665d in _dl_relocate_object (scope=0xf7fcfb20, reloc_mode=1,
>     consider_profiling=1) at dl-reloc.c:272
> #2  0xf7fdc500 in dl_main (phdr=<optimized out>, phnum=<optimized out>,
>     user_entry=0xffffcf0c, auxv=0xffffd098) at rtld.c:2074
> #3  0xf7ff0db7 in _dl_sysdep_start (
>     start_argptr=start_argptr@entry=0xffffcfa0,
>     dl_main=dl_main@entry=0xf7fda6c0 <dl_main>) at ../elf/dl-sysdep.c:249
> #4  0xf7fddcd5 in _dl_start_final (arg=0xffffcfa0) at rtld.c:308
> #5  _dl_start (arg=0xffffcfa0) at rtld.c:414
> #6  0xf7fd9a57 in _start ()
>    from /export/build/gnu/glibc-32bit-test/build-i686-linux/elf/ld.so
> (gdb)
> 
> and then the GOT entry of calloc is updated:

OK.

> (gdb) bt
> #0  0xf7fe568d in elf_machine_rel (reloc=<optimized out>,
>     skip_ifunc=<optimized out>, reloc_addr_arg=<optimized out>,
>     version=<optimized out>, sym=<optimized out>, map=<optimized out>)
>     at ../sysdeps/i386/dl-machine.h:329
> #1  elf_dynamic_do_Rel (skip_ifunc=<optimized out>, lazy=<optimized out>,
>     nrelative=<optimized out>, relsize=<optimized out>,
>     reladdr=<optimized out>, map=<optimized out>) at do-rel.h:137
> #2  _dl_relocate_object (scope=0xf7ffdab8, reloc_mode=reloc_mode@entry=0,
>     consider_profiling=1, consider_profiling@entry=0) at dl-reloc.c:258
> #3  0xf7fdc618 in dl_main (phdr=<optimized out>, phnum=<optimized out>,
>     user_entry=0xffffcf0c, auxv=0xffffd098) at rtld.c:2133
> #4  0xf7ff0db7 in _dl_sysdep_start (
>     start_argptr=start_argptr@entry=0xffffcfa0,
>     dl_main=dl_main@entry=0xf7fda6c0 <dl_main>) at ../elf/dl-sysdep.c:249
> #5  0xf7fddcd5 in _dl_start_final (arg=0xffffcfa0) at rtld.c:308
> #6  _dl_start (arg=0xffffcfa0) at rtld.c:414
> #7  0xf7fd9a57 in _start ()
>    from /export/build/gnu/glibc-32bit-test/build-i686-linux/elf/ld.so
> (gdb)
> 
> After, calloc isn't called and magic in tst-audit2 isn't updated.  Both
> orders are correct.  Here is a patch to make sure that calloc in tst-audit2.c is
> called at least once from ld.so.

In the past before your ld optimization the JUMP_SLOT would have been
relocated to libc.so.6's version of calloc, or the test version of calloc,
correct?

The change from JUMP_SLOT -> GLOB_DAT for calloc means that we call calloc
one more time using the dl-minimal.c implementation. This seems dangerous to
me since in ld.so we take extreme caution not to attempt to free this result
because dl-minimal.c doesn't support freeing anything but the most recent
allocation. We also don't want to call free() from libc.so.6 with data that
was calloc'd from the dl-minimal.c implementation.

Are we certain that this additional calloc, now using dl-minimal.c, isn't
going to cause problems? When is it freed? What implementation of free is used?
 
>> > 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?
>> >
> No, i didn't test them nor I expect any problems.

Well, right away if we have one more call of calloc to dl-minimal.c, that's
another allocation the tool can't track. It's not huegely problematic because
they already can't track all the early allocations via dl-minimal.c.

In summary:
- My next worry is about free of calloc'd data that is now using dl-minimal.c

If we can answer that question, then I think this patch to adjust tst-audit2.c
is the best solution.

Cheers,
Carlos.