From patchwork Sat May 16 23:44:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 39280 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 144A1385E830; Sat, 16 May 2020 23:44:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 144A1385E830 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1589672685; bh=r6z036+D26gAg5mCuskBl6/7tXmsdqRMrS30RniwlqI=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=p98Mgw8+mD5QTKBV4IhvtcmWlCzTe0qXAZD+co5rmESdIILmhyeOjlDg4tj9t0Mri 3RW+z9VIOH4F9GMyGjkJppGh8CbJqoOJz3rG3M3+l481GUR0BFZr5+bVd1Vo4ZgoUo xtBsU4knuGQWKXetMQ2VkDT5+LUCEfSdsGiWnDFc= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-il1-x144.google.com (mail-il1-x144.google.com [IPv6:2607:f8b0:4864:20::144]) by sourceware.org (Postfix) with ESMTPS id 9F60E385DC1F for ; Sat, 16 May 2020 23:44:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9F60E385DC1F Received: by mail-il1-x144.google.com with SMTP id o67so1696587ila.0 for ; Sat, 16 May 2020 16:44:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=r6z036+D26gAg5mCuskBl6/7tXmsdqRMrS30RniwlqI=; b=Bir4xlnpeRN/tikq1WdofI6Tbxd+KD6jgTmApWGf2tNXbRDYu3zGBtGgVJxpWVHJBa njYxyTrf03N/nFwnQsqZzv6xWUafJroWIFtKGyz0VeiLJeo8y91yWxEd1Im7mJz4/Iww OpBykIea0R5vQEnuHNpsvFxPv6NqrouZZrGaLgy4e1q+VJw8Ap6D9PqE07s5UpQnxGcV QNCOOPu0FfShTaoHs/XY8N+8QG+5jEv5TOf0fIzV70yJ999LSPbEX9GE9xDcaW5auDHj Lt9plPOwmbkFndPTYZ9ey6h1hjwrTqByCQxE2MJ/zr5ewpXuPqXxV1X4DZF8f31H/x4S fYrw== X-Gm-Message-State: AOAM531/r0q0uiYiwCiBh4vEPSiFJPwyE2xU7tLOkB5Gxsxq2npEQA/D LexPyIw71cOEphyryw3xwsDi3deBAuFfaZ3yyH++mw== X-Google-Smtp-Source: ABdhPJwYNpd4vr1P6PMH5ksknVxWH67o1JZKUOON/xds657x29py1u9fnMMekUiF7XRAmBsEtlbt3HzPyr1lnvrkQiU= X-Received: by 2002:a92:6f08:: with SMTP id k8mr9450804ilc.213.1589672680998; Sat, 16 May 2020 16:44:40 -0700 (PDT) MIME-Version: 1.0 References: <20200428215243.236312-1-hjl.tools@gmail.com> <20200428215243.236312-2-hjl.tools@gmail.com> <871rnjoknd.fsf@mid.deneb.enyo.de> In-Reply-To: <871rnjoknd.fsf@mid.deneb.enyo.de> Date: Sat, 16 May 2020 16:44:04 -0700 Message-ID: Subject: [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] To: Florian Weimer X-Spam-Status: No, score=-8.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: "H.J. Lu via Libc-alpha" From: "H.J. Lu" Reply-To: "H.J. Lu" Cc: "H.J. Lu via Libc-alpha" Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Sat, May 16, 2020 at 10:27 AM Florian Weimer wrote: > > * H. J. Lu via Libc-alpha: > > > diff --git a/sysdeps/x86/cet-tunables.h b/sysdeps/x86/cet-tunables.h > > index 5e1e42df10..0088b89d3e 100644 > > --- a/sysdeps/x86/cet-tunables.h > > +++ b/sysdeps/x86/cet-tunables.h > > @@ -16,14 +16,32 @@ > > License along with the GNU C Library; if not, see > > . */ > > > > -/* Valid control values: > > +#ifndef _CET_TUNABLES_H > > +#define _CET_TUNABLES_H > > + > > +/* For each CET feature, IBT and SHSTK, valid control values: > > 0: Enable CET features based on ELF property note. > > 1: Always disable CET features. > > 2: Always enable CET features. > > 3: Enable CET features permissively. > > + > > + Bits 0-1: IBT > > + Bits 2-3: SHSTK > > */ > > #define CET_ELF_PROPERTY 0 > > #define CET_ALWAYS_OFF 1 > > #define CET_ALWAYS_ON 2 > > #define CET_PERMISSIVE 3 > > -#define CET_MAX CET_PERMISSIVE > > +#define CET_CONTROL_MASK 3 > > +#define CET_IBT_SHIFT 0 > > +#define CET_SHSTK_SHIFT 2 > > + > > +/* Get CET control value. */ > > + > > +static inline unsigned int > > +get_cet_control_value (unsigned int shift) > > +{ > > + return (GL(dl_x86_feature_1)[1] >> shift) & CET_CONTROL_MASK; > > +} > > + > > +#endif /* cet-tunables.h */ > > Is there a reason why this has to be a single bitmask? Maybe a > bitfield would better document the intent? Here is the updated patch to use bitfields. OK for master branch? From cd9ccac3dbcb286de61f4123ead91bf893321c13 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Tue, 28 Apr 2020 07:46:17 -0700 Subject: [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 1. Change _dl_x86_feature_1[2] to _dl_x86_feature_1. 2. Add _dl_x86_feature_control after _dl_x86_feature_1, which is a struct of 2 bitfields for IBT and SHSTK control This fixes [BZ #25887]. --- sysdeps/i386/dl-machine.h | 2 +- sysdeps/unix/sysv/linux/x86/cpu-features.c | 2 +- sysdeps/x86/{cet-tunables.h => cet-control.h} | 34 ++++++++++----- sysdeps/x86/cpu-features.c | 12 +++--- sysdeps/x86/cpu-tunables.c | 31 +++----------- sysdeps/x86/dl-cet.c | 42 +++++++++---------- sysdeps/x86/dl-procruntime.c | 19 ++++++++- sysdeps/x86/ldsodefs.h | 1 + 8 files changed, 74 insertions(+), 69 deletions(-) rename sysdeps/x86/{cet-tunables.h => cet-control.h} (61%) diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h index 8af0789a9c..0f08079e48 100644 --- a/sysdeps/i386/dl-machine.h +++ b/sysdeps/i386/dl-machine.h @@ -71,7 +71,7 @@ elf_machine_runtime_setup (struct link_map *l, int lazy, int profile) extern void _dl_runtime_profile_shstk (Elf32_Word) attribute_hidden; /* Check if SHSTK is enabled by kernel. */ bool shstk_enabled - = (GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0; + = (GL(dl_x86_feature_1) & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0; if (l->l_info[DT_JMPREL] && lazy) { diff --git a/sysdeps/unix/sysv/linux/x86/cpu-features.c b/sysdeps/unix/sysv/linux/x86/cpu-features.c index d67b300595..fc0e32827d 100644 --- a/sysdeps/unix/sysv/linux/x86/cpu-features.c +++ b/sysdeps/unix/sysv/linux/x86/cpu-features.c @@ -34,7 +34,7 @@ static inline void x86_setup_tls (void) { __libc_setup_tls (); - THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1)[0]); + THREAD_SETMEM (THREAD_SELF, header.feature_1, GL(dl_x86_feature_1)); } # define ARCH_SETUP_TLS() x86_setup_tls () diff --git a/sysdeps/x86/cet-tunables.h b/sysdeps/x86/cet-control.h similarity index 61% rename from sysdeps/x86/cet-tunables.h rename to sysdeps/x86/cet-control.h index 5e1e42df10..3a314f9609 100644 --- a/sysdeps/x86/cet-tunables.h +++ b/sysdeps/x86/cet-control.h @@ -16,14 +16,26 @@ License along with the GNU C Library; if not, see . */ -/* Valid control values: - 0: Enable CET features based on ELF property note. - 1: Always disable CET features. - 2: Always enable CET features. - 3: Enable CET features permissively. - */ -#define CET_ELF_PROPERTY 0 -#define CET_ALWAYS_OFF 1 -#define CET_ALWAYS_ON 2 -#define CET_PERMISSIVE 3 -#define CET_MAX CET_PERMISSIVE +#ifndef _CET_CONTROL_H +#define _CET_CONTROL_H + +/* For each CET feature, IBT and SHSTK, valid control values */ +enum dl_x86_cet_control +{ + /* Enable CET features based on ELF property note. */ + elf_property = 0, + /* Always enable CET features. */ + always_on, + /* Always disable CET features. */ + always_off, + /* Enable CET features permissively. */ + permissive +}; + +struct dl_x86_feature_control +{ + enum dl_x86_cet_control ibt : 2; + enum dl_x86_cet_control shstk : 2; +}; + +#endif /* cet-control.h */ diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c index bfb415f05a..cf1d84f2af 100644 --- a/sysdeps/x86/cpu-features.c +++ b/sysdeps/x86/cpu-features.c @@ -39,7 +39,6 @@ extern void TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *) #if CET_ENABLED # include -# include #endif static void @@ -620,7 +619,7 @@ no_cpuid: if (cet_status) { - GL(dl_x86_feature_1)[0] = cet_status; + GL(dl_x86_feature_1) = cet_status; # ifndef SHARED /* Check if IBT and SHSTK are enabled by kernel. */ @@ -644,14 +643,13 @@ no_cpuid: /* Clear the disabled bits in dl_x86_feature_1. */ if (res == 0) - GL(dl_x86_feature_1)[0] &= ~cet_feature; + GL(dl_x86_feature_1) &= ~cet_feature; } /* Lock CET if IBT or SHSTK is enabled in executable. Don't - lock CET if SHSTK is enabled permissively. */ - if (((GL(dl_x86_feature_1)[1] >> CET_MAX) - & ((1 << CET_MAX) - 1)) - != CET_PERMISSIVE) + lock CET if IBT or SHSTK is enabled permissively. */ + if (GL(dl_x86_feature_control).ibt != permissive + && GL(dl_x86_feature_control).shstk != permissive) dl_cet_lock_cet (); } # endif diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c index 861bd7bcaa..58abc1ecfc 100644 --- a/sysdeps/x86/cpu-tunables.c +++ b/sysdeps/x86/cpu-tunables.c @@ -336,28 +336,18 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp) } # if CET_ENABLED -# include attribute_hidden void TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp) { if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~((1 << CET_MAX) - 1); - GL(dl_x86_feature_1)[1] |= CET_ALWAYS_ON; - } + GL(dl_x86_feature_control).ibt = always_on; else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~((1 << CET_MAX) - 1); - GL(dl_x86_feature_1)[1] |= CET_ALWAYS_OFF; - } + GL(dl_x86_feature_control).ibt = always_off; else if (DEFAULT_MEMCMP (valp->strval, "permissive", sizeof ("permissive")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~((1 << CET_MAX) - 1); - GL(dl_x86_feature_1)[1] |= CET_PERMISSIVE; - } + GL(dl_x86_feature_control).ibt = permissive; } attribute_hidden @@ -365,21 +355,12 @@ void TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp) { if (DEFAULT_MEMCMP (valp->strval, "on", sizeof ("on")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~(((1 << CET_MAX) - 1) << CET_MAX); - GL(dl_x86_feature_1)[1] |= (CET_ALWAYS_ON << CET_MAX); - } + GL(dl_x86_feature_control).shstk = always_on; else if (DEFAULT_MEMCMP (valp->strval, "off", sizeof ("off")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~(((1 << CET_MAX) - 1) << CET_MAX); - GL(dl_x86_feature_1)[1] |= (CET_ALWAYS_OFF << CET_MAX); - } + GL(dl_x86_feature_control).shstk = always_off; else if (DEFAULT_MEMCMP (valp->strval, "permissive", sizeof ("permissive")) == 0) - { - GL(dl_x86_feature_1)[1] &= ~(((1 << CET_MAX) - 1) << CET_MAX); - GL(dl_x86_feature_1)[1] |= (CET_PERMISSIVE << CET_MAX); - } + GL(dl_x86_feature_control).shstk = permissive; } # endif #endif diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c index c7029f1b51..82880c1999 100644 --- a/sysdeps/x86/dl-cet.c +++ b/sysdeps/x86/dl-cet.c @@ -20,7 +20,6 @@ #include #include #include -#include /* GNU_PROPERTY_X86_FEATURE_1_IBT and GNU_PROPERTY_X86_FEATURE_1_SHSTK are defined in , which are only available for C sources. @@ -39,23 +38,22 @@ static void dl_cet_check (struct link_map *m, const char *program) { /* Check how IBT should be enabled. */ - unsigned int enable_ibt_type - = GL(dl_x86_feature_1)[1] & ((1 << CET_MAX) - 1); + enum dl_x86_cet_control enable_ibt_type + = GL(dl_x86_feature_control).ibt; /* Check how SHSTK should be enabled. */ - unsigned int enable_shstk_type - = ((GL(dl_x86_feature_1)[1] >> CET_MAX) & ((1 << CET_MAX) - 1)); + enum dl_x86_cet_control enable_shstk_type + = GL(dl_x86_feature_control).shstk; /* No legacy object check if both IBT and SHSTK are always on. */ - if (enable_ibt_type == CET_ALWAYS_ON - && enable_shstk_type == CET_ALWAYS_ON) + if (enable_ibt_type == always_on && enable_shstk_type == always_on) return; /* Check if IBT is enabled by kernel. */ bool ibt_enabled - = (GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0; + = (GL(dl_x86_feature_1) & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0; /* Check if SHSTK is enabled by kernel. */ bool shstk_enabled - = (GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0; + = (GL(dl_x86_feature_1) & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0; if (ibt_enabled || shstk_enabled) { @@ -65,9 +63,9 @@ dl_cet_check (struct link_map *m, const char *program) /* Check if IBT and SHSTK are enabled in object. */ bool enable_ibt = (ibt_enabled - && enable_ibt_type != CET_ALWAYS_OFF); + && enable_ibt_type != always_off); bool enable_shstk = (shstk_enabled - && enable_shstk_type != CET_ALWAYS_OFF); + && enable_shstk_type != always_off); if (program) { /* Enable IBT and SHSTK only if they are enabled in executable. @@ -76,10 +74,10 @@ dl_cet_check (struct link_map *m, const char *program) GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK */ enable_ibt &= (HAS_CPU_FEATURE (IBT) - && (enable_ibt_type == CET_ALWAYS_ON + && (enable_ibt_type == always_on || (m->l_cet & lc_ibt) != 0)); enable_shstk &= (HAS_CPU_FEATURE (SHSTK) - && (enable_shstk_type == CET_ALWAYS_ON + && (enable_shstk_type == always_on || (m->l_cet & lc_shstk) != 0)); } @@ -111,7 +109,7 @@ dl_cet_check (struct link_map *m, const char *program) /* IBT is enabled only if it is enabled in executable as well as all shared objects. */ - enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON + enable_ibt &= (enable_ibt_type == always_on || (l->l_cet & lc_ibt) != 0); if (!found_ibt_legacy && enable_ibt != ibt_enabled) { @@ -121,7 +119,7 @@ dl_cet_check (struct link_map *m, const char *program) /* SHSTK is enabled only if it is enabled in executable as well as all shared objects. */ - enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON + enable_shstk &= (enable_shstk_type == always_on || (l->l_cet & lc_shstk) != 0); if (enable_shstk != shstk_enabled) { @@ -137,7 +135,7 @@ dl_cet_check (struct link_map *m, const char *program) { if (!program) { - if (enable_ibt_type != CET_PERMISSIVE) + if (enable_ibt_type != permissive) { /* When IBT is enabled, we cannot dlopen a shared object without IBT. */ @@ -148,7 +146,7 @@ dl_cet_check (struct link_map *m, const char *program) N_("rebuild shared object with IBT support enabled")); } - if (enable_shstk_type != CET_PERMISSIVE) + if (enable_shstk_type != permissive) { /* When SHSTK is enabled, we cannot dlopen a shared object without SHSTK. */ @@ -159,8 +157,8 @@ dl_cet_check (struct link_map *m, const char *program) N_("rebuild shared object with SHSTK support enabled")); } - if (enable_ibt_type != CET_PERMISSIVE - && enable_shstk_type != CET_PERMISSIVE) + if (enable_ibt_type != permissive + && enable_shstk_type != permissive) return; } @@ -190,7 +188,7 @@ dl_cet_check (struct link_map *m, const char *program) } /* Clear the disabled bits in dl_x86_feature_1. */ - GL(dl_x86_feature_1)[0] &= ~cet_feature; + GL(dl_x86_feature_1) &= ~cet_feature; cet_feature_changed = true; } @@ -199,9 +197,9 @@ dl_cet_check (struct link_map *m, const char *program) if (program && (ibt_enabled || shstk_enabled)) { if ((!ibt_enabled - || enable_ibt_type != CET_PERMISSIVE) + || enable_ibt_type != permissive) && (!shstk_enabled - || enable_shstk_type != CET_PERMISSIVE)) + || enable_shstk_type != permissive)) { /* Lock CET if IBT or SHSTK is enabled in executable unless IBT or SHSTK is enabled permissively. */ diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c index 88a7cac646..129f0dd338 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -47,11 +47,26 @@ # if !defined PROCINFO_DECL && defined SHARED ._dl_x86_feature_1 # else -PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] +PROCINFO_CLASS unsigned int _dl_x86_feature_1 +# endif +# ifndef PROCINFO_DECL += 0 +# endif +# if !defined SHARED || defined PROCINFO_DECL +; +# else +, +# endif + +# if !defined PROCINFO_DECL && defined SHARED + ._dl_x86_feature_control +# else +PROCINFO_CLASS struct dl_x86_feature_control _dl_x86_feature_control # endif # ifndef PROCINFO_DECL = { - 0, + .ibt = elf_property, + .shstk = elf_property } # endif # if !defined SHARED || defined PROCINFO_DECL diff --git a/sysdeps/x86/ldsodefs.h b/sysdeps/x86/ldsodefs.h index 072f826666..5b1b09dbf8 100644 --- a/sysdeps/x86/ldsodefs.h +++ b/sysdeps/x86/ldsodefs.h @@ -61,6 +61,7 @@ struct La_x32_retval; struct La_x86_64_retval *, \ const char *) +#include #include_next #endif -- 2.26.2