From patchwork Fri Jan 13 19:03:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 18898 Received: (qmail 107302 invoked by alias); 13 Jan 2017 19:03:17 -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 107291 invoked by uid 89); 13 Jan 2017 19:03:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS, URIBL_RED autolearn=no version=3.3.2 spammy=H*RU:209.85.220.194, Hx-spam-relays-external:209.85.220.194, Carlos, carlos X-HELO: mail-qk0-f194.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=y6A8wi/0eiw+7pUxGP5Zj0au4aHlVcw1WpIFn39y8ic=; b=ouIAjCnL2esOONOXkzT9Ob1KD02jRhQ5V/DMkxElV0jC2TahK0Apl96TT4eR6JztcW WDAzPqm9/A72IpLZYjY8QTUtnf0/bvwNs8ILuZ/58za0sKBGbKk08dzgxaSOYtzakXtq HGz4ZzWsWmC0VlbD0x9A+i6X/+7OWP3mdd9FBzjlg+yewpTbiZDMhrrkumzRcKCueVob Tc7uNz8CjGR/Wsp8/jx9GW+dDQagfn2MM2xlzJ2LtzTtOxCiEOL2rssCKxllG5ZkD92O NT6RuMFg26Enk5ndTTTpKfLnv53E23qxH/Rl6WIg1ZvFIw8sr8mkElKC2sFE8aR36ANj GWWw== X-Gm-Message-State: AIkVDXJoKKpZP+G9cGO+bZcEloMRkoRhyriS5emrcb1CtppsOpeUIIIf3HQu/JpDDtf9Oxkcf9NEL7moG9xXOQ== X-Received: by 10.55.74.3 with SMTP id x3mr3684942qka.275.1484334184513; Fri, 13 Jan 2017 11:03:04 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <20161004184621.GB27454@intel.com> From: "H.J. Lu" Date: Fri, 13 Jan 2017 11:03:03 -0800 Message-ID: Subject: Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019] To: "Carlos O'Donell" Cc: Joseph Myers , GNU C Library On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu wrote: > On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell wrote: >> On 10/05/2016 02:16 PM, H.J. Lu wrote: >>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers wrote: >>>> On Wed, 5 Oct 2016, H.J. Lu wrote: >>>> >>>>> I can try __builtin_memcpy, instread of __builtin_memmove. There are 2 >>> >>> I changed it to use __builtin_memset. >>> >>>>> acceptable results. One is ld.so issues an error and the other is program runs. >>>>> On x86, ld.so issues an error. I don't know what should happen on others. >>>> >>>> You could make the test pass on either of those results (while failing if >>>> ld.so crashes). >>>> >>> >>> I moved the test to elf. It passes if the test runs or ld.so issues an >>> error. Please try it on arm, powerpc and s390. >> >> This is the wrong way to test this. >> >> The point of this test is this: >> >> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED >> on DSO B, when resolved to a symbol definition in DSO B, when the symbol in >> DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC >> resolver is called, because DSO B's resolver might need global data to make >> the IFUNC decision e.g. GOT setup. >> >> The invariant we want to hold true for IFUNC is that to call the resolver >> function you must have relocated the DSO which contains the resolver. This _should_ >> have been done by a symbol reocation dependency analysis, but that isn't working >> correctly IMO or needs deeper analysis in the dynamic loader. >> >> The solution we want in place today is to issue some kind of diagnostic until we >> fix the real problem. >> >> The test should look like this: >> >> - DSO A with an unversioned symbol reference to 'foo'. >> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the >> resolver function which references global data from DSO C to decide which of >> two functions to return. >> - DSO C with global data set to a value. >> >> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get >> relocated first, then B, such that B's GOT is setup to access C's global data. >> >> When handling the reference to 'foo' in DSO A we should on x86_64 and i686 >> get the error about needing to relink DSO A so it depends on DSO B, to form >> the initialization order of C->B->A. >> >> I expect this test case will now crash the other arches, rather than just >> avoiding the crash by relying on internal libc.so details about which ifuncs >> you're using. >> >> This is one step towards a better definition of IFUNC semantics, which need to >> be more clearly defined (something I wish I had time to define and fix so >> more projects could use them). > > IFUNC resolver can fail for various reasons. My goal is to make sure > that IFUNC inside of glibc works correctly or an error message is given > when glibc isn't used properly. In case of x86, CPU feature info is > retrieved and stored in ld.so very early at startup, which is used by IFUNC > and only accessible in libc.so and libm.so after they have been relocated. > My change in x86 ld.so checks it and my test verifies the check. My fix > won't cover other possible IFUNC failures. > When the IFUNC relocation is performed before the providing shared library is unrelocated, the returned function address will be 0 and program will segfault when the function is called. Please apply this patch and run the test if your platform has IFUNC. I only enabled the unsafe resolver check for i386 and x86-64. It is straightforward to add check for other platforms. H.J. From 5db1cdaef209ba5b742308d5dcb299aaacdf3f92 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Thu, 12 Jan 2017 15:27:05 -0800 Subject: [PATCH] Add an IFUNC testcase for [BZ #20019] When the IFUNC relocation is performed before the providing shared library is unrelocated, the returned function address will be 0 and program will segfault when the function is called. Add a testcase to verify that ld.so issues a diagnostic. [BZ #20019] * elf/Makefile (modules-names): Add tst-ifuncmod1a, tst-ifuncmod1b, tst-ifuncmod1c and tst-ifuncmod1d. (tests): Add tst-ifunc1. (tests-special): Add $(objpfx)tst-ifunc1.out. ($(objpfx)tst-ifunc1.out): New rule. ($(objpfx)tst-ifunc1): New dependency. ($(objpfx)tst-ifuncmod1a.so): LIkewise. ($(objpfx)tst-ifuncmod1b.so): LIkewise. ($(objpfx)tst-ifuncmod1c.so): LIkewise. (LDFLAGS-tst-ifuncmod1b.so): New. * elf/tst-ifunc1.c: New file. * elf/tst-ifunc1.sh: LIkewise. * elf/tst-ifuncmod1a.c: LIkewise. * elf/tst-ifuncmod1b.c: LIkewise. * elf/tst-ifuncmod1c.c: LIkewise. * elf/tst-ifuncmod1d.c: LIkewise. * sysdeps/generic/ldsodefs.h (dl_check_ifunc_resolver): New function. * sysdeps/i386/dl-machine.h (elf_machine_rel): Use it. * sysdeps/x86_64/dl-machine.h (elf_machine_rela): LIkewise. --- elf/Makefile | 19 +++++++++++++++++-- elf/tst-ifunc1.c | 26 ++++++++++++++++++++++++++ elf/tst-ifunc1.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ elf/tst-ifuncmod1a.c | 25 +++++++++++++++++++++++++ elf/tst-ifuncmod1b.c | 25 +++++++++++++++++++++++++ elf/tst-ifuncmod1c.c | 27 +++++++++++++++++++++++++++ elf/tst-ifuncmod1d.c | 24 ++++++++++++++++++++++++ sysdeps/generic/ldsodefs.h | 26 ++++++++++++++++++++++++++ sysdeps/i386/dl-machine.h | 13 +------------ sysdeps/x86_64/dl-machine.h | 13 +------------ 10 files changed, 215 insertions(+), 26 deletions(-) create mode 100644 elf/tst-ifunc1.c create mode 100755 elf/tst-ifunc1.sh create mode 100644 elf/tst-ifuncmod1a.c create mode 100644 elf/tst-ifuncmod1b.c create mode 100644 elf/tst-ifuncmod1c.c create mode 100644 elf/tst-ifuncmod1d.c diff --git a/elf/Makefile b/elf/Makefile index c7a2969..259362d 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -307,9 +307,11 @@ tests += ifuncmain1 ifuncmain1pic ifuncmain1vis ifuncmain1vispic \ ifuncmain1staticpic \ ifuncmain2 ifuncmain2pic ifuncmain3 ifuncmain4 \ ifuncmain5 ifuncmain5pic ifuncmain5staticpic \ - ifuncmain7 ifuncmain7pic + ifuncmain7 ifuncmain7pic tst-ifunc1 +tests-special += $(objpfx)tst-ifunc1.out ifunc-test-modules = ifuncdep1 ifuncdep1pic ifuncdep2 ifuncdep2pic \ - ifuncdep5 ifuncdep5pic + ifuncdep5 ifuncdep5pic tst-ifuncmod1a \ + tst-ifuncmod1b tst-ifuncmod1c tst-ifuncmod1d extra-test-objs += $(ifunc-test-modules:=.o) test-extras += $(ifunc-test-modules) ifeq (yes,$(have-fpie)) @@ -1026,6 +1028,19 @@ CFLAGS-tst-pie2.c += $(pie-ccflag) $(objpfx)tst-piemod1.so: $(libsupport) $(objpfx)tst-pie1: $(objpfx)tst-piemod1.so +$(objpfx)tst-ifunc1.out: tst-ifunc1.sh $(objpfx)tst-ifunc1 + $(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \ + '$(test-wrapper-env)' '$(run-program-env)'; \ + $(evaluate-test) + +$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so \ + $(objpfx)tst-ifuncmod1c.so \ + $(objpfx)tst-ifuncmod1d.so +$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1b.so +$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1d.so +$(objpfx)tst-ifuncmod1c.so: $(objpfx)tst-ifuncmod1d.so +LDFLAGS-tst-ifuncmod1b.so = -Wl,-z,now + ifeq (yes,$(build-shared)) all-built-dso := $(common-objpfx)elf/ld.so $(common-objpfx)libc.so \ $(filter-out $(common-objpfx)linkobj/libc.so, \ diff --git a/elf/tst-ifunc1.c b/elf/tst-ifunc1.c new file mode 100644 index 0000000..e44dbc3 --- /dev/null +++ b/elf/tst-ifunc1.c @@ -0,0 +1,26 @@ +/* Test case for unsafe IFUNC resolver. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +extern void foo (void); + +int +main (void) +{ + foo (); + return 0; +} diff --git a/elf/tst-ifunc1.sh b/elf/tst-ifunc1.sh new file mode 100755 index 0000000..e523b44 --- /dev/null +++ b/elf/tst-ifunc1.sh @@ -0,0 +1,43 @@ +#!/bin/bash +# A Test case for unsafe IFUNC resolver. +# Copyright (C) 2017 Free Software Foundation, Inc. +# This file is part of the GNU C Library. + +# The GNU C Library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. + +# The GNU C Library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. + +# You should have received a copy of the GNU Lesser General Public +# License along with the GNU C Library; if not, see +# . + +set -e + +objpfx=$1; shift +test_via_rtld_prefix=$1; shift +test_wrapper_env=$1; shift +run_program_env=$1; shift +logfile=${objpfx}tst-ifunc1.out + +> $logfile +fail=0 + +${test_wrapper_env} \ +${run_program_env} \ +${objpfx}tst-ifunc1 2>&1 || fail=1 + +if test $fail = 1; then + # If it fails to run, check for the expected error from ld.so. + fail=0 + ${test_wrapper_env} \ + ${run_program_env} \ + ${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1 +fi + +exit $fail diff --git a/elf/tst-ifuncmod1a.c b/elf/tst-ifuncmod1a.c new file mode 100644 index 0000000..5e2183c --- /dev/null +++ b/elf/tst-ifuncmod1a.c @@ -0,0 +1,25 @@ +/* Test case for unsafe IFUNC resolver. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +extern void bar (void); + +void +foo (void) +{ + bar (); +} diff --git a/elf/tst-ifuncmod1b.c b/elf/tst-ifuncmod1b.c new file mode 100644 index 0000000..0c6fe10 --- /dev/null +++ b/elf/tst-ifuncmod1b.c @@ -0,0 +1,25 @@ +/* Test case for unsafe IFUNC resolver. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +extern void xxx (void); + +void +bar (void) +{ + xxx (); +} diff --git a/elf/tst-ifuncmod1c.c b/elf/tst-ifuncmod1c.c new file mode 100644 index 0000000..81cd31c --- /dev/null +++ b/elf/tst-ifuncmod1c.c @@ -0,0 +1,27 @@ +/* Test case for unsafe IFUNC resolver. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +extern void real_xxx (void); + +static void * +xxx_resolver (void) +{ + return &real_xxx; +} + +extern void xxx (void) __attribute__ ((ifunc ("xxx_resolver"))); diff --git a/elf/tst-ifuncmod1d.c b/elf/tst-ifuncmod1d.c new file mode 100644 index 0000000..5715ded --- /dev/null +++ b/elf/tst-ifuncmod1d.c @@ -0,0 +1,24 @@ +/* Test case for unsafe IFUNC resolver. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +void +xxx (void) +{ +} + +__typeof (xxx) real_xxx __attribute__ ((alias("xxx"))); diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index f26a8b2..0c53c70 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -1085,6 +1085,32 @@ extern void _dl_non_dynamic_init (void) internal_function; extern void _dl_aux_init (ElfW(auxv_t) *av) internal_function; +# ifdef STDERR_FILENO +/* Define here after RTLD_PROGNAME is defined and if STDERR_FILENO is + defined. Since it is unsafe for IFUNC resolver to reference external + symbols, issue a fatal error when it happens. */ +static __always_inline void +dl_check_ifunc_resolver (struct link_map *sym_map, struct link_map *map, + const ElfW(Sym) *const refsym) +{ + if (sym_map != map + && sym_map->l_type != lt_executable + && !sym_map->l_relocated) + { + const char *strtab + = (const char *) D_PTR (map, l_info[DT_STRTAB]); + _dl_fatal_printf ("\ +%s: Relink `%s' with `%s' or place `%s' before `%s' for IFUNC symbol `%s'\n", + RTLD_PROGNAME, + map->l_libname->name, + sym_map->l_libname->name, + sym_map->l_libname->name, + map->l_libname->name, + strtab + refsym->st_name); + } +} +# endif + __END_DECLS #endif /* ldsodefs.h */ diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h index 6eca69d..b13d97c 100644 --- a/sysdeps/i386/dl-machine.h +++ b/sysdeps/i386/dl-machine.h @@ -323,18 +323,7 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc, && __builtin_expect (!skip_ifunc, 1)) { # ifndef RTLD_BOOTSTRAP - if (sym_map != map - && sym_map->l_type != lt_executable - && !sym_map->l_relocated) - { - const char *strtab - = (const char *) D_PTR (map, l_info[DT_STRTAB]); - _dl_fatal_printf ("\ -%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n", - RTLD_PROGNAME, map->l_name, - sym_map->l_name, - strtab + refsym->st_name); - } + dl_check_ifunc_resolver (sym_map, map, refsym); # endif value = ((Elf32_Addr (*) (void)) value) (); } diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h index 3e7ae22..5737955 100644 --- a/sysdeps/x86_64/dl-machine.h +++ b/sysdeps/x86_64/dl-machine.h @@ -333,18 +333,7 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc, && __builtin_expect (!skip_ifunc, 1)) { # ifndef RTLD_BOOTSTRAP - if (sym_map != map - && sym_map->l_type != lt_executable - && !sym_map->l_relocated) - { - const char *strtab - = (const char *) D_PTR (map, l_info[DT_STRTAB]); - _dl_fatal_printf ("\ -%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n", - RTLD_PROGNAME, map->l_name, - sym_map->l_name, - strtab + refsym->st_name); - } + dl_check_ifunc_resolver (sym_map, map, refsym); # endif value = ((ElfW(Addr) (*) (void)) value) (); } -- 2.9.3