Message ID | 20200317005817.GA313702@gmail.com |
---|---|
State | Committed |
Headers |
Return-Path: <hjl.tools@gmail.com> X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by sourceware.org (Postfix) with ESMTPS id 3581C3937431 for <libc-alpha@sourceware.org>; Tue, 17 Mar 2020 00:58:21 +0000 (GMT) Received: by mail-pl1-x643.google.com with SMTP id g2so6310373plo.3 for <libc-alpha@sourceware.org>; Mon, 16 Mar 2020 17:58:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=2JQY4baGQxfiT3LsT1h9EIWyxnLk9GNUICabUb+fMvM=; b=W8EbIRemz3aXUe+YZP+B2CHc3HQy7AZL0gF442XKkR5NydsiGbxJBGKrqBNnIlRIhs mOVenR72AQ7EU2PJhPHMA4FFX7W5/Xw0jNrQRV9Wk+EGfOH8xxVoQupVQc7rnb1BH8PQ QyceM6GGmasScy+bMO30x5A7gQ58CK8hYhLo9CAMx19mnQWxbR7MnqXyjI9se+nY4IyS hX+lcOM4GJ7TmeRkLLfKCrRkI6BY3UAQ/qX8jw2PODNsSdf3EfXfugcm5JQlUUFwsLKQ G7lQnBdnhoOLeMRb5uIO7hIMGEW4sysD4s7RV5rd4hkNYsTtC6Gq8sWl63mVBwPuUIxR E8Kg== X-Gm-Message-State: ANhLgQ1Mf1UpdMWnh2JrEG1pNrdklieD4Ykx7vCUUNXAZlggi+pHQKsD 5wvOQbWk6m2fa+QFhwtSdZftCs75 X-Google-Smtp-Source: ADFU+vvhEEELyiwBoZtEoDwjWAQ8BdRtsAZ5YT+F+/V0GaH+ED7zB2jdYoIiT/uIwaI0gH0Ymd0+nQ== X-Received: by 2002:a17:902:9b8b:: with SMTP id y11mr1886571plp.189.1584406699397; Mon, 16 Mar 2020 17:58:19 -0700 (PDT) Received: from gnu-cfl-2.localdomain (c-69-181-90-243.hsd1.ca.comcast.net. [69.181.90.243]) by smtp.gmail.com with ESMTPSA id gn11sm808523pjb.42.2020.03.16.17.58.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Mar 2020 17:58:18 -0700 (PDT) Received: by gnu-cfl-2.localdomain (Postfix, from userid 1000) id C2823C04A7; Mon, 16 Mar 2020 17:58:17 -0700 (PDT) Date: Mon, 16 Mar 2020 17:58:17 -0700 From: "H.J. Lu" <hjl.tools@gmail.com> To: Carlos O'Donell <carlos@redhat.com> Cc: Florian Weimer <fw@deneb.enyo.de>, GNU C Library <libc-alpha@sourceware.org> Subject: V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397] Message-ID: <20200317005817.GA313702@gmail.com> References: <CAMe9rOoON6Dt-WaMmAQ3m76GMxp_RnKjLUZvL=QyheqCa_4=Vg@mail.gmail.com> <877dzs5p4u.fsf@mid.deneb.enyo.de> <20200310133654.GA244749@gmail.com> <20200313132509.GA745249@gmail.com> <2bcb5482-4fc9-7edc-2e83-f9076863f82a@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2bcb5482-4fc9-7edc-2e83-f9076863f82a@redhat.com> X-Spam-Status: No, score=-25.2 required=5.0 tests=DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <http://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <http://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <http://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> X-List-Received-Date: Tue, 17 Mar 2020 00:58:24 -0000 |
Series |
V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
|
|
Commit Message
H.J. Lu
March 17, 2020, 12:58 a.m. UTC
On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote: > On 3/13/20 9:25 AM, H.J. Lu via Libc-alpha wrote: > > On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote: > >> On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote: > >>> * H. J. Lu: > >>> > >>>>> I think the error message should say *where* indirect branch tracking > >>>>> is not enabled. I assume this is a property of the loaded object. > >>>> Fixed. Now I got > >>>> > >>>> .... libvirtd[1048]: internal error: Failed to load module > >>>> '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so': > >>>> /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't > >>>> enabled: Invalid argument > >>> What I meant is that the error message should say that the object > >>> needs to be rebuilt with IBT/SHSTK support. In the above, it's > >>> unclear whether the process or the object has to enable IBT. > >>> > >>> (The Invalid Argument part should also be suppressed.) > >> Like this? The diff against V2 patch is: > >> > >> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > >> index f527a1414c..b2843488be 100644 > >> --- a/sysdeps/x86/dl-cet.c > >> +++ b/sysdeps/x86/dl-cet.c > >> @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program) > >> /* When IBT is enabled, we cannot dlopen a shared > >> object without IBT. */ > >> if (found_ibt_legacy) > >> - _dl_signal_error (EINVAL, > >> + _dl_signal_error (0, > >> m->l_initfini[ibt_legacy]->l_name, > >> "dlopen", > >> - N_("indirect branch tracking isn't enabled")); > >> + N_("rebuilt with IBT support needed")); > >> } > >> > >> if (enable_shstk_type != CET_PERMISSIVE) > >> @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program) > >> /* When SHSTK is enabled, we cannot dlopen a shared > >> object without SHSTK. */ > >> if (found_shstk_legacy) > >> - _dl_signal_error (EINVAL, > >> + _dl_signal_error (0, > >> m->l_initfini[shstk_legacy]->l_name, > >> "dlopen", > >> - N_("shadow stack isn't enabled")); > >> + N_("rebuilt with SHSTK support needed")); > >> } > >> > >> if (enable_ibt_type != CET_PERMISSIVE > >> > > PING. > > Please post v4 with adjusted error message, and expanded comment in new test, > and I'll review that quickly. > > > H.J. > > --- > > Since legacy bitmap doesn't cover jitted code generated by legacy JIT > > engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP > > and treats indirect branch tracking similar to shadow stack by removing > > legacy bitmap support. > > OK. > > > > > > * sysdeps/unix/sysv/linux/x86/dl-cet.h > > (dl_cet_allocate_legacy_bitmap): Removed. > > * sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > (ARCH_CET_LEGACY_BITMAP): Removed. > > * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-7. > > (CFLAGS-tst-cet-legacy-mod-5a.c): Changed to > > -fcf-protection=branch. > > (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. > > (CFLAGS-tst-cet-legacy-7.c): New. > > * sysdeps/x86/dl-cet.c (dl_cet_mark_legacy_region): Removed. > > (dl_cet_check): Remove legacy bitmap support. Don't allow > > dlopening legacy shared library when IBT is enabled. Set > > feature_1 if IBT or SHSTK is enabled in executable. > > * sysdeps/x86/dl-procruntime.c (_dl_x86_legacy_bitmap): Removed. > > * sysdeps/x86/tst-cet-legacy-4.c: Include <string.h> and > > <support/check.h>. > > (do_test): Check indirect branch tracking error. Use > > FAIL_EXIT1. > > * sysdeps/x86/tst-cet-legacy-7.c: New file. > > Hopefully you don't plan to include this in the commit log :-) > Leftover from my old version :-). > > --- > > sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- > > .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - > > sysdeps/x86/Makefile | 7 +- > > sysdeps/x86/dl-cet.c | 217 +++++------------- > > sysdeps/x86/dl-procruntime.c | 11 - > > sysdeps/x86/tst-cet-legacy-4.c | 19 +- > > sysdeps/x86/tst-cet-legacy-5.c | 2 +- > > sysdeps/x86/tst-cet-legacy-6.c | 2 +- > > sysdeps/x86/tst-cet-legacy-7.c | 36 +++ > > 9 files changed, 111 insertions(+), 208 deletions(-) > > create mode 100644 sysdeps/x86/tst-cet-legacy-7.c > > > > diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h > > index 9b2aaa238c..ae97a433a2 100644 > > --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h > > +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h > > @@ -18,26 +18,6 @@ > > #include <sys/prctl.h> > > #include <asm/prctl.h> > > > > -static inline int __attribute__ ((always_inline)) > > -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) > > -{ > > - /* Allocate legacy bitmap. */ > > -#ifdef __LP64__ > > - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, > > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); > > -#else > > - unsigned long long legacy_bitmap_u64[2]; > > - int res = INTERNAL_SYSCALL_CALL (arch_prctl, > > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); > > - if (res == 0) > > - { > > - legacy_bitmap[0] = legacy_bitmap_u64[0]; > > - legacy_bitmap[1] = legacy_bitmap_u64[1]; > > - } > > - return res; > > -#endif > > -} > > - > > OK. > > > static inline int __attribute__ ((always_inline)) > > dl_cet_disable_cet (unsigned int cet_feature) > > { > > diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > index f67f3299b9..45ad0b052f 100644 > > --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > @@ -24,9 +24,4 @@ > > OUT: allocated shadow stack address: *addr. > > */ > > # define ARCH_CET_ALLOC_SHSTK 0x3004 > > -/* Return legacy region bitmap info in unsigned long long *addr: > > - address: addr[0]. > > - size: addr[1]. > > - */ > > -# define ARCH_CET_LEGACY_BITMAP 0x3005 > > OK. > > > #endif /* ARCH_CET_STATUS */ > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > > index 95182a508c..ec96b59a78 100644 > > --- a/sysdeps/x86/Makefile > > +++ b/sysdeps/x86/Makefile > > @@ -20,7 +20,7 @@ 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-5a tst-cet-legacy-6a > > + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 > > OK. > > > 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 \ > > @@ -43,14 +43,15 @@ 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-5a.c += -fcf-protection=branch > > CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection > > CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection > > CFLAGS-tst-cet-legacy-6a.c += -fcf-protection > > CFLAGS-tst-cet-legacy-6b.c += -fcf-protection > > -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none > > +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch > > CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection > > CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection > > +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none > > OK. > > > > > $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ > > $(objpfx)tst-cet-legacy-mod-2.so > > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > > index ca3b5849bc..b2843488be 100644 > > --- a/sysdeps/x86/dl-cet.c > > +++ b/sysdeps/x86/dl-cet.c > > @@ -33,63 +33,6 @@ > > # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK > > #endif > > > > -static int > > -dl_cet_mark_legacy_region (struct link_map *l) > > -{ > > - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ > > - size_t i, phnum = l->l_phnum; > > - const ElfW(Phdr) *phdr = l->l_phdr; > > -#ifdef __x86_64__ > > - typedef unsigned long long word_t; > > -#else > > - typedef unsigned long word_t; > > -#endif > > - unsigned int bits_to_set; > > - word_t mask_to_set; > > -#define BITS_PER_WORD (sizeof (word_t) * 8) > > -#define BITMAP_FIRST_WORD_MASK(start) \ > > - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) > > -#define BITMAP_LAST_WORD_MASK(nbits) \ > > - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) > > - > > - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; > > - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; > > - word_t *p; > > - size_t page_size = GLRO(dl_pagesize); > > - > > - for (i = 0; i < phnum; i++) > > - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) > > - { > > - /* One bit in legacy bitmap represents a page. */ > > - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; > > - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; > > - ElfW(Addr) end = start + len; > > - > > - if ((end / 8) > bitmap_size) > > - return -EINVAL; > > - > > - p = bitmap + (start / BITS_PER_WORD); > > - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); > > - mask_to_set = BITMAP_FIRST_WORD_MASK (start); > > - > > - while (len >= bits_to_set) > > - { > > - *p |= mask_to_set; > > - len -= bits_to_set; > > - bits_to_set = BITS_PER_WORD; > > - mask_to_set = ~((word_t) 0); > > - p++; > > - } > > - if (len) > > - { > > - mask_to_set &= BITMAP_LAST_WORD_MASK (end); > > - *p |= mask_to_set; > > - } > > - } > > - > > - return 0; > > -} > > - > > OK. > > > /* Check if object M is compatible with CET. */ > > > > static void > > @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) > > if (ibt_enabled || shstk_enabled) > > { > > struct link_map *l = NULL; > > + unsigned int ibt_legacy = 0, shstk_legacy = 0; > > + bool found_ibt_legacy = false, found_shstk_legacy = false; > > OK. > > > > > /* Check if IBT and SHSTK are enabled in object. */ > > bool enable_ibt = (ibt_enabled > > @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) > > support IBT nor SHSTK. */ > > if (enable_ibt || enable_shstk) > > { > > - int res; > > unsigned int i; > > - unsigned int first_legacy, last_legacy; > > - bool need_legacy_bitmap = false; > > OK. > > > > > i = m->l_searchlist.r_nlist; > > while (i-- > 0) > > @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) > > continue; > > #endif > > > > - if (enable_ibt > > - && enable_ibt_type != CET_ALWAYS_ON > > - && !(l->l_cet & lc_ibt)) > > + /* IBT is enabled only if it is enabled in executable as > > + well as all shared objects. */ > > + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON > > + || (l->l_cet & lc_ibt) != 0); > > + if (!found_ibt_legacy && enable_ibt != ibt_enabled) > > { > > - /* Remember the first and last legacy objects. */ > > - if (!need_legacy_bitmap) > > - last_legacy = i; > > - first_legacy = i; > > - need_legacy_bitmap = true; > > + found_ibt_legacy = true; > > + ibt_legacy = i; > > OK. > > > } > > > > /* SHSTK is enabled only if it is enabled in executable as > > well as all shared objects. */ > > enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON > > || (l->l_cet & lc_shstk) != 0); > > - } > > - > > - if (need_legacy_bitmap) > > - { > > - if (GL(dl_x86_legacy_bitmap)[0]) > > - { > > - /* Change legacy bitmap to writable. */ > > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > > - GL(dl_x86_legacy_bitmap)[1], > > - PROT_READ | PROT_WRITE) < 0) > > - { > > -mprotect_failure: > > - if (program) > > - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", > > - l->l_name); > > - else > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("mprotect legacy bitmap failed")); > > - } > > - } > > - else > > + if (enable_shstk != shstk_enabled) > > OK. > > > { > > - /* Allocate legacy bitmap. */ > > - int res = dl_cet_allocate_legacy_bitmap > > - (GL(dl_x86_legacy_bitmap)); > > - if (res != 0) > > - { > > - if (program) > > - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", > > - l->l_name); > > - else > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("legacy bitmap isn't available")); > > - } > > + found_shstk_legacy = true; > > + shstk_legacy = i; > > OK. > > > } > > - > > - /* Put legacy shared objects in legacy bitmap. */ > > - for (i = first_legacy; i <= last_legacy; i++) > > - { > > - l = m->l_initfini[i]; > > - > > - if (l->l_init_called || (l->l_cet & lc_ibt)) > > - continue; > > - > > -#ifdef SHARED > > - if (l == &GL(dl_rtld_map) > > - || l->l_real == &GL(dl_rtld_map) > > - || (program && l == m)) > > - continue; > > -#endif > > - > > - /* If IBT is enabled in executable and IBT isn't enabled > > - in this shard object, mark PT_LOAD segments with PF_X > > - in legacy code page bitmap. */ > > - res = dl_cet_mark_legacy_region (l); > > - if (res != 0) > > - { > > - if (program) > > - _dl_fatal_printf ("%s: failed to mark legacy code region\n", > > - l->l_name); > > - else > > - _dl_signal_error (-res, l->l_name, "dlopen", > > - N_("failed to mark legacy code region")); > > - } > > - } > > - > > - /* Change legacy bitmap to read-only. */ > > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > > - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) > > - goto mprotect_failure; > > OK. > > > } > > } > > > > @@ -259,23 +135,40 @@ mprotect_failure: > > > > if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) > > { > > - if (!program > > - && enable_shstk_type != CET_PERMISSIVE) > > + if (!program) > > { > > - /* When SHSTK is enabled, we can't dlopening a shared > > - object without SHSTK. */ > > - if (enable_shstk != shstk_enabled) > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("shadow stack isn't enabled")); > > - return; > > + if (enable_ibt_type != CET_PERMISSIVE) > > + { > > + /* When IBT is enabled, we cannot dlopen a shared > > + object without IBT. */ > > + if (found_ibt_legacy) > > + _dl_signal_error (0, > > + m->l_initfini[ibt_legacy]->l_name, > > + "dlopen", > > + N_("rebuilt with IBT support needed")); > > + } > > Suggest: > "rebuild shared object with IBT support enabled" Fixed. > > > + > > + if (enable_shstk_type != CET_PERMISSIVE) > > + { > > + /* When SHSTK is enabled, we cannot dlopen a shared > > + object without SHSTK. */ > > + if (found_shstk_legacy) > > + _dl_signal_error (0, > > + m->l_initfini[shstk_legacy]->l_name, > > + "dlopen", > > + N_("rebuilt with SHSTK support needed")); > > + } > > Suggest: > "rebuild shared object with SHSTK support enabled" > Fixed. > > + > > + if (enable_ibt_type != CET_PERMISSIVE > > + && enable_shstk_type != CET_PERMISSIVE) > > + return; > > } > > > > /* Disable IBT and/or SHSTK if they are enabled by kernel, but > > disabled in executable or shared objects. */ > > unsigned int cet_feature = 0; > > > > - /* Disable IBT only during program startup. */ > > - if (program && !enable_ibt) > > + if (!enable_ibt) > > OK. > > > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; > > if (!enable_shstk) > > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; > > @@ -286,8 +179,14 @@ mprotect_failure: > > if (program) > > _dl_fatal_printf ("%s: can't disable CET\n", program); > > else > > - _dl_signal_error (-res, l->l_name, "dlopen", > > - N_("can't disable CET")); > > + { > > + if (found_ibt_legacy) > > + l = m->l_initfini[ibt_legacy]; > > + else > > + l = m->l_initfini[shstk_legacy]; > > + _dl_signal_error (-res, l->l_name, "dlopen", > > + N_("can't disable CET")); > > OK. > > > + } > > } > > > > /* Clear the disabled bits in dl_x86_feature_1. */ > > @@ -297,17 +196,21 @@ mprotect_failure: > > } > > > > #ifdef SHARED > > - if (program > > - && (!shstk_enabled > > - || enable_shstk_type != CET_PERMISSIVE) > > - && (ibt_enabled || shstk_enabled)) > > + if (program && (ibt_enabled || shstk_enabled)) > > { > > - /* Lock CET if IBT or SHSTK is enabled in executable. Don't > > - lock CET if SHSTK is enabled permissively. */ > > - int res = dl_cet_lock_cet (); > > - if (res != 0) > > - _dl_fatal_printf ("%s: can't lock CET\n", program); > > + if ((!ibt_enabled > > + || enable_ibt_type != CET_PERMISSIVE) > > + && (!shstk_enabled > > + || enable_shstk_type != CET_PERMISSIVE)) > > + { > > + /* Lock CET if IBT or SHSTK is enabled in executable unless > > + IBT or SHSTK is enabled permissively. */ > > + int res = dl_cet_lock_cet (); > > + if (res != 0) > > + _dl_fatal_printf ("%s: can't lock CET\n", program); > > OK. > > > + } > > > > + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ > > cet_feature_changed = true; > > } > > #endif > > diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c > > index fb36801f3e..5e39a38133 100644 > > --- a/sysdeps/x86/dl-procruntime.c > > +++ b/sysdeps/x86/dl-procruntime.c > > @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] > > # else > > , > > # endif > > - > > -# if !defined PROCINFO_DECL && defined SHARED > > - ._dl_x86_legacy_bitmap > > -# else > > -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] > > -# endif > > -# if !defined SHARED || defined PROCINFO_DECL > > -; > > -# else > > -, > > -# endif > > OK. > > > #endif > > diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c > > index a77078afc9..a922339333 100644 > > --- a/sysdeps/x86/tst-cet-legacy-4.c > > +++ b/sysdeps/x86/tst-cet-legacy-4.c > > @@ -20,6 +20,9 @@ > > #include <dlfcn.h> > > #include <stdio.h> > > #include <stdlib.h> > > +#include <string.h> > > + > > +#include <support/check.h> > > > > static int > > do_test (void) > > @@ -31,22 +34,18 @@ do_test (void) > > h = dlopen (modname, RTLD_LAZY); > > if (h == NULL) > > { > > - printf ("cannot open '%s': %s\n", modname, dlerror ()); > > - exit (1); > > + const char *err = dlerror (); > > + if (!strstr (err, "rebuilt with IBT support needed")) > > OK (needs adjustment if string is changed). > Fixed. > > + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); > > + return 0; > > } > > > > fp = dlsym (h, "test"); > > if (fp == NULL) > > - { > > - printf ("cannot get symbol 'test': %s\n", dlerror ()); > > - exit (1); > > - } > > + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); > > > > if (fp () != 0) > > - { > > - puts ("test () != 0"); > > - exit (1); > > - } > > + FAIL_EXIT1 ("test () != 0"); > > > > dlclose (h); > > > > diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c > > index 6c9bba06f5..350c7e41df 100644 > > --- a/sysdeps/x86/tst-cet-legacy-5.c > > +++ b/sysdeps/x86/tst-cet-legacy-5.c > > @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) > > if (fail) > > { > > const char *err = dlerror (); > > - if (strstr (err, "shadow stack isn't enabled") == NULL) > > + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) > > OK (also needs adjustment) Fixed. > > > { > > printf ("incorrect dlopen '%s' error: %s\n", modname, > > err); > > diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c > > index 877e77747d..9f2748fcb6 100644 > > --- a/sysdeps/x86/tst-cet-legacy-6.c > > +++ b/sysdeps/x86/tst-cet-legacy-6.c > > @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) > > if (fail) > > { > > const char *err = dlerror (); > > - if (strstr (err, "shadow stack isn't enabled") == NULL) > > + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) > > OK. (also needs adjustment) Fixed. > > > { > > printf ("incorrect dlopen '%s' error: %s\n", modname, > > err); > > diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c > > new file mode 100644 > > index 0000000000..cf4c2779ce > > --- /dev/null > > +++ b/sysdeps/x86/tst-cet-legacy-7.c > > @@ -0,0 +1,36 @@ > > +/* Check compatibility of legacy executable with a JIT engine. > > + Copyright (C) 2020 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 > > + <https://www.gnu.org/licenses/>. */ > > + > > +#include <stdio.h> > > +#include <sys/mman.h> > > +#include <support/xunistd.h> > > + > > Needs a pargraph explaining what this test is supposed to do. > Done. > I expect this test is supposed to just pass and not fail because it's > run with -fcf-protection=none. Yes. > > What about the equivalent test to test for failure? Added. > > > +static int > > +do_test (void) > > +{ > > + void (*funcp) (void); > > + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > > + MAP_ANONYMOUS | MAP_PRIVATE, -1); > > + printf ("mmap = %p\n", funcp); > > + /* Write RET instruction. */ > > + *(char *) funcp = 0xc3; > > + funcp (); > > + return 0; > > OK. > > > +} > > + > > +#include <support/test-driver.c> > > -- 2.24.1 > > OK for master? Thanks. H.J. --- Since legacy bitmap doesn't cover jitted code generated by legacy JIT engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP and treats indirect branch tracking similar to shadow stack by removing legacy bitmap support. --- sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - sysdeps/x86/Makefile | 9 +- sysdeps/x86/dl-cet.c | 217 +++++------------- sysdeps/x86/dl-procruntime.c | 11 - sysdeps/x86/tst-cet-legacy-4.c | 19 +- sysdeps/x86/tst-cet-legacy-5.c | 3 +- sysdeps/x86/tst-cet-legacy-6.c | 3 +- sysdeps/x86/tst-cet-legacy-7.c | 38 +++ sysdeps/x86/tst-cet-legacy-8.c | 55 +++++ 10 files changed, 172 insertions(+), 208 deletions(-) create mode 100644 sysdeps/x86/tst-cet-legacy-7.c create mode 100644 sysdeps/x86/tst-cet-legacy-8.c
Comments
On 3/16/20 8:58 PM, H.J. Lu wrote: > On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote: >> On 3/13/20 9:25 AM, H.J. Lu via Libc-alpha wrote: >>> On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote: >>>> On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote: >>>>> * H. J. Lu: >>>>> >>>>>>> I think the error message should say *where* indirect branch tracking >>>>>>> is not enabled. I assume this is a property of the loaded object. >>>>>> Fixed. Now I got >>>>>> >>>>>> .... libvirtd[1048]: internal error: Failed to load module >>>>>> '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so': >>>>>> /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't >>>>>> enabled: Invalid argument >>>>> What I meant is that the error message should say that the object >>>>> needs to be rebuilt with IBT/SHSTK support. In the above, it's >>>>> unclear whether the process or the object has to enable IBT. >>>>> >>>>> (The Invalid Argument part should also be suppressed.) >>>> Like this? The diff against V2 patch is: >>>> >>>> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c >>>> index f527a1414c..b2843488be 100644 >>>> --- a/sysdeps/x86/dl-cet.c >>>> +++ b/sysdeps/x86/dl-cet.c >>>> @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program) >>>> /* When IBT is enabled, we cannot dlopen a shared >>>> object without IBT. */ >>>> if (found_ibt_legacy) >>>> - _dl_signal_error (EINVAL, >>>> + _dl_signal_error (0, >>>> m->l_initfini[ibt_legacy]->l_name, >>>> "dlopen", >>>> - N_("indirect branch tracking isn't enabled")); >>>> + N_("rebuilt with IBT support needed")); >>>> } >>>> >>>> if (enable_shstk_type != CET_PERMISSIVE) >>>> @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program) >>>> /* When SHSTK is enabled, we cannot dlopen a shared >>>> object without SHSTK. */ >>>> if (found_shstk_legacy) >>>> - _dl_signal_error (EINVAL, >>>> + _dl_signal_error (0, >>>> m->l_initfini[shstk_legacy]->l_name, >>>> "dlopen", >>>> - N_("shadow stack isn't enabled")); >>>> + N_("rebuilt with SHSTK support needed")); >>>> } >>>> >>>> if (enable_ibt_type != CET_PERMISSIVE >>>> >>> PING. >> >> Please post v4 with adjusted error message, and expanded comment in new test, >> and I'll review that quickly. >> >>> H.J. >>> --- >>> Since legacy bitmap doesn't cover jitted code generated by legacy JIT >>> engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP >>> and treats indirect branch tracking similar to shadow stack by removing >>> legacy bitmap support. >> >> OK. >> >> >>> >>> * sysdeps/unix/sysv/linux/x86/dl-cet.h >>> (dl_cet_allocate_legacy_bitmap): Removed. >>> * sysdeps/unix/sysv/linux/x86/include/asm/prctl.h >>> (ARCH_CET_LEGACY_BITMAP): Removed. >>> * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-7. >>> (CFLAGS-tst-cet-legacy-mod-5a.c): Changed to >>> -fcf-protection=branch. >>> (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. >>> (CFLAGS-tst-cet-legacy-7.c): New. >>> * sysdeps/x86/dl-cet.c (dl_cet_mark_legacy_region): Removed. >>> (dl_cet_check): Remove legacy bitmap support. Don't allow >>> dlopening legacy shared library when IBT is enabled. Set >>> feature_1 if IBT or SHSTK is enabled in executable. >>> * sysdeps/x86/dl-procruntime.c (_dl_x86_legacy_bitmap): Removed. >>> * sysdeps/x86/tst-cet-legacy-4.c: Include <string.h> and >>> <support/check.h>. >>> (do_test): Check indirect branch tracking error. Use >>> FAIL_EXIT1. >>> * sysdeps/x86/tst-cet-legacy-7.c: New file. >> >> Hopefully you don't plan to include this in the commit log :-) >> > > Leftover from my old version :-). > >>> --- >>> sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- >>> .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - >>> sysdeps/x86/Makefile | 7 +- >>> sysdeps/x86/dl-cet.c | 217 +++++------------- >>> sysdeps/x86/dl-procruntime.c | 11 - >>> sysdeps/x86/tst-cet-legacy-4.c | 19 +- >>> sysdeps/x86/tst-cet-legacy-5.c | 2 +- >>> sysdeps/x86/tst-cet-legacy-6.c | 2 +- >>> sysdeps/x86/tst-cet-legacy-7.c | 36 +++ >>> 9 files changed, 111 insertions(+), 208 deletions(-) >>> create mode 100644 sysdeps/x86/tst-cet-legacy-7.c >>> >>> diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h >>> index 9b2aaa238c..ae97a433a2 100644 >>> --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h >>> +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h >>> @@ -18,26 +18,6 @@ >>> #include <sys/prctl.h> >>> #include <asm/prctl.h> >>> >>> -static inline int __attribute__ ((always_inline)) >>> -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) >>> -{ >>> - /* Allocate legacy bitmap. */ >>> -#ifdef __LP64__ >>> - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, >>> - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); >>> -#else >>> - unsigned long long legacy_bitmap_u64[2]; >>> - int res = INTERNAL_SYSCALL_CALL (arch_prctl, >>> - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); >>> - if (res == 0) >>> - { >>> - legacy_bitmap[0] = legacy_bitmap_u64[0]; >>> - legacy_bitmap[1] = legacy_bitmap_u64[1]; >>> - } >>> - return res; >>> -#endif >>> -} >>> - >> >> OK. >> >>> static inline int __attribute__ ((always_inline)) >>> dl_cet_disable_cet (unsigned int cet_feature) >>> { >>> diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h >>> index f67f3299b9..45ad0b052f 100644 >>> --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h >>> +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h >>> @@ -24,9 +24,4 @@ >>> OUT: allocated shadow stack address: *addr. >>> */ >>> # define ARCH_CET_ALLOC_SHSTK 0x3004 >>> -/* Return legacy region bitmap info in unsigned long long *addr: >>> - address: addr[0]. >>> - size: addr[1]. >>> - */ >>> -# define ARCH_CET_LEGACY_BITMAP 0x3005 >> >> OK. >> >>> #endif /* ARCH_CET_STATUS */ >>> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile >>> index 95182a508c..ec96b59a78 100644 >>> --- a/sysdeps/x86/Makefile >>> +++ b/sysdeps/x86/Makefile >>> @@ -20,7 +20,7 @@ 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-5a tst-cet-legacy-6a >>> + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 >> >> OK. >> >>> 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 \ >>> @@ -43,14 +43,15 @@ 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-5a.c += -fcf-protection=branch >>> CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection >>> CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection >>> CFLAGS-tst-cet-legacy-6a.c += -fcf-protection >>> CFLAGS-tst-cet-legacy-6b.c += -fcf-protection >>> -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none >>> +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch >>> CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection >>> CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection >>> +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none >> >> OK. >> >>> >>> $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ >>> $(objpfx)tst-cet-legacy-mod-2.so >>> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c >>> index ca3b5849bc..b2843488be 100644 >>> --- a/sysdeps/x86/dl-cet.c >>> +++ b/sysdeps/x86/dl-cet.c >>> @@ -33,63 +33,6 @@ >>> # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK >>> #endif >>> >>> -static int >>> -dl_cet_mark_legacy_region (struct link_map *l) >>> -{ >>> - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ >>> - size_t i, phnum = l->l_phnum; >>> - const ElfW(Phdr) *phdr = l->l_phdr; >>> -#ifdef __x86_64__ >>> - typedef unsigned long long word_t; >>> -#else >>> - typedef unsigned long word_t; >>> -#endif >>> - unsigned int bits_to_set; >>> - word_t mask_to_set; >>> -#define BITS_PER_WORD (sizeof (word_t) * 8) >>> -#define BITMAP_FIRST_WORD_MASK(start) \ >>> - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) >>> -#define BITMAP_LAST_WORD_MASK(nbits) \ >>> - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) >>> - >>> - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; >>> - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; >>> - word_t *p; >>> - size_t page_size = GLRO(dl_pagesize); >>> - >>> - for (i = 0; i < phnum; i++) >>> - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) >>> - { >>> - /* One bit in legacy bitmap represents a page. */ >>> - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; >>> - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; >>> - ElfW(Addr) end = start + len; >>> - >>> - if ((end / 8) > bitmap_size) >>> - return -EINVAL; >>> - >>> - p = bitmap + (start / BITS_PER_WORD); >>> - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); >>> - mask_to_set = BITMAP_FIRST_WORD_MASK (start); >>> - >>> - while (len >= bits_to_set) >>> - { >>> - *p |= mask_to_set; >>> - len -= bits_to_set; >>> - bits_to_set = BITS_PER_WORD; >>> - mask_to_set = ~((word_t) 0); >>> - p++; >>> - } >>> - if (len) >>> - { >>> - mask_to_set &= BITMAP_LAST_WORD_MASK (end); >>> - *p |= mask_to_set; >>> - } >>> - } >>> - >>> - return 0; >>> -} >>> - >> >> OK. >> >>> /* Check if object M is compatible with CET. */ >>> >>> static void >>> @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) >>> if (ibt_enabled || shstk_enabled) >>> { >>> struct link_map *l = NULL; >>> + unsigned int ibt_legacy = 0, shstk_legacy = 0; >>> + bool found_ibt_legacy = false, found_shstk_legacy = false; >> >> OK. >> >>> >>> /* Check if IBT and SHSTK are enabled in object. */ >>> bool enable_ibt = (ibt_enabled >>> @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) >>> support IBT nor SHSTK. */ >>> if (enable_ibt || enable_shstk) >>> { >>> - int res; >>> unsigned int i; >>> - unsigned int first_legacy, last_legacy; >>> - bool need_legacy_bitmap = false; >> >> OK. >> >>> >>> i = m->l_searchlist.r_nlist; >>> while (i-- > 0) >>> @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) >>> continue; >>> #endif >>> >>> - if (enable_ibt >>> - && enable_ibt_type != CET_ALWAYS_ON >>> - && !(l->l_cet & lc_ibt)) >>> + /* IBT is enabled only if it is enabled in executable as >>> + well as all shared objects. */ >>> + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON >>> + || (l->l_cet & lc_ibt) != 0); >>> + if (!found_ibt_legacy && enable_ibt != ibt_enabled) >>> { >>> - /* Remember the first and last legacy objects. */ >>> - if (!need_legacy_bitmap) >>> - last_legacy = i; >>> - first_legacy = i; >>> - need_legacy_bitmap = true; >>> + found_ibt_legacy = true; >>> + ibt_legacy = i; >> >> OK. >> >>> } >>> >>> /* SHSTK is enabled only if it is enabled in executable as >>> well as all shared objects. */ >>> enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON >>> || (l->l_cet & lc_shstk) != 0); >>> - } >>> - >>> - if (need_legacy_bitmap) >>> - { >>> - if (GL(dl_x86_legacy_bitmap)[0]) >>> - { >>> - /* Change legacy bitmap to writable. */ >>> - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], >>> - GL(dl_x86_legacy_bitmap)[1], >>> - PROT_READ | PROT_WRITE) < 0) >>> - { >>> -mprotect_failure: >>> - if (program) >>> - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", >>> - l->l_name); >>> - else >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", >>> - N_("mprotect legacy bitmap failed")); >>> - } >>> - } >>> - else >>> + if (enable_shstk != shstk_enabled) >> >> OK. >> >>> { >>> - /* Allocate legacy bitmap. */ >>> - int res = dl_cet_allocate_legacy_bitmap >>> - (GL(dl_x86_legacy_bitmap)); >>> - if (res != 0) >>> - { >>> - if (program) >>> - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", >>> - l->l_name); >>> - else >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", >>> - N_("legacy bitmap isn't available")); >>> - } >>> + found_shstk_legacy = true; >>> + shstk_legacy = i; >> >> OK. >> >>> } >>> - >>> - /* Put legacy shared objects in legacy bitmap. */ >>> - for (i = first_legacy; i <= last_legacy; i++) >>> - { >>> - l = m->l_initfini[i]; >>> - >>> - if (l->l_init_called || (l->l_cet & lc_ibt)) >>> - continue; >>> - >>> -#ifdef SHARED >>> - if (l == &GL(dl_rtld_map) >>> - || l->l_real == &GL(dl_rtld_map) >>> - || (program && l == m)) >>> - continue; >>> -#endif >>> - >>> - /* If IBT is enabled in executable and IBT isn't enabled >>> - in this shard object, mark PT_LOAD segments with PF_X >>> - in legacy code page bitmap. */ >>> - res = dl_cet_mark_legacy_region (l); >>> - if (res != 0) >>> - { >>> - if (program) >>> - _dl_fatal_printf ("%s: failed to mark legacy code region\n", >>> - l->l_name); >>> - else >>> - _dl_signal_error (-res, l->l_name, "dlopen", >>> - N_("failed to mark legacy code region")); >>> - } >>> - } >>> - >>> - /* Change legacy bitmap to read-only. */ >>> - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], >>> - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) >>> - goto mprotect_failure; >> >> OK. >> >>> } >>> } >>> >>> @@ -259,23 +135,40 @@ mprotect_failure: >>> >>> if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) >>> { >>> - if (!program >>> - && enable_shstk_type != CET_PERMISSIVE) >>> + if (!program) >>> { >>> - /* When SHSTK is enabled, we can't dlopening a shared >>> - object without SHSTK. */ >>> - if (enable_shstk != shstk_enabled) >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", >>> - N_("shadow stack isn't enabled")); >>> - return; >>> + if (enable_ibt_type != CET_PERMISSIVE) >>> + { >>> + /* When IBT is enabled, we cannot dlopen a shared >>> + object without IBT. */ >>> + if (found_ibt_legacy) >>> + _dl_signal_error (0, >>> + m->l_initfini[ibt_legacy]->l_name, >>> + "dlopen", >>> + N_("rebuilt with IBT support needed")); >>> + } >> >> Suggest: >> "rebuild shared object with IBT support enabled" > > Fixed. > >> >>> + >>> + if (enable_shstk_type != CET_PERMISSIVE) >>> + { >>> + /* When SHSTK is enabled, we cannot dlopen a shared >>> + object without SHSTK. */ >>> + if (found_shstk_legacy) >>> + _dl_signal_error (0, >>> + m->l_initfini[shstk_legacy]->l_name, >>> + "dlopen", >>> + N_("rebuilt with SHSTK support needed")); >>> + } >> >> Suggest: >> "rebuild shared object with SHSTK support enabled" >> > > Fixed. > >>> + >>> + if (enable_ibt_type != CET_PERMISSIVE >>> + && enable_shstk_type != CET_PERMISSIVE) >>> + return; >>> } >>> >>> /* Disable IBT and/or SHSTK if they are enabled by kernel, but >>> disabled in executable or shared objects. */ >>> unsigned int cet_feature = 0; >>> >>> - /* Disable IBT only during program startup. */ >>> - if (program && !enable_ibt) >>> + if (!enable_ibt) >> >> OK. >> >>> cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; >>> if (!enable_shstk) >>> cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; >>> @@ -286,8 +179,14 @@ mprotect_failure: >>> if (program) >>> _dl_fatal_printf ("%s: can't disable CET\n", program); >>> else >>> - _dl_signal_error (-res, l->l_name, "dlopen", >>> - N_("can't disable CET")); >>> + { >>> + if (found_ibt_legacy) >>> + l = m->l_initfini[ibt_legacy]; >>> + else >>> + l = m->l_initfini[shstk_legacy]; >>> + _dl_signal_error (-res, l->l_name, "dlopen", >>> + N_("can't disable CET")); >> >> OK. >> >>> + } >>> } >>> >>> /* Clear the disabled bits in dl_x86_feature_1. */ >>> @@ -297,17 +196,21 @@ mprotect_failure: >>> } >>> >>> #ifdef SHARED >>> - if (program >>> - && (!shstk_enabled >>> - || enable_shstk_type != CET_PERMISSIVE) >>> - && (ibt_enabled || shstk_enabled)) >>> + if (program && (ibt_enabled || shstk_enabled)) >>> { >>> - /* Lock CET if IBT or SHSTK is enabled in executable. Don't >>> - lock CET if SHSTK is enabled permissively. */ >>> - int res = dl_cet_lock_cet (); >>> - if (res != 0) >>> - _dl_fatal_printf ("%s: can't lock CET\n", program); >>> + if ((!ibt_enabled >>> + || enable_ibt_type != CET_PERMISSIVE) >>> + && (!shstk_enabled >>> + || enable_shstk_type != CET_PERMISSIVE)) >>> + { >>> + /* Lock CET if IBT or SHSTK is enabled in executable unless >>> + IBT or SHSTK is enabled permissively. */ >>> + int res = dl_cet_lock_cet (); >>> + if (res != 0) >>> + _dl_fatal_printf ("%s: can't lock CET\n", program); >> >> OK. >> >>> + } >>> >>> + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ >>> cet_feature_changed = true; >>> } >>> #endif >>> diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c >>> index fb36801f3e..5e39a38133 100644 >>> --- a/sysdeps/x86/dl-procruntime.c >>> +++ b/sysdeps/x86/dl-procruntime.c >>> @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] >>> # else >>> , >>> # endif >>> - >>> -# if !defined PROCINFO_DECL && defined SHARED >>> - ._dl_x86_legacy_bitmap >>> -# else >>> -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] >>> -# endif >>> -# if !defined SHARED || defined PROCINFO_DECL >>> -; >>> -# else >>> -, >>> -# endif >> >> OK. >> >>> #endif >>> diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c >>> index a77078afc9..a922339333 100644 >>> --- a/sysdeps/x86/tst-cet-legacy-4.c >>> +++ b/sysdeps/x86/tst-cet-legacy-4.c >>> @@ -20,6 +20,9 @@ >>> #include <dlfcn.h> >>> #include <stdio.h> >>> #include <stdlib.h> >>> +#include <string.h> >>> + >>> +#include <support/check.h> >>> >>> static int >>> do_test (void) >>> @@ -31,22 +34,18 @@ do_test (void) >>> h = dlopen (modname, RTLD_LAZY); >>> if (h == NULL) >>> { >>> - printf ("cannot open '%s': %s\n", modname, dlerror ()); >>> - exit (1); >>> + const char *err = dlerror (); >>> + if (!strstr (err, "rebuilt with IBT support needed")) >> >> OK (needs adjustment if string is changed). >> > > Fixed. > >>> + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); >>> + return 0; >>> } >>> >>> fp = dlsym (h, "test"); >>> if (fp == NULL) >>> - { >>> - printf ("cannot get symbol 'test': %s\n", dlerror ()); >>> - exit (1); >>> - } >>> + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); >>> >>> if (fp () != 0) >>> - { >>> - puts ("test () != 0"); >>> - exit (1); >>> - } >>> + FAIL_EXIT1 ("test () != 0"); >>> >>> dlclose (h); >>> >>> diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c >>> index 6c9bba06f5..350c7e41df 100644 >>> --- a/sysdeps/x86/tst-cet-legacy-5.c >>> +++ b/sysdeps/x86/tst-cet-legacy-5.c >>> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) >>> if (fail) >>> { >>> const char *err = dlerror (); >>> - if (strstr (err, "shadow stack isn't enabled") == NULL) >>> + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) >> >> OK (also needs adjustment) > > Fixed. > >> >>> { >>> printf ("incorrect dlopen '%s' error: %s\n", modname, >>> err); >>> diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c >>> index 877e77747d..9f2748fcb6 100644 >>> --- a/sysdeps/x86/tst-cet-legacy-6.c >>> +++ b/sysdeps/x86/tst-cet-legacy-6.c >>> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) >>> if (fail) >>> { >>> const char *err = dlerror (); >>> - if (strstr (err, "shadow stack isn't enabled") == NULL) >>> + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) >> >> OK. (also needs adjustment) > > Fixed. > >> >>> { >>> printf ("incorrect dlopen '%s' error: %s\n", modname, >>> err); >>> diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c >>> new file mode 100644 >>> index 0000000000..cf4c2779ce >>> --- /dev/null >>> +++ b/sysdeps/x86/tst-cet-legacy-7.c >>> @@ -0,0 +1,36 @@ >>> +/* Check compatibility of legacy executable with a JIT engine. >>> + Copyright (C) 2020 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 >>> + <https://www.gnu.org/licenses/>. */ >>> + >>> +#include <stdio.h> >>> +#include <sys/mman.h> >>> +#include <support/xunistd.h> >>> + >> >> Needs a pargraph explaining what this test is supposed to do. >> > > Done. > >> I expect this test is supposed to just pass and not fail because it's >> run with -fcf-protection=none. > > Yes. > >> >> What about the equivalent test to test for failure? > > Added. > >> >>> +static int >>> +do_test (void) >>> +{ >>> + void (*funcp) (void); >>> + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, >>> + MAP_ANONYMOUS | MAP_PRIVATE, -1); >>> + printf ("mmap = %p\n", funcp); >>> + /* Write RET instruction. */ >>> + *(char *) funcp = 0xc3; >>> + funcp (); >>> + return 0; >> >> OK. >> >>> +} >>> + >>> +#include <support/test-driver.c> >>> -- 2.24.1 >> >> > > OK for master? Thank you for the extra test. OK if you use EXPECTED_SIGNAL for the new test. Reviewed-by: Carlos O'Donell <carlos@redhat.com> > Thanks. > > H.J. > --- > Since legacy bitmap doesn't cover jitted code generated by legacy JIT > engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP > and treats indirect branch tracking similar to shadow stack by removing > legacy bitmap support. > --- > sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- > .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - > sysdeps/x86/Makefile | 9 +- > sysdeps/x86/dl-cet.c | 217 +++++------------- > sysdeps/x86/dl-procruntime.c | 11 - > sysdeps/x86/tst-cet-legacy-4.c | 19 +- > sysdeps/x86/tst-cet-legacy-5.c | 3 +- > sysdeps/x86/tst-cet-legacy-6.c | 3 +- > sysdeps/x86/tst-cet-legacy-7.c | 38 +++ > sysdeps/x86/tst-cet-legacy-8.c | 55 +++++ > 10 files changed, 172 insertions(+), 208 deletions(-) > create mode 100644 sysdeps/x86/tst-cet-legacy-7.c > create mode 100644 sysdeps/x86/tst-cet-legacy-8.c > > diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h > index 9b2aaa238c..ae97a433a2 100644 > --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h > +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h > @@ -18,26 +18,6 @@ > #include <sys/prctl.h> > #include <asm/prctl.h> > > -static inline int __attribute__ ((always_inline)) > -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) > -{ > - /* Allocate legacy bitmap. */ > -#ifdef __LP64__ > - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); > -#else > - unsigned long long legacy_bitmap_u64[2]; > - int res = INTERNAL_SYSCALL_CALL (arch_prctl, > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); > - if (res == 0) > - { > - legacy_bitmap[0] = legacy_bitmap_u64[0]; > - legacy_bitmap[1] = legacy_bitmap_u64[1]; > - } > - return res; > -#endif > -} > - > static inline int __attribute__ ((always_inline)) > dl_cet_disable_cet (unsigned int cet_feature) > { > diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > index f67f3299b9..45ad0b052f 100644 > --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > @@ -24,9 +24,4 @@ > OUT: allocated shadow stack address: *addr. > */ > # define ARCH_CET_ALLOC_SHSTK 0x3004 > -/* Return legacy region bitmap info in unsigned long long *addr: > - address: addr[0]. > - size: addr[1]. > - */ > -# define ARCH_CET_LEGACY_BITMAP 0x3005 > #endif /* ARCH_CET_STATUS */ > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > index 95182a508c..4ffa699e5f 100644 > --- a/sysdeps/x86/Makefile > +++ b/sysdeps/x86/Makefile > @@ -20,7 +20,8 @@ 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-5a tst-cet-legacy-6a > + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \ > + tst-cet-legacy-8 > 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 \ > @@ -43,14 +44,16 @@ 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-5a.c += -fcf-protection=branch > CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection > CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection > CFLAGS-tst-cet-legacy-6a.c += -fcf-protection > CFLAGS-tst-cet-legacy-6b.c += -fcf-protection > -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none > +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch > CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection > CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection > +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none > +CFLAGS-tst-cet-legacy-8.c += -mshstk OK. > > $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ > $(objpfx)tst-cet-legacy-mod-2.so > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > index ca3b5849bc..c7029f1b51 100644 > --- a/sysdeps/x86/dl-cet.c > +++ b/sysdeps/x86/dl-cet.c > @@ -33,63 +33,6 @@ > # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK > #endif > > -static int > -dl_cet_mark_legacy_region (struct link_map *l) > -{ > - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ > - size_t i, phnum = l->l_phnum; > - const ElfW(Phdr) *phdr = l->l_phdr; > -#ifdef __x86_64__ > - typedef unsigned long long word_t; > -#else > - typedef unsigned long word_t; > -#endif > - unsigned int bits_to_set; > - word_t mask_to_set; > -#define BITS_PER_WORD (sizeof (word_t) * 8) > -#define BITMAP_FIRST_WORD_MASK(start) \ > - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) > -#define BITMAP_LAST_WORD_MASK(nbits) \ > - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) > - > - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; > - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; > - word_t *p; > - size_t page_size = GLRO(dl_pagesize); > - > - for (i = 0; i < phnum; i++) > - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) > - { > - /* One bit in legacy bitmap represents a page. */ > - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; > - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; > - ElfW(Addr) end = start + len; > - > - if ((end / 8) > bitmap_size) > - return -EINVAL; > - > - p = bitmap + (start / BITS_PER_WORD); > - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); > - mask_to_set = BITMAP_FIRST_WORD_MASK (start); > - > - while (len >= bits_to_set) > - { > - *p |= mask_to_set; > - len -= bits_to_set; > - bits_to_set = BITS_PER_WORD; > - mask_to_set = ~((word_t) 0); > - p++; > - } > - if (len) > - { > - mask_to_set &= BITMAP_LAST_WORD_MASK (end); > - *p |= mask_to_set; > - } > - } > - > - return 0; > -} > - > /* Check if object M is compatible with CET. */ > > static void > @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) > if (ibt_enabled || shstk_enabled) > { > struct link_map *l = NULL; > + unsigned int ibt_legacy = 0, shstk_legacy = 0; > + bool found_ibt_legacy = false, found_shstk_legacy = false; > > /* Check if IBT and SHSTK are enabled in object. */ > bool enable_ibt = (ibt_enabled > @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) > support IBT nor SHSTK. */ > if (enable_ibt || enable_shstk) > { > - int res; > unsigned int i; > - unsigned int first_legacy, last_legacy; > - bool need_legacy_bitmap = false; > > i = m->l_searchlist.r_nlist; > while (i-- > 0) > @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) > continue; > #endif > > - if (enable_ibt > - && enable_ibt_type != CET_ALWAYS_ON > - && !(l->l_cet & lc_ibt)) > + /* IBT is enabled only if it is enabled in executable as > + well as all shared objects. */ > + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON > + || (l->l_cet & lc_ibt) != 0); > + if (!found_ibt_legacy && enable_ibt != ibt_enabled) > { > - /* Remember the first and last legacy objects. */ > - if (!need_legacy_bitmap) > - last_legacy = i; > - first_legacy = i; > - need_legacy_bitmap = true; > + found_ibt_legacy = true; > + ibt_legacy = i; > } > > /* SHSTK is enabled only if it is enabled in executable as > well as all shared objects. */ > enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON > || (l->l_cet & lc_shstk) != 0); > - } > - > - if (need_legacy_bitmap) > - { > - if (GL(dl_x86_legacy_bitmap)[0]) > - { > - /* Change legacy bitmap to writable. */ > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > - GL(dl_x86_legacy_bitmap)[1], > - PROT_READ | PROT_WRITE) < 0) > - { > -mprotect_failure: > - if (program) > - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", > - l->l_name); > - else > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > - N_("mprotect legacy bitmap failed")); > - } > - } > - else > + if (enable_shstk != shstk_enabled) > { > - /* Allocate legacy bitmap. */ > - int res = dl_cet_allocate_legacy_bitmap > - (GL(dl_x86_legacy_bitmap)); > - if (res != 0) > - { > - if (program) > - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", > - l->l_name); > - else > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > - N_("legacy bitmap isn't available")); > - } > + found_shstk_legacy = true; > + shstk_legacy = i; > } > - > - /* Put legacy shared objects in legacy bitmap. */ > - for (i = first_legacy; i <= last_legacy; i++) > - { > - l = m->l_initfini[i]; > - > - if (l->l_init_called || (l->l_cet & lc_ibt)) > - continue; > - > -#ifdef SHARED > - if (l == &GL(dl_rtld_map) > - || l->l_real == &GL(dl_rtld_map) > - || (program && l == m)) > - continue; > -#endif > - > - /* If IBT is enabled in executable and IBT isn't enabled > - in this shard object, mark PT_LOAD segments with PF_X > - in legacy code page bitmap. */ > - res = dl_cet_mark_legacy_region (l); > - if (res != 0) > - { > - if (program) > - _dl_fatal_printf ("%s: failed to mark legacy code region\n", > - l->l_name); > - else > - _dl_signal_error (-res, l->l_name, "dlopen", > - N_("failed to mark legacy code region")); > - } > - } > - > - /* Change legacy bitmap to read-only. */ > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) > - goto mprotect_failure; > } > } > > @@ -259,23 +135,40 @@ mprotect_failure: > > if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) > { > - if (!program > - && enable_shstk_type != CET_PERMISSIVE) > + if (!program) > { > - /* When SHSTK is enabled, we can't dlopening a shared > - object without SHSTK. */ > - if (enable_shstk != shstk_enabled) > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > - N_("shadow stack isn't enabled")); > - return; > + if (enable_ibt_type != CET_PERMISSIVE) > + { > + /* When IBT is enabled, we cannot dlopen a shared > + object without IBT. */ > + if (found_ibt_legacy) > + _dl_signal_error (0, > + m->l_initfini[ibt_legacy]->l_name, > + "dlopen", > + N_("rebuild shared object with IBT support enabled")); > + } > + > + if (enable_shstk_type != CET_PERMISSIVE) > + { > + /* When SHSTK is enabled, we cannot dlopen a shared > + object without SHSTK. */ > + if (found_shstk_legacy) > + _dl_signal_error (0, > + m->l_initfini[shstk_legacy]->l_name, > + "dlopen", > + N_("rebuild shared object with SHSTK support enabled")); > + } > + > + if (enable_ibt_type != CET_PERMISSIVE > + && enable_shstk_type != CET_PERMISSIVE) > + return; > } > > /* Disable IBT and/or SHSTK if they are enabled by kernel, but > disabled in executable or shared objects. */ > unsigned int cet_feature = 0; > > - /* Disable IBT only during program startup. */ > - if (program && !enable_ibt) > + if (!enable_ibt) > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; > if (!enable_shstk) > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; > @@ -286,8 +179,14 @@ mprotect_failure: > if (program) > _dl_fatal_printf ("%s: can't disable CET\n", program); > else > - _dl_signal_error (-res, l->l_name, "dlopen", > - N_("can't disable CET")); > + { > + if (found_ibt_legacy) > + l = m->l_initfini[ibt_legacy]; > + else > + l = m->l_initfini[shstk_legacy]; > + _dl_signal_error (-res, l->l_name, "dlopen", > + N_("can't disable CET")); > + } > } > > /* Clear the disabled bits in dl_x86_feature_1. */ > @@ -297,17 +196,21 @@ mprotect_failure: > } > > #ifdef SHARED > - if (program > - && (!shstk_enabled > - || enable_shstk_type != CET_PERMISSIVE) > - && (ibt_enabled || shstk_enabled)) > + if (program && (ibt_enabled || shstk_enabled)) > { > - /* Lock CET if IBT or SHSTK is enabled in executable. Don't > - lock CET if SHSTK is enabled permissively. */ > - int res = dl_cet_lock_cet (); > - if (res != 0) > - _dl_fatal_printf ("%s: can't lock CET\n", program); > + if ((!ibt_enabled > + || enable_ibt_type != CET_PERMISSIVE) > + && (!shstk_enabled > + || enable_shstk_type != CET_PERMISSIVE)) > + { > + /* Lock CET if IBT or SHSTK is enabled in executable unless > + IBT or SHSTK is enabled permissively. */ > + int res = dl_cet_lock_cet (); > + if (res != 0) > + _dl_fatal_printf ("%s: can't lock CET\n", program); > + } > > + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ > cet_feature_changed = true; > } > #endif > diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c > index fb36801f3e..5e39a38133 100644 > --- a/sysdeps/x86/dl-procruntime.c > +++ b/sysdeps/x86/dl-procruntime.c > @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] > # else > , > # endif > - > -# if !defined PROCINFO_DECL && defined SHARED > - ._dl_x86_legacy_bitmap > -# else > -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] > -# endif > -# if !defined SHARED || defined PROCINFO_DECL > -; > -# else > -, > -# endif > #endif > diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c > index a77078afc9..ee54b878ed 100644 > --- a/sysdeps/x86/tst-cet-legacy-4.c > +++ b/sysdeps/x86/tst-cet-legacy-4.c > @@ -20,6 +20,9 @@ > #include <dlfcn.h> > #include <stdio.h> > #include <stdlib.h> > +#include <string.h> > + > +#include <support/check.h> > > static int > do_test (void) > @@ -31,22 +34,18 @@ do_test (void) > h = dlopen (modname, RTLD_LAZY); > if (h == NULL) > { > - printf ("cannot open '%s': %s\n", modname, dlerror ()); > - exit (1); > + const char *err = dlerror (); > + if (!strstr (err, "rebuild shared object with IBT support enabled")) > + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); > + return 0; > } > > fp = dlsym (h, "test"); > if (fp == NULL) > - { > - printf ("cannot get symbol 'test': %s\n", dlerror ()); > - exit (1); > - } > + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); > > if (fp () != 0) > - { > - puts ("test () != 0"); > - exit (1); > - } > + FAIL_EXIT1 ("test () != 0"); > > dlclose (h); > > diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c > index 6c9bba06f5..e2e95b6749 100644 > --- a/sysdeps/x86/tst-cet-legacy-5.c > +++ b/sysdeps/x86/tst-cet-legacy-5.c > @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) > if (fail) > { > const char *err = dlerror (); > - if (strstr (err, "shadow stack isn't enabled") == NULL) > + if (strstr (err, "rebuild shared object with SHSTK support enabled") > + == NULL) > { > printf ("incorrect dlopen '%s' error: %s\n", modname, > err); > diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c > index 877e77747d..bdbbb9075f 100644 > --- a/sysdeps/x86/tst-cet-legacy-6.c > +++ b/sysdeps/x86/tst-cet-legacy-6.c > @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) > if (fail) > { > const char *err = dlerror (); > - if (strstr (err, "shadow stack isn't enabled") == NULL) > + if (strstr (err, "rebuild shared object with SHSTK support enabled") > + == NULL) > { > printf ("incorrect dlopen '%s' error: %s\n", modname, > err); > diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c > new file mode 100644 > index 0000000000..58bcb29a5f > --- /dev/null > +++ b/sysdeps/x86/tst-cet-legacy-7.c > @@ -0,0 +1,38 @@ > +/* Check compatibility of legacy executable with a JIT engine. > + Copyright (C) 2020 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <sys/mman.h> > +#include <support/xunistd.h> > + > +/* Check that mmapped legacy code works with -fcf-protection=none. */ > + > +static int > +do_test (void) > +{ > + void (*funcp) (void); > + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, -1); > + printf ("mmap = %p\n", funcp); > + /* Write RET instruction. */ > + *(char *) funcp = 0xc3; > + funcp (); > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c > new file mode 100644 > index 0000000000..b14b2a7290 > --- /dev/null > +++ b/sysdeps/x86/tst-cet-legacy-8.c > @@ -0,0 +1,55 @@ > +/* Check incompatibility with legacy JIT engine. > + Copyright (C) 2020 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 > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <x86intrin.h> > +#include <sys/mman.h> > +#include <support/test-driver.h> > +#include <support/xsignal.h> > +#include <support/xunistd.h> > + > +/* Check that mmapped legacy code trigges segfault with -fcf-protection. */ > + > +static void > +segfault_handler (int signo) > +{ > + exit (0); > +} > + > +static int > +do_test (void) > +{ > + /* NB: This test should trigger SIGSEGV on CET platforms. If SHSTK > + is disabled, assuming IBT is also disabled. */ > + if (_get_ssp () == 0) > + return EXIT_UNSUPPORTED; > + > + xsignal (SIGSEGV, segfault_handler); Please use EXPECTED_SIGNAL. > + > + void (*funcp) (void); > + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > + MAP_ANONYMOUS | MAP_PRIVATE, -1); > + printf ("mmap = %p\n", funcp); > + /* Write RET instruction. */ > + *(char *) funcp = 0xc3; > + funcp (); > + return EXIT_FAILURE; > +} > + > +#include <support/test-driver.c> >
On Tue, Mar 17, 2020 at 8:20 PM Carlos O'Donell <carlos@redhat.com> wrote: > > On 3/16/20 8:58 PM, H.J. Lu wrote: > > On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote: > >> On 3/13/20 9:25 AM, H.J. Lu via Libc-alpha wrote: > >>> On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote: > >>>> On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote: > >>>>> * H. J. Lu: > >>>>> > >>>>>>> I think the error message should say *where* indirect branch tracking > >>>>>>> is not enabled. I assume this is a property of the loaded object. > >>>>>> Fixed. Now I got > >>>>>> > >>>>>> .... libvirtd[1048]: internal error: Failed to load module > >>>>>> '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so': > >>>>>> /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't > >>>>>> enabled: Invalid argument > >>>>> What I meant is that the error message should say that the object > >>>>> needs to be rebuilt with IBT/SHSTK support. In the above, it's > >>>>> unclear whether the process or the object has to enable IBT. > >>>>> > >>>>> (The Invalid Argument part should also be suppressed.) > >>>> Like this? The diff against V2 patch is: > >>>> > >>>> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > >>>> index f527a1414c..b2843488be 100644 > >>>> --- a/sysdeps/x86/dl-cet.c > >>>> +++ b/sysdeps/x86/dl-cet.c > >>>> @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program) > >>>> /* When IBT is enabled, we cannot dlopen a shared > >>>> object without IBT. */ > >>>> if (found_ibt_legacy) > >>>> - _dl_signal_error (EINVAL, > >>>> + _dl_signal_error (0, > >>>> m->l_initfini[ibt_legacy]->l_name, > >>>> "dlopen", > >>>> - N_("indirect branch tracking isn't enabled")); > >>>> + N_("rebuilt with IBT support needed")); > >>>> } > >>>> > >>>> if (enable_shstk_type != CET_PERMISSIVE) > >>>> @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program) > >>>> /* When SHSTK is enabled, we cannot dlopen a shared > >>>> object without SHSTK. */ > >>>> if (found_shstk_legacy) > >>>> - _dl_signal_error (EINVAL, > >>>> + _dl_signal_error (0, > >>>> m->l_initfini[shstk_legacy]->l_name, > >>>> "dlopen", > >>>> - N_("shadow stack isn't enabled")); > >>>> + N_("rebuilt with SHSTK support needed")); > >>>> } > >>>> > >>>> if (enable_ibt_type != CET_PERMISSIVE > >>>> > >>> PING. > >> > >> Please post v4 with adjusted error message, and expanded comment in new test, > >> and I'll review that quickly. > >> > >>> H.J. > >>> --- > >>> Since legacy bitmap doesn't cover jitted code generated by legacy JIT > >>> engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP > >>> and treats indirect branch tracking similar to shadow stack by removing > >>> legacy bitmap support. > >> > >> OK. > >> > >> > >>> > >>> * sysdeps/unix/sysv/linux/x86/dl-cet.h > >>> (dl_cet_allocate_legacy_bitmap): Removed. > >>> * sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > >>> (ARCH_CET_LEGACY_BITMAP): Removed. > >>> * sysdeps/x86/Makefile (tests): Add tst-cet-legacy-7. > >>> (CFLAGS-tst-cet-legacy-mod-5a.c): Changed to > >>> -fcf-protection=branch. > >>> (CFLAGS-tst-cet-legacy-mod-6a.c): Likewise. > >>> (CFLAGS-tst-cet-legacy-7.c): New. > >>> * sysdeps/x86/dl-cet.c (dl_cet_mark_legacy_region): Removed. > >>> (dl_cet_check): Remove legacy bitmap support. Don't allow > >>> dlopening legacy shared library when IBT is enabled. Set > >>> feature_1 if IBT or SHSTK is enabled in executable. > >>> * sysdeps/x86/dl-procruntime.c (_dl_x86_legacy_bitmap): Removed. > >>> * sysdeps/x86/tst-cet-legacy-4.c: Include <string.h> and > >>> <support/check.h>. > >>> (do_test): Check indirect branch tracking error. Use > >>> FAIL_EXIT1. > >>> * sysdeps/x86/tst-cet-legacy-7.c: New file. > >> > >> Hopefully you don't plan to include this in the commit log :-) > >> > > > > Leftover from my old version :-). > > > >>> --- > >>> sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- > >>> .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - > >>> sysdeps/x86/Makefile | 7 +- > >>> sysdeps/x86/dl-cet.c | 217 +++++------------- > >>> sysdeps/x86/dl-procruntime.c | 11 - > >>> sysdeps/x86/tst-cet-legacy-4.c | 19 +- > >>> sysdeps/x86/tst-cet-legacy-5.c | 2 +- > >>> sysdeps/x86/tst-cet-legacy-6.c | 2 +- > >>> sysdeps/x86/tst-cet-legacy-7.c | 36 +++ > >>> 9 files changed, 111 insertions(+), 208 deletions(-) > >>> create mode 100644 sysdeps/x86/tst-cet-legacy-7.c > >>> > >>> diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h > >>> index 9b2aaa238c..ae97a433a2 100644 > >>> --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h > >>> +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h > >>> @@ -18,26 +18,6 @@ > >>> #include <sys/prctl.h> > >>> #include <asm/prctl.h> > >>> > >>> -static inline int __attribute__ ((always_inline)) > >>> -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) > >>> -{ > >>> - /* Allocate legacy bitmap. */ > >>> -#ifdef __LP64__ > >>> - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, > >>> - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); > >>> -#else > >>> - unsigned long long legacy_bitmap_u64[2]; > >>> - int res = INTERNAL_SYSCALL_CALL (arch_prctl, > >>> - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); > >>> - if (res == 0) > >>> - { > >>> - legacy_bitmap[0] = legacy_bitmap_u64[0]; > >>> - legacy_bitmap[1] = legacy_bitmap_u64[1]; > >>> - } > >>> - return res; > >>> -#endif > >>> -} > >>> - > >> > >> OK. > >> > >>> static inline int __attribute__ ((always_inline)) > >>> dl_cet_disable_cet (unsigned int cet_feature) > >>> { > >>> diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > >>> index f67f3299b9..45ad0b052f 100644 > >>> --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > >>> +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > >>> @@ -24,9 +24,4 @@ > >>> OUT: allocated shadow stack address: *addr. > >>> */ > >>> # define ARCH_CET_ALLOC_SHSTK 0x3004 > >>> -/* Return legacy region bitmap info in unsigned long long *addr: > >>> - address: addr[0]. > >>> - size: addr[1]. > >>> - */ > >>> -# define ARCH_CET_LEGACY_BITMAP 0x3005 > >> > >> OK. > >> > >>> #endif /* ARCH_CET_STATUS */ > >>> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > >>> index 95182a508c..ec96b59a78 100644 > >>> --- a/sysdeps/x86/Makefile > >>> +++ b/sysdeps/x86/Makefile > >>> @@ -20,7 +20,7 @@ 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-5a tst-cet-legacy-6a > >>> + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 > >> > >> OK. > >> > >>> 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 \ > >>> @@ -43,14 +43,15 @@ 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-5a.c += -fcf-protection=branch > >>> CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection > >>> CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection > >>> CFLAGS-tst-cet-legacy-6a.c += -fcf-protection > >>> CFLAGS-tst-cet-legacy-6b.c += -fcf-protection > >>> -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none > >>> +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch > >>> CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection > >>> CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection > >>> +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none > >> > >> OK. > >> > >>> > >>> $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ > >>> $(objpfx)tst-cet-legacy-mod-2.so > >>> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > >>> index ca3b5849bc..b2843488be 100644 > >>> --- a/sysdeps/x86/dl-cet.c > >>> +++ b/sysdeps/x86/dl-cet.c > >>> @@ -33,63 +33,6 @@ > >>> # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK > >>> #endif > >>> > >>> -static int > >>> -dl_cet_mark_legacy_region (struct link_map *l) > >>> -{ > >>> - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ > >>> - size_t i, phnum = l->l_phnum; > >>> - const ElfW(Phdr) *phdr = l->l_phdr; > >>> -#ifdef __x86_64__ > >>> - typedef unsigned long long word_t; > >>> -#else > >>> - typedef unsigned long word_t; > >>> -#endif > >>> - unsigned int bits_to_set; > >>> - word_t mask_to_set; > >>> -#define BITS_PER_WORD (sizeof (word_t) * 8) > >>> -#define BITMAP_FIRST_WORD_MASK(start) \ > >>> - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) > >>> -#define BITMAP_LAST_WORD_MASK(nbits) \ > >>> - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) > >>> - > >>> - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; > >>> - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; > >>> - word_t *p; > >>> - size_t page_size = GLRO(dl_pagesize); > >>> - > >>> - for (i = 0; i < phnum; i++) > >>> - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) > >>> - { > >>> - /* One bit in legacy bitmap represents a page. */ > >>> - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; > >>> - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; > >>> - ElfW(Addr) end = start + len; > >>> - > >>> - if ((end / 8) > bitmap_size) > >>> - return -EINVAL; > >>> - > >>> - p = bitmap + (start / BITS_PER_WORD); > >>> - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); > >>> - mask_to_set = BITMAP_FIRST_WORD_MASK (start); > >>> - > >>> - while (len >= bits_to_set) > >>> - { > >>> - *p |= mask_to_set; > >>> - len -= bits_to_set; > >>> - bits_to_set = BITS_PER_WORD; > >>> - mask_to_set = ~((word_t) 0); > >>> - p++; > >>> - } > >>> - if (len) > >>> - { > >>> - mask_to_set &= BITMAP_LAST_WORD_MASK (end); > >>> - *p |= mask_to_set; > >>> - } > >>> - } > >>> - > >>> - return 0; > >>> -} > >>> - > >> > >> OK. > >> > >>> /* Check if object M is compatible with CET. */ > >>> > >>> static void > >>> @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) > >>> if (ibt_enabled || shstk_enabled) > >>> { > >>> struct link_map *l = NULL; > >>> + unsigned int ibt_legacy = 0, shstk_legacy = 0; > >>> + bool found_ibt_legacy = false, found_shstk_legacy = false; > >> > >> OK. > >> > >>> > >>> /* Check if IBT and SHSTK are enabled in object. */ > >>> bool enable_ibt = (ibt_enabled > >>> @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) > >>> support IBT nor SHSTK. */ > >>> if (enable_ibt || enable_shstk) > >>> { > >>> - int res; > >>> unsigned int i; > >>> - unsigned int first_legacy, last_legacy; > >>> - bool need_legacy_bitmap = false; > >> > >> OK. > >> > >>> > >>> i = m->l_searchlist.r_nlist; > >>> while (i-- > 0) > >>> @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) > >>> continue; > >>> #endif > >>> > >>> - if (enable_ibt > >>> - && enable_ibt_type != CET_ALWAYS_ON > >>> - && !(l->l_cet & lc_ibt)) > >>> + /* IBT is enabled only if it is enabled in executable as > >>> + well as all shared objects. */ > >>> + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON > >>> + || (l->l_cet & lc_ibt) != 0); > >>> + if (!found_ibt_legacy && enable_ibt != ibt_enabled) > >>> { > >>> - /* Remember the first and last legacy objects. */ > >>> - if (!need_legacy_bitmap) > >>> - last_legacy = i; > >>> - first_legacy = i; > >>> - need_legacy_bitmap = true; > >>> + found_ibt_legacy = true; > >>> + ibt_legacy = i; > >> > >> OK. > >> > >>> } > >>> > >>> /* SHSTK is enabled only if it is enabled in executable as > >>> well as all shared objects. */ > >>> enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON > >>> || (l->l_cet & lc_shstk) != 0); > >>> - } > >>> - > >>> - if (need_legacy_bitmap) > >>> - { > >>> - if (GL(dl_x86_legacy_bitmap)[0]) > >>> - { > >>> - /* Change legacy bitmap to writable. */ > >>> - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > >>> - GL(dl_x86_legacy_bitmap)[1], > >>> - PROT_READ | PROT_WRITE) < 0) > >>> - { > >>> -mprotect_failure: > >>> - if (program) > >>> - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", > >>> - l->l_name); > >>> - else > >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", > >>> - N_("mprotect legacy bitmap failed")); > >>> - } > >>> - } > >>> - else > >>> + if (enable_shstk != shstk_enabled) > >> > >> OK. > >> > >>> { > >>> - /* Allocate legacy bitmap. */ > >>> - int res = dl_cet_allocate_legacy_bitmap > >>> - (GL(dl_x86_legacy_bitmap)); > >>> - if (res != 0) > >>> - { > >>> - if (program) > >>> - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", > >>> - l->l_name); > >>> - else > >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", > >>> - N_("legacy bitmap isn't available")); > >>> - } > >>> + found_shstk_legacy = true; > >>> + shstk_legacy = i; > >> > >> OK. > >> > >>> } > >>> - > >>> - /* Put legacy shared objects in legacy bitmap. */ > >>> - for (i = first_legacy; i <= last_legacy; i++) > >>> - { > >>> - l = m->l_initfini[i]; > >>> - > >>> - if (l->l_init_called || (l->l_cet & lc_ibt)) > >>> - continue; > >>> - > >>> -#ifdef SHARED > >>> - if (l == &GL(dl_rtld_map) > >>> - || l->l_real == &GL(dl_rtld_map) > >>> - || (program && l == m)) > >>> - continue; > >>> -#endif > >>> - > >>> - /* If IBT is enabled in executable and IBT isn't enabled > >>> - in this shard object, mark PT_LOAD segments with PF_X > >>> - in legacy code page bitmap. */ > >>> - res = dl_cet_mark_legacy_region (l); > >>> - if (res != 0) > >>> - { > >>> - if (program) > >>> - _dl_fatal_printf ("%s: failed to mark legacy code region\n", > >>> - l->l_name); > >>> - else > >>> - _dl_signal_error (-res, l->l_name, "dlopen", > >>> - N_("failed to mark legacy code region")); > >>> - } > >>> - } > >>> - > >>> - /* Change legacy bitmap to read-only. */ > >>> - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > >>> - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) > >>> - goto mprotect_failure; > >> > >> OK. > >> > >>> } > >>> } > >>> > >>> @@ -259,23 +135,40 @@ mprotect_failure: > >>> > >>> if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) > >>> { > >>> - if (!program > >>> - && enable_shstk_type != CET_PERMISSIVE) > >>> + if (!program) > >>> { > >>> - /* When SHSTK is enabled, we can't dlopening a shared > >>> - object without SHSTK. */ > >>> - if (enable_shstk != shstk_enabled) > >>> - _dl_signal_error (EINVAL, l->l_name, "dlopen", > >>> - N_("shadow stack isn't enabled")); > >>> - return; > >>> + if (enable_ibt_type != CET_PERMISSIVE) > >>> + { > >>> + /* When IBT is enabled, we cannot dlopen a shared > >>> + object without IBT. */ > >>> + if (found_ibt_legacy) > >>> + _dl_signal_error (0, > >>> + m->l_initfini[ibt_legacy]->l_name, > >>> + "dlopen", > >>> + N_("rebuilt with IBT support needed")); > >>> + } > >> > >> Suggest: > >> "rebuild shared object with IBT support enabled" > > > > Fixed. > > > >> > >>> + > >>> + if (enable_shstk_type != CET_PERMISSIVE) > >>> + { > >>> + /* When SHSTK is enabled, we cannot dlopen a shared > >>> + object without SHSTK. */ > >>> + if (found_shstk_legacy) > >>> + _dl_signal_error (0, > >>> + m->l_initfini[shstk_legacy]->l_name, > >>> + "dlopen", > >>> + N_("rebuilt with SHSTK support needed")); > >>> + } > >> > >> Suggest: > >> "rebuild shared object with SHSTK support enabled" > >> > > > > Fixed. > > > >>> + > >>> + if (enable_ibt_type != CET_PERMISSIVE > >>> + && enable_shstk_type != CET_PERMISSIVE) > >>> + return; > >>> } > >>> > >>> /* Disable IBT and/or SHSTK if they are enabled by kernel, but > >>> disabled in executable or shared objects. */ > >>> unsigned int cet_feature = 0; > >>> > >>> - /* Disable IBT only during program startup. */ > >>> - if (program && !enable_ibt) > >>> + if (!enable_ibt) > >> > >> OK. > >> > >>> cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; > >>> if (!enable_shstk) > >>> cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; > >>> @@ -286,8 +179,14 @@ mprotect_failure: > >>> if (program) > >>> _dl_fatal_printf ("%s: can't disable CET\n", program); > >>> else > >>> - _dl_signal_error (-res, l->l_name, "dlopen", > >>> - N_("can't disable CET")); > >>> + { > >>> + if (found_ibt_legacy) > >>> + l = m->l_initfini[ibt_legacy]; > >>> + else > >>> + l = m->l_initfini[shstk_legacy]; > >>> + _dl_signal_error (-res, l->l_name, "dlopen", > >>> + N_("can't disable CET")); > >> > >> OK. > >> > >>> + } > >>> } > >>> > >>> /* Clear the disabled bits in dl_x86_feature_1. */ > >>> @@ -297,17 +196,21 @@ mprotect_failure: > >>> } > >>> > >>> #ifdef SHARED > >>> - if (program > >>> - && (!shstk_enabled > >>> - || enable_shstk_type != CET_PERMISSIVE) > >>> - && (ibt_enabled || shstk_enabled)) > >>> + if (program && (ibt_enabled || shstk_enabled)) > >>> { > >>> - /* Lock CET if IBT or SHSTK is enabled in executable. Don't > >>> - lock CET if SHSTK is enabled permissively. */ > >>> - int res = dl_cet_lock_cet (); > >>> - if (res != 0) > >>> - _dl_fatal_printf ("%s: can't lock CET\n", program); > >>> + if ((!ibt_enabled > >>> + || enable_ibt_type != CET_PERMISSIVE) > >>> + && (!shstk_enabled > >>> + || enable_shstk_type != CET_PERMISSIVE)) > >>> + { > >>> + /* Lock CET if IBT or SHSTK is enabled in executable unless > >>> + IBT or SHSTK is enabled permissively. */ > >>> + int res = dl_cet_lock_cet (); > >>> + if (res != 0) > >>> + _dl_fatal_printf ("%s: can't lock CET\n", program); > >> > >> OK. > >> > >>> + } > >>> > >>> + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ > >>> cet_feature_changed = true; > >>> } > >>> #endif > >>> diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c > >>> index fb36801f3e..5e39a38133 100644 > >>> --- a/sysdeps/x86/dl-procruntime.c > >>> +++ b/sysdeps/x86/dl-procruntime.c > >>> @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] > >>> # else > >>> , > >>> # endif > >>> - > >>> -# if !defined PROCINFO_DECL && defined SHARED > >>> - ._dl_x86_legacy_bitmap > >>> -# else > >>> -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] > >>> -# endif > >>> -# if !defined SHARED || defined PROCINFO_DECL > >>> -; > >>> -# else > >>> -, > >>> -# endif > >> > >> OK. > >> > >>> #endif > >>> diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c > >>> index a77078afc9..a922339333 100644 > >>> --- a/sysdeps/x86/tst-cet-legacy-4.c > >>> +++ b/sysdeps/x86/tst-cet-legacy-4.c > >>> @@ -20,6 +20,9 @@ > >>> #include <dlfcn.h> > >>> #include <stdio.h> > >>> #include <stdlib.h> > >>> +#include <string.h> > >>> + > >>> +#include <support/check.h> > >>> > >>> static int > >>> do_test (void) > >>> @@ -31,22 +34,18 @@ do_test (void) > >>> h = dlopen (modname, RTLD_LAZY); > >>> if (h == NULL) > >>> { > >>> - printf ("cannot open '%s': %s\n", modname, dlerror ()); > >>> - exit (1); > >>> + const char *err = dlerror (); > >>> + if (!strstr (err, "rebuilt with IBT support needed")) > >> > >> OK (needs adjustment if string is changed). > >> > > > > Fixed. > > > >>> + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); > >>> + return 0; > >>> } > >>> > >>> fp = dlsym (h, "test"); > >>> if (fp == NULL) > >>> - { > >>> - printf ("cannot get symbol 'test': %s\n", dlerror ()); > >>> - exit (1); > >>> - } > >>> + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); > >>> > >>> if (fp () != 0) > >>> - { > >>> - puts ("test () != 0"); > >>> - exit (1); > >>> - } > >>> + FAIL_EXIT1 ("test () != 0"); > >>> > >>> dlclose (h); > >>> > >>> diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c > >>> index 6c9bba06f5..350c7e41df 100644 > >>> --- a/sysdeps/x86/tst-cet-legacy-5.c > >>> +++ b/sysdeps/x86/tst-cet-legacy-5.c > >>> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) > >>> if (fail) > >>> { > >>> const char *err = dlerror (); > >>> - if (strstr (err, "shadow stack isn't enabled") == NULL) > >>> + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) > >> > >> OK (also needs adjustment) > > > > Fixed. > > > >> > >>> { > >>> printf ("incorrect dlopen '%s' error: %s\n", modname, > >>> err); > >>> diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c > >>> index 877e77747d..9f2748fcb6 100644 > >>> --- a/sysdeps/x86/tst-cet-legacy-6.c > >>> +++ b/sysdeps/x86/tst-cet-legacy-6.c > >>> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail) > >>> if (fail) > >>> { > >>> const char *err = dlerror (); > >>> - if (strstr (err, "shadow stack isn't enabled") == NULL) > >>> + if (strstr (err, "rebuilt with SHSTK support needed") == NULL) > >> > >> OK. (also needs adjustment) > > > > Fixed. > > > >> > >>> { > >>> printf ("incorrect dlopen '%s' error: %s\n", modname, > >>> err); > >>> diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c > >>> new file mode 100644 > >>> index 0000000000..cf4c2779ce > >>> --- /dev/null > >>> +++ b/sysdeps/x86/tst-cet-legacy-7.c > >>> @@ -0,0 +1,36 @@ > >>> +/* Check compatibility of legacy executable with a JIT engine. > >>> + Copyright (C) 2020 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 > >>> + <https://www.gnu.org/licenses/>. */ > >>> + > >>> +#include <stdio.h> > >>> +#include <sys/mman.h> > >>> +#include <support/xunistd.h> > >>> + > >> > >> Needs a pargraph explaining what this test is supposed to do. > >> > > > > Done. > > > >> I expect this test is supposed to just pass and not fail because it's > >> run with -fcf-protection=none. > > > > Yes. > > > >> > >> What about the equivalent test to test for failure? > > > > Added. > > > >> > >>> +static int > >>> +do_test (void) > >>> +{ > >>> + void (*funcp) (void); > >>> + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > >>> + MAP_ANONYMOUS | MAP_PRIVATE, -1); > >>> + printf ("mmap = %p\n", funcp); > >>> + /* Write RET instruction. */ > >>> + *(char *) funcp = 0xc3; > >>> + funcp (); > >>> + return 0; > >> > >> OK. > >> > >>> +} > >>> + > >>> +#include <support/test-driver.c> > >>> -- 2.24.1 > >> > >> > > > > OK for master? > > Thank you for the extra test. > > OK if you use EXPECTED_SIGNAL for the new test. > > Reviewed-by: Carlos O'Donell <carlos@redhat.com> > > > Thanks. > > > > H.J. > > --- > > Since legacy bitmap doesn't cover jitted code generated by legacy JIT > > engine, it isn't very useful. This patch removes ARCH_CET_LEGACY_BITMAP > > and treats indirect branch tracking similar to shadow stack by removing > > legacy bitmap support. > > --- > > sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 -- > > .../unix/sysv/linux/x86/include/asm/prctl.h | 5 - > > sysdeps/x86/Makefile | 9 +- > > sysdeps/x86/dl-cet.c | 217 +++++------------- > > sysdeps/x86/dl-procruntime.c | 11 - > > sysdeps/x86/tst-cet-legacy-4.c | 19 +- > > sysdeps/x86/tst-cet-legacy-5.c | 3 +- > > sysdeps/x86/tst-cet-legacy-6.c | 3 +- > > sysdeps/x86/tst-cet-legacy-7.c | 38 +++ > > sysdeps/x86/tst-cet-legacy-8.c | 55 +++++ > > 10 files changed, 172 insertions(+), 208 deletions(-) > > create mode 100644 sysdeps/x86/tst-cet-legacy-7.c > > create mode 100644 sysdeps/x86/tst-cet-legacy-8.c > > > > diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h > > index 9b2aaa238c..ae97a433a2 100644 > > --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h > > +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h > > @@ -18,26 +18,6 @@ > > #include <sys/prctl.h> > > #include <asm/prctl.h> > > > > -static inline int __attribute__ ((always_inline)) > > -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) > > -{ > > - /* Allocate legacy bitmap. */ > > -#ifdef __LP64__ > > - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, > > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); > > -#else > > - unsigned long long legacy_bitmap_u64[2]; > > - int res = INTERNAL_SYSCALL_CALL (arch_prctl, > > - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); > > - if (res == 0) > > - { > > - legacy_bitmap[0] = legacy_bitmap_u64[0]; > > - legacy_bitmap[1] = legacy_bitmap_u64[1]; > > - } > > - return res; > > -#endif > > -} > > - > > static inline int __attribute__ ((always_inline)) > > dl_cet_disable_cet (unsigned int cet_feature) > > { > > diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > index f67f3299b9..45ad0b052f 100644 > > --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h > > @@ -24,9 +24,4 @@ > > OUT: allocated shadow stack address: *addr. > > */ > > # define ARCH_CET_ALLOC_SHSTK 0x3004 > > -/* Return legacy region bitmap info in unsigned long long *addr: > > - address: addr[0]. > > - size: addr[1]. > > - */ > > -# define ARCH_CET_LEGACY_BITMAP 0x3005 > > #endif /* ARCH_CET_STATUS */ > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > > index 95182a508c..4ffa699e5f 100644 > > --- a/sysdeps/x86/Makefile > > +++ b/sysdeps/x86/Makefile > > @@ -20,7 +20,8 @@ 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-5a tst-cet-legacy-6a > > + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \ > > + tst-cet-legacy-8 > > 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 \ > > @@ -43,14 +44,16 @@ 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-5a.c += -fcf-protection=branch > > CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection > > CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection > > CFLAGS-tst-cet-legacy-6a.c += -fcf-protection > > CFLAGS-tst-cet-legacy-6b.c += -fcf-protection > > -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none > > +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch > > CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection > > CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection > > +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none > > +CFLAGS-tst-cet-legacy-8.c += -mshstk > > OK. > > > > > $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ > > $(objpfx)tst-cet-legacy-mod-2.so > > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > > index ca3b5849bc..c7029f1b51 100644 > > --- a/sysdeps/x86/dl-cet.c > > +++ b/sysdeps/x86/dl-cet.c > > @@ -33,63 +33,6 @@ > > # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK > > #endif > > > > -static int > > -dl_cet_mark_legacy_region (struct link_map *l) > > -{ > > - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ > > - size_t i, phnum = l->l_phnum; > > - const ElfW(Phdr) *phdr = l->l_phdr; > > -#ifdef __x86_64__ > > - typedef unsigned long long word_t; > > -#else > > - typedef unsigned long word_t; > > -#endif > > - unsigned int bits_to_set; > > - word_t mask_to_set; > > -#define BITS_PER_WORD (sizeof (word_t) * 8) > > -#define BITMAP_FIRST_WORD_MASK(start) \ > > - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) > > -#define BITMAP_LAST_WORD_MASK(nbits) \ > > - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) > > - > > - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; > > - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; > > - word_t *p; > > - size_t page_size = GLRO(dl_pagesize); > > - > > - for (i = 0; i < phnum; i++) > > - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) > > - { > > - /* One bit in legacy bitmap represents a page. */ > > - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; > > - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; > > - ElfW(Addr) end = start + len; > > - > > - if ((end / 8) > bitmap_size) > > - return -EINVAL; > > - > > - p = bitmap + (start / BITS_PER_WORD); > > - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); > > - mask_to_set = BITMAP_FIRST_WORD_MASK (start); > > - > > - while (len >= bits_to_set) > > - { > > - *p |= mask_to_set; > > - len -= bits_to_set; > > - bits_to_set = BITS_PER_WORD; > > - mask_to_set = ~((word_t) 0); > > - p++; > > - } > > - if (len) > > - { > > - mask_to_set &= BITMAP_LAST_WORD_MASK (end); > > - *p |= mask_to_set; > > - } > > - } > > - > > - return 0; > > -} > > - > > /* Check if object M is compatible with CET. */ > > > > static void > > @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) > > if (ibt_enabled || shstk_enabled) > > { > > struct link_map *l = NULL; > > + unsigned int ibt_legacy = 0, shstk_legacy = 0; > > + bool found_ibt_legacy = false, found_shstk_legacy = false; > > > > /* Check if IBT and SHSTK are enabled in object. */ > > bool enable_ibt = (ibt_enabled > > @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) > > support IBT nor SHSTK. */ > > if (enable_ibt || enable_shstk) > > { > > - int res; > > unsigned int i; > > - unsigned int first_legacy, last_legacy; > > - bool need_legacy_bitmap = false; > > > > i = m->l_searchlist.r_nlist; > > while (i-- > 0) > > @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) > > continue; > > #endif > > > > - if (enable_ibt > > - && enable_ibt_type != CET_ALWAYS_ON > > - && !(l->l_cet & lc_ibt)) > > + /* IBT is enabled only if it is enabled in executable as > > + well as all shared objects. */ > > + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON > > + || (l->l_cet & lc_ibt) != 0); > > + if (!found_ibt_legacy && enable_ibt != ibt_enabled) > > { > > - /* Remember the first and last legacy objects. */ > > - if (!need_legacy_bitmap) > > - last_legacy = i; > > - first_legacy = i; > > - need_legacy_bitmap = true; > > + found_ibt_legacy = true; > > + ibt_legacy = i; > > } > > > > /* SHSTK is enabled only if it is enabled in executable as > > well as all shared objects. */ > > enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON > > || (l->l_cet & lc_shstk) != 0); > > - } > > - > > - if (need_legacy_bitmap) > > - { > > - if (GL(dl_x86_legacy_bitmap)[0]) > > - { > > - /* Change legacy bitmap to writable. */ > > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > > - GL(dl_x86_legacy_bitmap)[1], > > - PROT_READ | PROT_WRITE) < 0) > > - { > > -mprotect_failure: > > - if (program) > > - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", > > - l->l_name); > > - else > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("mprotect legacy bitmap failed")); > > - } > > - } > > - else > > + if (enable_shstk != shstk_enabled) > > { > > - /* Allocate legacy bitmap. */ > > - int res = dl_cet_allocate_legacy_bitmap > > - (GL(dl_x86_legacy_bitmap)); > > - if (res != 0) > > - { > > - if (program) > > - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", > > - l->l_name); > > - else > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("legacy bitmap isn't available")); > > - } > > + found_shstk_legacy = true; > > + shstk_legacy = i; > > } > > - > > - /* Put legacy shared objects in legacy bitmap. */ > > - for (i = first_legacy; i <= last_legacy; i++) > > - { > > - l = m->l_initfini[i]; > > - > > - if (l->l_init_called || (l->l_cet & lc_ibt)) > > - continue; > > - > > -#ifdef SHARED > > - if (l == &GL(dl_rtld_map) > > - || l->l_real == &GL(dl_rtld_map) > > - || (program && l == m)) > > - continue; > > -#endif > > - > > - /* If IBT is enabled in executable and IBT isn't enabled > > - in this shard object, mark PT_LOAD segments with PF_X > > - in legacy code page bitmap. */ > > - res = dl_cet_mark_legacy_region (l); > > - if (res != 0) > > - { > > - if (program) > > - _dl_fatal_printf ("%s: failed to mark legacy code region\n", > > - l->l_name); > > - else > > - _dl_signal_error (-res, l->l_name, "dlopen", > > - N_("failed to mark legacy code region")); > > - } > > - } > > - > > - /* Change legacy bitmap to read-only. */ > > - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], > > - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) > > - goto mprotect_failure; > > } > > } > > > > @@ -259,23 +135,40 @@ mprotect_failure: > > > > if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) > > { > > - if (!program > > - && enable_shstk_type != CET_PERMISSIVE) > > + if (!program) > > { > > - /* When SHSTK is enabled, we can't dlopening a shared > > - object without SHSTK. */ > > - if (enable_shstk != shstk_enabled) > > - _dl_signal_error (EINVAL, l->l_name, "dlopen", > > - N_("shadow stack isn't enabled")); > > - return; > > + if (enable_ibt_type != CET_PERMISSIVE) > > + { > > + /* When IBT is enabled, we cannot dlopen a shared > > + object without IBT. */ > > + if (found_ibt_legacy) > > + _dl_signal_error (0, > > + m->l_initfini[ibt_legacy]->l_name, > > + "dlopen", > > + N_("rebuild shared object with IBT support enabled")); > > + } > > + > > + if (enable_shstk_type != CET_PERMISSIVE) > > + { > > + /* When SHSTK is enabled, we cannot dlopen a shared > > + object without SHSTK. */ > > + if (found_shstk_legacy) > > + _dl_signal_error (0, > > + m->l_initfini[shstk_legacy]->l_name, > > + "dlopen", > > + N_("rebuild shared object with SHSTK support enabled")); > > + } > > + > > + if (enable_ibt_type != CET_PERMISSIVE > > + && enable_shstk_type != CET_PERMISSIVE) > > + return; > > } > > > > /* Disable IBT and/or SHSTK if they are enabled by kernel, but > > disabled in executable or shared objects. */ > > unsigned int cet_feature = 0; > > > > - /* Disable IBT only during program startup. */ > > - if (program && !enable_ibt) > > + if (!enable_ibt) > > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; > > if (!enable_shstk) > > cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; > > @@ -286,8 +179,14 @@ mprotect_failure: > > if (program) > > _dl_fatal_printf ("%s: can't disable CET\n", program); > > else > > - _dl_signal_error (-res, l->l_name, "dlopen", > > - N_("can't disable CET")); > > + { > > + if (found_ibt_legacy) > > + l = m->l_initfini[ibt_legacy]; > > + else > > + l = m->l_initfini[shstk_legacy]; > > + _dl_signal_error (-res, l->l_name, "dlopen", > > + N_("can't disable CET")); > > + } > > } > > > > /* Clear the disabled bits in dl_x86_feature_1. */ > > @@ -297,17 +196,21 @@ mprotect_failure: > > } > > > > #ifdef SHARED > > - if (program > > - && (!shstk_enabled > > - || enable_shstk_type != CET_PERMISSIVE) > > - && (ibt_enabled || shstk_enabled)) > > + if (program && (ibt_enabled || shstk_enabled)) > > { > > - /* Lock CET if IBT or SHSTK is enabled in executable. Don't > > - lock CET if SHSTK is enabled permissively. */ > > - int res = dl_cet_lock_cet (); > > - if (res != 0) > > - _dl_fatal_printf ("%s: can't lock CET\n", program); > > + if ((!ibt_enabled > > + || enable_ibt_type != CET_PERMISSIVE) > > + && (!shstk_enabled > > + || enable_shstk_type != CET_PERMISSIVE)) > > + { > > + /* Lock CET if IBT or SHSTK is enabled in executable unless > > + IBT or SHSTK is enabled permissively. */ > > + int res = dl_cet_lock_cet (); > > + if (res != 0) > > + _dl_fatal_printf ("%s: can't lock CET\n", program); > > + } > > > > + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ > > cet_feature_changed = true; > > } > > #endif > > diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c > > index fb36801f3e..5e39a38133 100644 > > --- a/sysdeps/x86/dl-procruntime.c > > +++ b/sysdeps/x86/dl-procruntime.c > > @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] > > # else > > , > > # endif > > - > > -# if !defined PROCINFO_DECL && defined SHARED > > - ._dl_x86_legacy_bitmap > > -# else > > -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] > > -# endif > > -# if !defined SHARED || defined PROCINFO_DECL > > -; > > -# else > > -, > > -# endif > > #endif > > diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c > > index a77078afc9..ee54b878ed 100644 > > --- a/sysdeps/x86/tst-cet-legacy-4.c > > +++ b/sysdeps/x86/tst-cet-legacy-4.c > > @@ -20,6 +20,9 @@ > > #include <dlfcn.h> > > #include <stdio.h> > > #include <stdlib.h> > > +#include <string.h> > > + > > +#include <support/check.h> > > > > static int > > do_test (void) > > @@ -31,22 +34,18 @@ do_test (void) > > h = dlopen (modname, RTLD_LAZY); > > if (h == NULL) > > { > > - printf ("cannot open '%s': %s\n", modname, dlerror ()); > > - exit (1); > > + const char *err = dlerror (); > > + if (!strstr (err, "rebuild shared object with IBT support enabled")) > > + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); > > + return 0; > > } > > > > fp = dlsym (h, "test"); > > if (fp == NULL) > > - { > > - printf ("cannot get symbol 'test': %s\n", dlerror ()); > > - exit (1); > > - } > > + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); > > > > if (fp () != 0) > > - { > > - puts ("test () != 0"); > > - exit (1); > > - } > > + FAIL_EXIT1 ("test () != 0"); > > > > dlclose (h); > > > > diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c > > index 6c9bba06f5..e2e95b6749 100644 > > --- a/sysdeps/x86/tst-cet-legacy-5.c > > +++ b/sysdeps/x86/tst-cet-legacy-5.c > > @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) > > if (fail) > > { > > const char *err = dlerror (); > > - if (strstr (err, "shadow stack isn't enabled") == NULL) > > + if (strstr (err, "rebuild shared object with SHSTK support enabled") > > + == NULL) > > { > > printf ("incorrect dlopen '%s' error: %s\n", modname, > > err); > > diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c > > index 877e77747d..bdbbb9075f 100644 > > --- a/sysdeps/x86/tst-cet-legacy-6.c > > +++ b/sysdeps/x86/tst-cet-legacy-6.c > > @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) > > if (fail) > > { > > const char *err = dlerror (); > > - if (strstr (err, "shadow stack isn't enabled") == NULL) > > + if (strstr (err, "rebuild shared object with SHSTK support enabled") > > + == NULL) > > { > > printf ("incorrect dlopen '%s' error: %s\n", modname, > > err); > > diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c > > new file mode 100644 > > index 0000000000..58bcb29a5f > > --- /dev/null > > +++ b/sysdeps/x86/tst-cet-legacy-7.c > > @@ -0,0 +1,38 @@ > > +/* Check compatibility of legacy executable with a JIT engine. > > + Copyright (C) 2020 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 > > + <https://www.gnu.org/licenses/>. */ > > + > > +#include <stdio.h> > > +#include <sys/mman.h> > > +#include <support/xunistd.h> > > + > > +/* Check that mmapped legacy code works with -fcf-protection=none. */ > > + > > +static int > > +do_test (void) > > +{ > > + void (*funcp) (void); > > + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > > + MAP_ANONYMOUS | MAP_PRIVATE, -1); > > + printf ("mmap = %p\n", funcp); > > + /* Write RET instruction. */ > > + *(char *) funcp = 0xc3; > > + funcp (); > > + return 0; > > +} > > + > > +#include <support/test-driver.c> > > diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c > > new file mode 100644 > > index 0000000000..b14b2a7290 > > --- /dev/null > > +++ b/sysdeps/x86/tst-cet-legacy-8.c > > @@ -0,0 +1,55 @@ > > +/* Check incompatibility with legacy JIT engine. > > + Copyright (C) 2020 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 > > + <https://www.gnu.org/licenses/>. */ > > + > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <x86intrin.h> > > +#include <sys/mman.h> > > +#include <support/test-driver.h> > > +#include <support/xsignal.h> > > +#include <support/xunistd.h> > > + > > +/* Check that mmapped legacy code trigges segfault with -fcf-protection. */ > > + > > +static void > > +segfault_handler (int signo) > > +{ > > + exit (0); > > +} > > + > > +static int > > +do_test (void) > > +{ > > + /* NB: This test should trigger SIGSEGV on CET platforms. If SHSTK > > + is disabled, assuming IBT is also disabled. */ > > + if (_get_ssp () == 0) > > + return EXIT_UNSUPPORTED; > > + > > + xsignal (SIGSEGV, segfault_handler); > > Please use EXPECTED_SIGNAL. > > > + > > + void (*funcp) (void); > > + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, > > + MAP_ANONYMOUS | MAP_PRIVATE, -1); > > + printf ("mmap = %p\n", funcp); > > + /* Write RET instruction. */ > > + *(char *) funcp = 0xc3; > > + funcp (); > > + return EXIT_FAILURE; > > +} > > + > > +#include <support/test-driver.c> > > This is the patch I am checking in. Thanks.
On 3/18/20 7:34 AM, H.J. Lu wrote:
> This is the patch I am checking in.
LGTM. Thanks for continuing to add more tests for these behaviours! :-)
diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h index 9b2aaa238c..ae97a433a2 100644 --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h @@ -18,26 +18,6 @@ #include <sys/prctl.h> #include <asm/prctl.h> -static inline int __attribute__ ((always_inline)) -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap) -{ - /* Allocate legacy bitmap. */ -#ifdef __LP64__ - return (int) INTERNAL_SYSCALL_CALL (arch_prctl, - ARCH_CET_LEGACY_BITMAP, legacy_bitmap); -#else - unsigned long long legacy_bitmap_u64[2]; - int res = INTERNAL_SYSCALL_CALL (arch_prctl, - ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64); - if (res == 0) - { - legacy_bitmap[0] = legacy_bitmap_u64[0]; - legacy_bitmap[1] = legacy_bitmap_u64[1]; - } - return res; -#endif -} - static inline int __attribute__ ((always_inline)) dl_cet_disable_cet (unsigned int cet_feature) { diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h index f67f3299b9..45ad0b052f 100644 --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h @@ -24,9 +24,4 @@ OUT: allocated shadow stack address: *addr. */ # define ARCH_CET_ALLOC_SHSTK 0x3004 -/* Return legacy region bitmap info in unsigned long long *addr: - address: addr[0]. - size: addr[1]. - */ -# define ARCH_CET_LEGACY_BITMAP 0x3005 #endif /* ARCH_CET_STATUS */ diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 95182a508c..4ffa699e5f 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -20,7 +20,8 @@ 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-5a tst-cet-legacy-6a + tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7 \ + tst-cet-legacy-8 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 \ @@ -43,14 +44,16 @@ 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-5a.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection CFLAGS-tst-cet-legacy-6a.c += -fcf-protection CFLAGS-tst-cet-legacy-6b.c += -fcf-protection -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none +CFLAGS-tst-cet-legacy-8.c += -mshstk $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \ $(objpfx)tst-cet-legacy-mod-2.so diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c index ca3b5849bc..c7029f1b51 100644 --- a/sysdeps/x86/dl-cet.c +++ b/sysdeps/x86/dl-cet.c @@ -33,63 +33,6 @@ # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK #endif -static int -dl_cet_mark_legacy_region (struct link_map *l) -{ - /* Mark PT_LOAD segments with PF_X in legacy code page bitmap. */ - size_t i, phnum = l->l_phnum; - const ElfW(Phdr) *phdr = l->l_phdr; -#ifdef __x86_64__ - typedef unsigned long long word_t; -#else - typedef unsigned long word_t; -#endif - unsigned int bits_to_set; - word_t mask_to_set; -#define BITS_PER_WORD (sizeof (word_t) * 8) -#define BITMAP_FIRST_WORD_MASK(start) \ - (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1))) -#define BITMAP_LAST_WORD_MASK(nbits) \ - (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1))) - - word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0]; - word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1]; - word_t *p; - size_t page_size = GLRO(dl_pagesize); - - for (i = 0; i < phnum; i++) - if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X)) - { - /* One bit in legacy bitmap represents a page. */ - ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size; - ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size; - ElfW(Addr) end = start + len; - - if ((end / 8) > bitmap_size) - return -EINVAL; - - p = bitmap + (start / BITS_PER_WORD); - bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD); - mask_to_set = BITMAP_FIRST_WORD_MASK (start); - - while (len >= bits_to_set) - { - *p |= mask_to_set; - len -= bits_to_set; - bits_to_set = BITS_PER_WORD; - mask_to_set = ~((word_t) 0); - p++; - } - if (len) - { - mask_to_set &= BITMAP_LAST_WORD_MASK (end); - *p |= mask_to_set; - } - } - - return 0; -} - /* Check if object M is compatible with CET. */ static void @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program) if (ibt_enabled || shstk_enabled) { struct link_map *l = NULL; + unsigned int ibt_legacy = 0, shstk_legacy = 0; + bool found_ibt_legacy = false, found_shstk_legacy = false; /* Check if IBT and SHSTK are enabled in object. */ bool enable_ibt = (ibt_enabled @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program) support IBT nor SHSTK. */ if (enable_ibt || enable_shstk) { - int res; unsigned int i; - unsigned int first_legacy, last_legacy; - bool need_legacy_bitmap = false; i = m->l_searchlist.r_nlist; while (i-- > 0) @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program) continue; #endif - if (enable_ibt - && enable_ibt_type != CET_ALWAYS_ON - && !(l->l_cet & lc_ibt)) + /* IBT is enabled only if it is enabled in executable as + well as all shared objects. */ + enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON + || (l->l_cet & lc_ibt) != 0); + if (!found_ibt_legacy && enable_ibt != ibt_enabled) { - /* Remember the first and last legacy objects. */ - if (!need_legacy_bitmap) - last_legacy = i; - first_legacy = i; - need_legacy_bitmap = true; + found_ibt_legacy = true; + ibt_legacy = i; } /* SHSTK is enabled only if it is enabled in executable as well as all shared objects. */ enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON || (l->l_cet & lc_shstk) != 0); - } - - if (need_legacy_bitmap) - { - if (GL(dl_x86_legacy_bitmap)[0]) - { - /* Change legacy bitmap to writable. */ - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], - GL(dl_x86_legacy_bitmap)[1], - PROT_READ | PROT_WRITE) < 0) - { -mprotect_failure: - if (program) - _dl_fatal_printf ("%s: mprotect legacy bitmap failed\n", - l->l_name); - else - _dl_signal_error (EINVAL, l->l_name, "dlopen", - N_("mprotect legacy bitmap failed")); - } - } - else + if (enable_shstk != shstk_enabled) { - /* Allocate legacy bitmap. */ - int res = dl_cet_allocate_legacy_bitmap - (GL(dl_x86_legacy_bitmap)); - if (res != 0) - { - if (program) - _dl_fatal_printf ("%s: legacy bitmap isn't available\n", - l->l_name); - else - _dl_signal_error (EINVAL, l->l_name, "dlopen", - N_("legacy bitmap isn't available")); - } + found_shstk_legacy = true; + shstk_legacy = i; } - - /* Put legacy shared objects in legacy bitmap. */ - for (i = first_legacy; i <= last_legacy; i++) - { - l = m->l_initfini[i]; - - if (l->l_init_called || (l->l_cet & lc_ibt)) - continue; - -#ifdef SHARED - if (l == &GL(dl_rtld_map) - || l->l_real == &GL(dl_rtld_map) - || (program && l == m)) - continue; -#endif - - /* If IBT is enabled in executable and IBT isn't enabled - in this shard object, mark PT_LOAD segments with PF_X - in legacy code page bitmap. */ - res = dl_cet_mark_legacy_region (l); - if (res != 0) - { - if (program) - _dl_fatal_printf ("%s: failed to mark legacy code region\n", - l->l_name); - else - _dl_signal_error (-res, l->l_name, "dlopen", - N_("failed to mark legacy code region")); - } - } - - /* Change legacy bitmap to read-only. */ - if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0], - GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0) - goto mprotect_failure; } } @@ -259,23 +135,40 @@ mprotect_failure: if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled) { - if (!program - && enable_shstk_type != CET_PERMISSIVE) + if (!program) { - /* When SHSTK is enabled, we can't dlopening a shared - object without SHSTK. */ - if (enable_shstk != shstk_enabled) - _dl_signal_error (EINVAL, l->l_name, "dlopen", - N_("shadow stack isn't enabled")); - return; + if (enable_ibt_type != CET_PERMISSIVE) + { + /* When IBT is enabled, we cannot dlopen a shared + object without IBT. */ + if (found_ibt_legacy) + _dl_signal_error (0, + m->l_initfini[ibt_legacy]->l_name, + "dlopen", + N_("rebuild shared object with IBT support enabled")); + } + + if (enable_shstk_type != CET_PERMISSIVE) + { + /* When SHSTK is enabled, we cannot dlopen a shared + object without SHSTK. */ + if (found_shstk_legacy) + _dl_signal_error (0, + m->l_initfini[shstk_legacy]->l_name, + "dlopen", + N_("rebuild shared object with SHSTK support enabled")); + } + + if (enable_ibt_type != CET_PERMISSIVE + && enable_shstk_type != CET_PERMISSIVE) + return; } /* Disable IBT and/or SHSTK if they are enabled by kernel, but disabled in executable or shared objects. */ unsigned int cet_feature = 0; - /* Disable IBT only during program startup. */ - if (program && !enable_ibt) + if (!enable_ibt) cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT; if (!enable_shstk) cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK; @@ -286,8 +179,14 @@ mprotect_failure: if (program) _dl_fatal_printf ("%s: can't disable CET\n", program); else - _dl_signal_error (-res, l->l_name, "dlopen", - N_("can't disable CET")); + { + if (found_ibt_legacy) + l = m->l_initfini[ibt_legacy]; + else + l = m->l_initfini[shstk_legacy]; + _dl_signal_error (-res, l->l_name, "dlopen", + N_("can't disable CET")); + } } /* Clear the disabled bits in dl_x86_feature_1. */ @@ -297,17 +196,21 @@ mprotect_failure: } #ifdef SHARED - if (program - && (!shstk_enabled - || enable_shstk_type != CET_PERMISSIVE) - && (ibt_enabled || shstk_enabled)) + if (program && (ibt_enabled || shstk_enabled)) { - /* Lock CET if IBT or SHSTK is enabled in executable. Don't - lock CET if SHSTK is enabled permissively. */ - int res = dl_cet_lock_cet (); - if (res != 0) - _dl_fatal_printf ("%s: can't lock CET\n", program); + if ((!ibt_enabled + || enable_ibt_type != CET_PERMISSIVE) + && (!shstk_enabled + || enable_shstk_type != CET_PERMISSIVE)) + { + /* Lock CET if IBT or SHSTK is enabled in executable unless + IBT or SHSTK is enabled permissively. */ + int res = dl_cet_lock_cet (); + if (res != 0) + _dl_fatal_printf ("%s: can't lock CET\n", program); + } + /* Set feature_1 if IBT or SHSTK is enabled in executable. */ cet_feature_changed = true; } #endif diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c index fb36801f3e..5e39a38133 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] # else , # endif - -# if !defined PROCINFO_DECL && defined SHARED - ._dl_x86_legacy_bitmap -# else -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2] -# endif -# if !defined SHARED || defined PROCINFO_DECL -; -# else -, -# endif #endif diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c index a77078afc9..ee54b878ed 100644 --- a/sysdeps/x86/tst-cet-legacy-4.c +++ b/sysdeps/x86/tst-cet-legacy-4.c @@ -20,6 +20,9 @@ #include <dlfcn.h> #include <stdio.h> #include <stdlib.h> +#include <string.h> + +#include <support/check.h> static int do_test (void) @@ -31,22 +34,18 @@ do_test (void) h = dlopen (modname, RTLD_LAZY); if (h == NULL) { - printf ("cannot open '%s': %s\n", modname, dlerror ()); - exit (1); + const char *err = dlerror (); + if (!strstr (err, "rebuild shared object with IBT support enabled")) + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); + return 0; } fp = dlsym (h, "test"); if (fp == NULL) - { - printf ("cannot get symbol 'test': %s\n", dlerror ()); - exit (1); - } + FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ()); if (fp () != 0) - { - puts ("test () != 0"); - exit (1); - } + FAIL_EXIT1 ("test () != 0"); dlclose (h); diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c index 6c9bba06f5..e2e95b6749 100644 --- a/sysdeps/x86/tst-cet-legacy-5.c +++ b/sysdeps/x86/tst-cet-legacy-5.c @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) if (fail) { const char *err = dlerror (); - if (strstr (err, "shadow stack isn't enabled") == NULL) + if (strstr (err, "rebuild shared object with SHSTK support enabled") + == NULL) { printf ("incorrect dlopen '%s' error: %s\n", modname, err); diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c index 877e77747d..bdbbb9075f 100644 --- a/sysdeps/x86/tst-cet-legacy-6.c +++ b/sysdeps/x86/tst-cet-legacy-6.c @@ -35,7 +35,8 @@ do_test_1 (const char *modname, bool fail) if (fail) { const char *err = dlerror (); - if (strstr (err, "shadow stack isn't enabled") == NULL) + if (strstr (err, "rebuild shared object with SHSTK support enabled") + == NULL) { printf ("incorrect dlopen '%s' error: %s\n", modname, err); diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c new file mode 100644 index 0000000000..58bcb29a5f --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-7.c @@ -0,0 +1,38 @@ +/* Check compatibility of legacy executable with a JIT engine. + Copyright (C) 2020 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 + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <sys/mman.h> +#include <support/xunistd.h> + +/* Check that mmapped legacy code works with -fcf-protection=none. */ + +static int +do_test (void) +{ + void (*funcp) (void); + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1); + printf ("mmap = %p\n", funcp); + /* Write RET instruction. */ + *(char *) funcp = 0xc3; + funcp (); + return 0; +} + +#include <support/test-driver.c> diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c new file mode 100644 index 0000000000..b14b2a7290 --- /dev/null +++ b/sysdeps/x86/tst-cet-legacy-8.c @@ -0,0 +1,55 @@ +/* Check incompatibility with legacy JIT engine. + Copyright (C) 2020 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 + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <stdlib.h> +#include <x86intrin.h> +#include <sys/mman.h> +#include <support/test-driver.h> +#include <support/xsignal.h> +#include <support/xunistd.h> + +/* Check that mmapped legacy code trigges segfault with -fcf-protection. */ + +static void +segfault_handler (int signo) +{ + exit (0); +} + +static int +do_test (void) +{ + /* NB: This test should trigger SIGSEGV on CET platforms. If SHSTK + is disabled, assuming IBT is also disabled. */ + if (_get_ssp () == 0) + return EXIT_UNSUPPORTED; + + xsignal (SIGSEGV, segfault_handler); + + void (*funcp) (void); + funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, -1); + printf ("mmap = %p\n", funcp); + /* Write RET instruction. */ + *(char *) funcp = 0xc3; + funcp (); + return EXIT_FAILURE; +} + +#include <support/test-driver.c>