From patchwork Tue Apr 9 22:56:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 32239 Received: (qmail 44342 invoked by alias); 9 Apr 2019 22:56:53 -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 44332 invoked by uid 89); 9 Apr 2019 22:56:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-13.9 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=1913, newly, fir X-HELO: mail-ot1-f66.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=teyi/uiQslib7Vjsvci3TtXAbKWh+m8gx+fyOMN7TQg=; b=jmc6X5jFmzPQrY20xYzo96w0GMIgvB5wCzumSKAVyBjVK1PdJDVPV6yzcwtx256Qzw twVNdxePSNqXYb2NENHiC46GPcRFBlDI9PBExbfJlw0G7zjFOZcTpEGvCjoV/18/53MA 7zfCyjkGn9frEg+pB0VArmgnPs1fULpke2/6olk4icfUmZFPNS8AJzLRdIntGb40PZci VHVXWTOM4w0uLRy61f2fWmlzEeH1AhQQs23fDgQ3jn5vBoATf38L9Yk97RdnK1U0DVE/ 6HFhxA65zrBleFhqdEj8MGbPpjmCvBmXbasUKryEpG247AoqWUeDmcTIsauLdvpycAE4 eS7A== MIME-Version: 1.0 References: <20190224160159.1804-1-hjl.tools@gmail.com> <87bm2qls3q.fsf@oldenburg2.str.redhat.com> <877eddifpg.fsf@oldenburg2.str.redhat.com> In-Reply-To: <877eddifpg.fsf@oldenburg2.str.redhat.com> From: "H.J. Lu" Date: Tue, 9 Apr 2019 15:56:10 -0700 Message-ID: Subject: PING^1 [PATCH] Call _dl_open_check after relocation is finished [BZ #24259] To: Florian Weimer Cc: GNU C Library On Tue, Mar 5, 2019 at 6:00 AM Florian Weimer wrote: > > * H. J. Lu: > > >> If you report the error at this, doesn't this mean the object is still > >> around and in a bad state? This looks related to this bug: > > > > Yes. > > > >> > >> > >> Would the CET bug go away if we got rid after the object without trace > >> after a failure in _dl_open_check? > > > > Yes. > > > >> I can look into fixing the other bug, but I don't know how hard that's > >> going to be. > > I did that now and the required infrastructure changes are fairly > involved. So I think we should add something that works today. > > A natural place for the CET compatibility check would be > elf_machine_reject_phdr_p (currently used only on MIPS). This way, we > can continue searching for a CET-compatible library along the search > path. We only want to check it in dlopen path. A legacy shared library can loaded before main () and shadow stack will be disabled. > >> > diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c > >> > new file mode 100644 > >> > index 0000000000..fbf640f664 > >> > --- /dev/null > >> > +++ b/sysdeps/x86/tst-cet-legacy-5.c > >> > >> > +static void > >> > +do_test_1 (const char *modname, bool fail) > >> > +{ > >> > + int (*fp) (void); > >> > + void *h; > >> > + > >> > + h = dlopen (modname, RTLD_LAZY); > >> > + if (h == NULL) > >> > + { > >> > + if (fail) > >> > + { > >> > + const char *err = dlerror (); > >> > + if (strstr (err, "shadow stack isn't enabled") == NULL) > >> > + { > >> > + printf ("incorrect dlopen '%s' error: %s\n", modname, > >> > + dlerror ()); > >> > + exit (1); > >> > + } > >> > + > >> > + return; > >> > + } > >> > >> Is the return supposed to be taken if running on non-CET hardware? I'm > >> looking for the UNSUPPORTED case. > > > > This path is taken only on CET hardware. For non-CET hardware, 'h' shouldn't > > be NULL. > > Please add logging to the test for the unsupported case, so that the aim > of the test is clearer and when it fails to achieve its objective. > The tests should run on CET/non-CET kernel/processor. Here is the updated patch with FAIL_EXIT1. OK for master? Thanks. From bf2b26ea60c8131ad97a2606a112c1b9231f79b4 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Sat, 23 Feb 2019 10:43:17 -0800 Subject: [PATCH] Call _dl_open_check after relocation is finished [BZ #24259] When dlopening a shared object, _dl_open_check will throw an exception if CET shadow stack is enabled and the shared object has no shadow stack support. dl_open_worker must call _dl_open_check after relocation is finished. Otherwise, the dependency shared objects may be mmapped without relocation. This will lead to run-time failure later when they are needed by another dlopened shared object which is shadow stack enabled. [BZ #24259] * elf/dl-open.c (dl_open_worker): Call _dl_open_check after relocation is finished. * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-5a and tst-cet-legacy-5b. (modules-names): Add tst-cet-legacy-mod-5a and tst-cet-legacy-mod-5b. (CFLAGS-tst-cet-legacy-5a.c): New. (CFLAGS-tst-cet-legacy-5b.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5a.c): Likewise. (CFLAGS-tst-cet-legacy-mod-5b.c): Likewise. ($(objpfx)tst-cet-legacy-5a): Likewise. ($(objpfx)tst-cet-legacy-5a.out): Likewise. ($(objpfx)tst-cet-legacy-mod-5a.so): Likewise. ($(objpfx)tst-cet-legacy-mod-5b.so): Likewise. ($(objpfx)tst-cet-legacy-5b): Likewise. ($(objpfx)tst-cet-legacy-5b.out): Likewise. (tst-cet-legacy-5b-ENV): Likewise. * sysdeps/x86/tst-cet-legacy-5.c: New file. * sysdeps/x86/tst-cet-legacy-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-5b.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5a.c: Likewise. * sysdeps/x86/tst-cet-legacy-mod-5b.c: Likewise. --- elf/dl-open.c | 7 ++- sysdeps/x86/Makefile | 22 ++++++- sysdeps/x86/tst-cet-legacy-5.c | 66 +++++++++++++++++++++ sysdeps/x86/tst-cet-legacy-5a.c | 1 + sysdeps/x86/tst-cet-legacy-5b.c | 1 + sysdeps/x86/tst-cet-legacy-mod-5.c | 91 +++++++++++++++++++++++++++++ sysdeps/x86/tst-cet-legacy-mod-5a.c | 1 + sysdeps/x86/tst-cet-legacy-mod-5b.c | 1 + 8 files changed, 185 insertions(+), 5 deletions(-) create mode 100644 sysdeps/x86/tst-cet-legacy-5.c create mode 100644 sysdeps/x86/tst-cet-legacy-5a.c create mode 100644 sysdeps/x86/tst-cet-legacy-5b.c create mode 100644 sysdeps/x86/tst-cet-legacy-mod-5.c create mode 100644 sysdeps/x86/tst-cet-legacy-mod-5a.c create mode 100644 sysdeps/x86/tst-cet-legacy-mod-5b.c diff --git a/elf/dl-open.c b/elf/dl-open.c index 12a4f8b853..a144a40790 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -292,8 +292,6 @@ dl_open_worker (void *a) _dl_debug_state (); LIBC_PROBE (map_complete, 3, args->nsid, r, new); - _dl_open_check (new); - /* Print scope information. */ if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_SCOPES)) _dl_show_scope (new, 0); @@ -366,6 +364,11 @@ dl_open_worker (void *a) _dl_relocate_object (l, l->l_scope, reloc_mode, 0); } + /* NB: Since _dl_open_check may throw an exception, it must be called + after relocation is finished. Otherwise, a shared object may be + mmapped without relocation. */ + _dl_open_check (new); + /* If the file is not loaded now as a dependency, add the search list of the newly loaded object to the scope. */ bool any_tls = false; diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 7ec46ca100..308bba80e5 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -19,13 +19,16 @@ ifeq ($(subdir),elf) sysdep-dl-routines += dl-cet tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \ - tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 + tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 \ + tst-cet-legacy-5a tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd) ifneq (no,$(have-tunables)) -tests += tst-cet-legacy-4a tst-cet-legacy-4b tst-cet-legacy-4c +tests += tst-cet-legacy-4a tst-cet-legacy-4b tst-cet-legacy-4c \ + tst-cet-legacy-5b endif modules-names += tst-cet-legacy-mod-1 tst-cet-legacy-mod-2 \ - tst-cet-legacy-mod-4 + tst-cet-legacy-mod-4 tst-cet-legacy-mod-5a \ + tst-cet-legacy-mod-5b CFLAGS-tst-cet-legacy-2.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-2a.c += -fcf-protection @@ -36,6 +39,10 @@ CFLAGS-tst-cet-legacy-4.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-4a.c += -fcf-protection CFLAGS-tst-cet-legacy-4b.c += -fcf-protection CFLAGS-tst-cet-legacy-mod-4.c += -fcf-protection=none +CFLAGS-tst-cet-legacy-5a.c += -fcf-protection +CFLAGS-tst-cet-legacy-5b.c += -fcf-protection +CFLAGS-tst-cet-legacy-mod-5a.c += -fcf-protection=none +CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ $(objpfx)tst-cet-legacy-mod-2.so @@ -47,6 +54,11 @@ $(objpfx)tst-cet-legacy-2a: $(objpfx)tst-cet-legacy-mod-2.so $(libdl) $(objpfx)tst-cet-legacy-2a.out: $(objpfx)tst-cet-legacy-mod-1.so $(objpfx)tst-cet-legacy-4: $(libdl) $(objpfx)tst-cet-legacy-4.out: $(objpfx)tst-cet-legacy-mod-4.so +$(objpfx)tst-cet-legacy-5a: $(libdl) +$(objpfx)tst-cet-legacy-5a.out: $(objpfx)tst-cet-legacy-mod-5a.so \ + $(objpfx)tst-cet-legacy-mod-5b.so +$(objpfx)tst-cet-legacy-mod-5a.so: $(common-objpfx)nptl/libpthread.so +$(objpfx)tst-cet-legacy-mod-5b.so: $(common-objpfx)nptl/libpthread.so ifneq (no,$(have-tunables)) $(objpfx)tst-cet-legacy-4a: $(libdl) $(objpfx)tst-cet-legacy-4a.out: $(objpfx)tst-cet-legacy-mod-4.so @@ -57,6 +69,10 @@ tst-cet-legacy-4b-ENV = GLIBC_TUNABLES=glibc.cpu.x86_shstk=on $(objpfx)tst-cet-legacy-4c: $(libdl) $(objpfx)tst-cet-legacy-4c.out: $(objpfx)tst-cet-legacy-mod-4.so tst-cet-legacy-4c-ENV = GLIBC_TUNABLES=glibc.cpu.x86_shstk=off +$(objpfx)tst-cet-legacy-5b: $(libdl) +$(objpfx)tst-cet-legacy-5b.out: $(objpfx)tst-cet-legacy-mod-5a.so \ + $(objpfx)tst-cet-legacy-mod-5b.so +tst-cet-legacy-5b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK endif endif diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c new file mode 100644 index 0000000000..48bf997e38 --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-5.c @@ -0,0 +1,66 @@ +/* Check compatibility of CET-enabled executable with dlopened legacy + shared object. + Copyright (C) 2019 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 + . */ + +#include +#include +#include +#include +#include +#include + +static void +do_test_1 (const char *modname, bool fail) +{ + int (*fp) (void); + void *h; + + h = dlopen (modname, RTLD_LAZY); + if (h == NULL) + { + if (fail) + { + const char *err = dlerror (); + if (strstr (err, "shadow stack isn't enabled") == NULL) + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, + dlerror ()); + return; + } + + FAIL_EXIT1 ("cannot open '%s': %s\n", modname, dlerror ()); + } + + fp = dlsym (h, "test"); + if (fp == NULL) + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); + + if (fp () != 0) + FAIL_EXIT1 ("test () != 0"); + + dlclose (h); +} + +static int +do_test (void) +{ + do_test_1 ("tst-cet-legacy-mod-5a.so", true); + do_test_1 ("tst-cet-legacy-mod-5b.so", false); + return 0; +} + +#include diff --git a/sysdeps/x86/tst-cet-legacy-5a.c b/sysdeps/x86/tst-cet-legacy-5a.c new file mode 100644 index 0000000000..fc5a609dff --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-5a.c @@ -0,0 +1 @@ +#include "tst-cet-legacy-5.c" diff --git a/sysdeps/x86/tst-cet-legacy-5b.c b/sysdeps/x86/tst-cet-legacy-5b.c new file mode 100644 index 0000000000..fc5a609dff --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-5b.c @@ -0,0 +1 @@ +#include "tst-cet-legacy-5.c" diff --git a/sysdeps/x86/tst-cet-legacy-mod-5.c b/sysdeps/x86/tst-cet-legacy-mod-5.c new file mode 100644 index 0000000000..38d0aaa727 --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-mod-5.c @@ -0,0 +1,91 @@ +/* Check compatibility of CET-enabled executable with dlopened legacy + shared object. + Copyright (C) 2019 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 + . */ + +#include +#include +#include +#include + +static pthread_cond_t cond = PTHREAD_COND_INITIALIZER; +static pthread_mutex_t mut = PTHREAD_MUTEX_INITIALIZER; + + +static void * +tf (void *p) +{ + int err; + + err = pthread_mutex_lock (&mut); + if (err != 0) + error (EXIT_FAILURE, err, "child: cannot get mutex"); + + puts ("child: got mutex; signalling"); + + pthread_cond_signal (&cond); + + puts ("child: unlock"); + + err = pthread_mutex_unlock (&mut); + if (err != 0) + error (EXIT_FAILURE, err, "child: cannot unlock"); + + puts ("child: done"); + + return NULL; +} + +int +test (void) +{ + pthread_t th; + int err; + + printf ("&cond = %p\n&mut = %p\n", &cond, &mut); + + puts ("parent: get mutex"); + + err = pthread_mutex_lock (&mut); + if (err != 0) + error (EXIT_FAILURE, err, "parent: cannot get mutex"); + + puts ("parent: create child"); + + err = pthread_create (&th, NULL, tf, NULL); + if (err != 0) + error (EXIT_FAILURE, err, "parent: cannot create thread"); + + puts ("parent: wait for condition"); + + /* This test will fail on spurious wake-ups, which are allowed; however, + the current implementation shouldn't produce spurious wake-ups in the + scenario we are testing here. */ + err = pthread_cond_wait (&cond, &mut); + if (err != 0) + error (EXIT_FAILURE, err, "parent: cannot wait fir signal"); + + puts ("parent: got signal"); + + err = pthread_join (th, NULL); + if (err != 0) + error (EXIT_FAILURE, err, "parent: failed to join"); + + puts ("done"); + + return 0; +} diff --git a/sysdeps/x86/tst-cet-legacy-mod-5a.c b/sysdeps/x86/tst-cet-legacy-mod-5a.c new file mode 100644 index 0000000000..daa43e4e8d --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-mod-5a.c @@ -0,0 +1 @@ +#include "tst-cet-legacy-mod-5.c" diff --git a/sysdeps/x86/tst-cet-legacy-mod-5b.c b/sysdeps/x86/tst-cet-legacy-mod-5b.c new file mode 100644 index 0000000000..daa43e4e8d --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-mod-5b.c @@ -0,0 +1 @@ +#include "tst-cet-legacy-mod-5.c" -- 2.20.1