From patchwork Mon May 25 18:54:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 6915 Received: (qmail 64621 invoked by alias); 25 May 2015 18:54:57 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 64609 invoked by uid 89); 25 May 2015 18:54:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.1 required=5.0 tests=AWL, BAYES_40, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-oi0-f54.google.com MIME-Version: 1.0 X-Received: by 10.202.62.212 with SMTP id l203mr17331061oia.67.1432580092696; Mon, 25 May 2015 11:54:52 -0700 (PDT) In-Reply-To: <55628397.7050202@redhat.com> References: <20150523131408.GA18203@gmail.com> <5561475D.9020406@redhat.com> <55628397.7050202@redhat.com> Date: Mon, 25 May 2015 11:54:52 -0700 Message-ID: Subject: Re: [PATCH] [BZ #18422] elf/tst-audit tests fail without PLT entries From: "H.J. Lu" To: "Carlos O'Donell" Cc: GNU C Library , Florian Weimer , Adam Conrad , Aurelien Jarno , Andreas Schwab , Jeff Law On Sun, May 24, 2015 at 7:06 PM, Carlos O'Donell 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: (gdb) bt #0 0xf7fe56bd in elf_machine_rel (reloc=, skip_ifunc=, reloc_addr_arg=, version=, sym=, map=) at ../sysdeps/i386/dl-machine.h:329 #1 elf_dynamic_do_Rel (skip_ifunc=, lazy=, nrelative=, relsize=, reladdr=, map=) 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=, phnum=, 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 ) 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=, phnum=, 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 ) 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: (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=, phnum=, 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 ) 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: (gdb) bt #0 0xf7fe568d in elf_machine_rel (reloc=, skip_ifunc=, reloc_addr_arg=, version=, sym=, map=) at ../sysdeps/i386/dl-machine.h:329 #1 elf_dynamic_do_Rel (skip_ifunc=, lazy=, nrelative=, relsize=, reladdr=, map=) 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=, phnum=, 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 ) 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. > 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. From 31d9cfee87edf3acf95640db262b950e3b0514f3 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Mon, 25 May 2015 11:30:57 -0700 Subject: [PATCH] Make sure that calloc is called at least once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PLT relocations aren't required when -z now used. Linker on master with: commit 25070364b0ce33eed46aa5d78ebebbec6accec7e Author: H.J. Lu Date: Sat May 16 07:00:21 2015 -0700 Don't generate PLT relocations for now binding There is no need for PLT relocations with -z now. We can use GOT relocations, which take less space, instead and replace 16-byte .plt entres with 8-byte .plt.got entries. bfd/ * elf32-i386.c (elf_i386_check_relocs): Create .plt.got section for now binding. (elf_i386_allocate_dynrelocs): Use .plt.got section for now binding. * elf64-x86-64.c (elf_x86_64_check_relocs): Create .plt.got section for now binding. (elf_x86_64_allocate_dynrelocs): Use .plt.got section for now binding. won't generate PLT relocations with -z now. elf/tst-audit2.c expect certain order of execution in ld.so.  With PLT relocations, the GOTPLT entry of calloc is update to calloc defined in tst-audit2: (gdb) bt     skip_ifunc=, reloc_addr_arg=,     version=, sym=, map=)     at ../sysdeps/i386/dl-machine.h:329 out>,     nrelative=, relsize=,     reladdr=, map=) at do-rel.h:137 reloc_mode=reloc_mode@entry=0,     consider_profiling=1, consider_profiling@entry=0) at dl-reloc.c:258     user_entry=0xffffcf1c, auxv=0xffffd0a8) at rtld.c:2133     start_argptr=start_argptr@entry=0xffffcfb0,     dl_main=dl_main@entry=0xf7fda6f0 ) at ../elf/dl-sysdep.c:249    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     reloc_mode=reloc_mode@entry=0, consider_profiling=1,     consider_profiling@entry=0) at dl-reloc.c:272     user_entry=0xffffcf1c, auxv=0xffffd0a8) at rtld.c:2133     start_argptr=start_argptr@entry=0xffffcfb0,     dl_main=dl_main@entry=0xf7fda6f0 ) at ../elf/dl-sysdep.c:249    from /export/build/gnu/glibc-32bit/build-i686-linux/elf/ld.so (gdb) With GOT relocation, calloc in ld.so is called first: (gdb) bt     consider_profiling=1) at dl-reloc.c:272     user_entry=0xffffcf0c, auxv=0xffffd098) at rtld.c:2074     start_argptr=start_argptr@entry=0xffffcfa0,     dl_main=dl_main@entry=0xf7fda6c0 ) at ../elf/dl-sysdep.c:249    from /export/build/gnu/glibc-32bit-test/build-i686-linux/elf/ld.so (gdb) and then the GOT entry of calloc is updated: (gdb) bt     skip_ifunc=, reloc_addr_arg=,     version=, sym=, map=)     at ../sysdeps/i386/dl-machine.h:329 out>,     nrelative=, relsize=,     reladdr=, map=) at do-rel.h:137 reloc_mode=reloc_mode@entry=0,     consider_profiling=1, consider_profiling@entry=0) at dl-reloc.c:258     user_entry=0xffffcf0c, auxv=0xffffd098) at rtld.c:2133     start_argptr=start_argptr@entry=0xffffcfa0,     dl_main=dl_main@entry=0xf7fda6c0 ) at ../elf/dl-sysdep.c:249    from /export/build/gnu/glibc-32bit-test/build-i686-linux/elf/ld.so (gdb) After that, since calloc isn't called from ld.so nor any other modules, magic in tst-audit2 isn't updated.  Both orders are correct. This patch makes sure that calloc in tst-audit2.c is called at least once from ld.so. [BZ #18422] * Makefile ($(objpfx)tst-audit2): Depend on $(libdl). ($(objpfx)tst-audit2.out): Also depend on $(objpfx)tst-auditmod9b.so. * elf/tst-audit2.c: Include . (calloc_called): New. (calloc): Allow to be called more than once. (do_test): dllopen/dlclose $ORIGIN/tst-auditmod9b.so. --- elf/Makefile | 3 ++- elf/tst-audit2.c | 23 ++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/elf/Makefile b/elf/Makefile index b06e0a7..dedf3c7 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -1034,7 +1034,8 @@ $(objpfx)tst-dlmopen3.out: $(objpfx)tst-dlmopen1mod.so $(objpfx)tst-audit1.out: $(objpfx)tst-auditmod1.so tst-audit1-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so -$(objpfx)tst-audit2.out: $(objpfx)tst-auditmod1.so +$(objpfx)tst-audit2: $(libdl) +$(objpfx)tst-audit2.out: $(objpfx)tst-auditmod1.so $(objpfx)tst-auditmod9b.so # Prevent GCC-5 from translating a malloc/memset pair into calloc CFLAGS-tst-audit2.c += -fno-builtin tst-audit2-ENV = LD_AUDIT=$(objpfx)tst-auditmod1.so diff --git a/elf/tst-audit2.c b/elf/tst-audit2.c index acad1b0..44c74d4 100644 --- a/elf/tst-audit2.c +++ b/elf/tst-audit2.c @@ -3,10 +3,12 @@ #include #include #include +#include #define MAGIC1 0xabcdef72 #define MAGIC2 0xd8675309 static __thread unsigned int magic[] = { MAGIC1, MAGIC2 }; +static __thread int calloc_called; #undef calloc @@ -16,13 +18,19 @@ static __thread unsigned int magic[] = { MAGIC1, MAGIC2 }; void * calloc (size_t n, size_t m) { - if (magic[0] != MAGIC1 || magic[1] != MAGIC2) + if (!calloc_called) { - printf ("{%x, %x} != {%x, %x}\n", magic[0], magic[1], MAGIC1, MAGIC2); - abort (); + /* Allow our calloc to be called more than once. */ + calloc_called = 1; + if (magic[0] != MAGIC1 || magic[1] != MAGIC2) + { + printf ("{%x, %x} != {%x, %x}\n", + magic[0], magic[1], MAGIC1, MAGIC2); + abort (); + } + magic[0] = MAGIC2; + magic[1] = MAGIC1; } - magic[0] = MAGIC2; - magic[1] = MAGIC1; n *= m; void *ptr = malloc (n); @@ -34,6 +42,11 @@ calloc (size_t n, size_t m) static int do_test (void) { + /* Make sure that our calloc is called from the dynamic linker at least + once. */ + void *h = dlopen("$ORIGIN/tst-auditmod9b.so", RTLD_LAZY); + if (h != NULL) + dlclose (h); if (magic[1] != MAGIC1 || magic[0] != MAGIC2) { printf ("{%x, %x} != {%x, %x}\n", magic[0], magic[1], MAGIC2, MAGIC1); -- 2.1.0