* [PATCH 0/3] x86: Add --enable-cet=permissive @ 2020-04-28 21:52 H.J. Lu 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu ` (3 more replies) 0 siblings, 4 replies; 21+ messages in thread From: H.J. Lu @ 2020-04-28 21:52 UTC (permalink / raw) To: libc-alpha When CET is enabled, it is an error to dlopen a non CET enabled shared library in CET enabled application. It may be desirable to make CET permissive, that is disable CET when dlopening a non CET enabled shared library. With the new --enable-cet=permissive configure option, CET is disabled when dlopening a non CET enabled shared library. To support --enable-cet=permissive, CET_MAX is renamed to CET_CONTROL_MASK and <dl-procruntime.c> is included in rtld.c to get architecture specific initializer in rtld_global. H.J. Lu (3): CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] rtld: Get architecture specific initializer in rtld_global x86: Add --enable-cet=permissive INSTALL | 26 +++++++++++-------- config.h.in | 3 +++ elf/rtld.c | 2 ++ manual/install.texi | 12 ++++++--- sysdeps/unix/sysv/linux/x86/Makefile | 2 +- sysdeps/x86/Makefile | 18 +++++++++---- sysdeps/x86/cet-tunables.h | 22 ++++++++++++++-- sysdeps/x86/configure | 21 ++++++++------- sysdeps/x86/configure.ac | 19 +++++++------- sysdeps/x86/cpu-features.c | 7 +++-- sysdeps/x86/cpu-tunables.c | 39 +++++++++++----------------- sysdeps/x86/dl-cet.c | 6 ++--- sysdeps/x86/dl-procruntime.c | 5 ++++ sysdeps/x86/tst-cet-legacy-5.c | 25 ++++++++++++------ sysdeps/x86/tst-cet-legacy-6.c | 25 ++++++++++++------ 15 files changed, 141 insertions(+), 91 deletions(-) -- 2.25.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] 2020-04-28 21:52 [PATCH 0/3] x86: Add --enable-cet=permissive H.J. Lu @ 2020-04-28 21:52 ` H.J. Lu 2020-05-16 16:37 ` PING: " H.J. Lu 2020-05-16 17:27 ` Florian Weimer 2020-04-28 21:52 ` [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global H.J. Lu ` (2 subsequent siblings) 3 siblings, 2 replies; 21+ messages in thread From: H.J. Lu @ 2020-04-28 21:52 UTC (permalink / raw) To: libc-alpha _dl_x86_feature_1[1] is used to control 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. */ #define CET_ELF_PROPERTY 0 #define CET_ALWAYS_OFF 1 #define CET_ALWAYS_ON 2 #define CET_PERMISSIVE 3 #define CET_MAX CET_PERMISSIVE CET control value takes 2 bits. Rename CET_MAX to CET_CONTROL_MASK. Add CET_IBT_SHIFT and CET_SHSTK_SHIFT. --- sysdeps/x86/cet-tunables.h | 22 +++++++++++++++++++-- sysdeps/x86/cpu-features.c | 7 +++---- sysdeps/x86/cpu-tunables.c | 39 +++++++++++++++----------------------- sysdeps/x86/dl-cet.c | 6 ++---- 4 files changed, 40 insertions(+), 34 deletions(-) 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 <https://www.gnu.org/licenses/>. */ -/* 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 */ diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c index 81a170a819..76a6476607 100644 --- a/sysdeps/x86/cpu-features.c +++ b/sysdeps/x86/cpu-features.c @@ -594,10 +594,9 @@ no_cpuid: } /* 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 (get_cet_control_value (CET_IBT_SHIFT) != CET_PERMISSIVE + && get_cet_control_value (CET_SHSTK_SHIFT) != CET_PERMISSIVE) dl_cet_lock_cet (); } # endif diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c index 861bd7bcaa..c8fc5e67d9 100644 --- a/sysdeps/x86/cpu-tunables.c +++ b/sysdeps/x86/cpu-tunables.c @@ -338,26 +338,26 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp) # if CET_ENABLED # include <cet-tunables.h> +/* Set CET control value. */ + +static inline void +set_cet_control_value (unsigned int value, unsigned int shift) +{ + GL(dl_x86_feature_1)[1] &= ~(CET_CONTROL_MASK << shift); + GL(dl_x86_feature_1)[1] |= value << shift; +} + 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; - } + set_cet_control_value (CET_ALWAYS_ON, CET_IBT_SHIFT); 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; - } + set_cet_control_value (CET_ALWAYS_OFF, CET_IBT_SHIFT); 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; - } + set_cet_control_value (CET_PERMISSIVE, CET_IBT_SHIFT); } attribute_hidden @@ -365,21 +365,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); - } + set_cet_control_value (CET_ALWAYS_ON, CET_SHSTK_SHIFT); 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); - } + set_cet_control_value (CET_ALWAYS_OFF, CET_SHSTK_SHIFT); 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); - } + set_cet_control_value (CET_PERMISSIVE, CET_SHSTK_SHIFT); } # endif #endif diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c index c7029f1b51..0f115540aa 100644 --- a/sysdeps/x86/dl-cet.c +++ b/sysdeps/x86/dl-cet.c @@ -39,11 +39,9 @@ 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); + unsigned int enable_ibt_type = get_cet_control_value (CET_IBT_SHIFT); /* Check how SHSTK should be enabled. */ - unsigned int enable_shstk_type - = ((GL(dl_x86_feature_1)[1] >> CET_MAX) & ((1 << CET_MAX) - 1)); + unsigned int enable_shstk_type = get_cet_control_value (CET_SHSTK_SHIFT); /* No legacy object check if both IBT and SHSTK are always on. */ if (enable_ibt_type == CET_ALWAYS_ON -- 2.25.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* PING: [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu @ 2020-05-16 16:37 ` H.J. Lu 2020-05-16 17:27 ` Florian Weimer 1 sibling, 0 replies; 21+ messages in thread From: H.J. Lu @ 2020-05-16 16:37 UTC (permalink / raw) To: GNU C Library, Carlos O'Donell; +Cc: Florian Weimer On Tue, Apr 28, 2020 at 2:52 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > _dl_x86_feature_1[1] is used to control 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. > */ > #define CET_ELF_PROPERTY 0 > #define CET_ALWAYS_OFF 1 > #define CET_ALWAYS_ON 2 > #define CET_PERMISSIVE 3 > #define CET_MAX CET_PERMISSIVE > > CET control value takes 2 bits. Rename CET_MAX to CET_CONTROL_MASK. Add > CET_IBT_SHIFT and CET_SHSTK_SHIFT. > --- > sysdeps/x86/cet-tunables.h | 22 +++++++++++++++++++-- > sysdeps/x86/cpu-features.c | 7 +++---- > sysdeps/x86/cpu-tunables.c | 39 +++++++++++++++----------------------- > sysdeps/x86/dl-cet.c | 6 ++---- > 4 files changed, 40 insertions(+), 34 deletions(-) > > 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 > <https://www.gnu.org/licenses/>. */ > > -/* 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 */ > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c > index 81a170a819..76a6476607 100644 > --- a/sysdeps/x86/cpu-features.c > +++ b/sysdeps/x86/cpu-features.c > @@ -594,10 +594,9 @@ no_cpuid: > } > > /* 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 (get_cet_control_value (CET_IBT_SHIFT) != CET_PERMISSIVE > + && get_cet_control_value (CET_SHSTK_SHIFT) != CET_PERMISSIVE) > dl_cet_lock_cet (); > } > # endif > diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c > index 861bd7bcaa..c8fc5e67d9 100644 > --- a/sysdeps/x86/cpu-tunables.c > +++ b/sysdeps/x86/cpu-tunables.c > @@ -338,26 +338,26 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp) > # if CET_ENABLED > # include <cet-tunables.h> > > +/* Set CET control value. */ > + > +static inline void > +set_cet_control_value (unsigned int value, unsigned int shift) > +{ > + GL(dl_x86_feature_1)[1] &= ~(CET_CONTROL_MASK << shift); > + GL(dl_x86_feature_1)[1] |= value << shift; > +} > + > 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; > - } > + set_cet_control_value (CET_ALWAYS_ON, CET_IBT_SHIFT); > 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; > - } > + set_cet_control_value (CET_ALWAYS_OFF, CET_IBT_SHIFT); > 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; > - } > + set_cet_control_value (CET_PERMISSIVE, CET_IBT_SHIFT); > } > > attribute_hidden > @@ -365,21 +365,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); > - } > + set_cet_control_value (CET_ALWAYS_ON, CET_SHSTK_SHIFT); > 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); > - } > + set_cet_control_value (CET_ALWAYS_OFF, CET_SHSTK_SHIFT); > 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); > - } > + set_cet_control_value (CET_PERMISSIVE, CET_SHSTK_SHIFT); > } > # endif > #endif > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c > index c7029f1b51..0f115540aa 100644 > --- a/sysdeps/x86/dl-cet.c > +++ b/sysdeps/x86/dl-cet.c > @@ -39,11 +39,9 @@ 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); > + unsigned int enable_ibt_type = get_cet_control_value (CET_IBT_SHIFT); > /* Check how SHSTK should be enabled. */ > - unsigned int enable_shstk_type > - = ((GL(dl_x86_feature_1)[1] >> CET_MAX) & ((1 << CET_MAX) - 1)); > + unsigned int enable_shstk_type = get_cet_control_value (CET_SHSTK_SHIFT); > > /* No legacy object check if both IBT and SHSTK are always on. */ > if (enable_ibt_type == CET_ALWAYS_ON > -- > 2.25.4 > PING. -- H.J. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu 2020-05-16 16:37 ` PING: " H.J. Lu @ 2020-05-16 17:27 ` Florian Weimer 2020-05-16 23:44 ` [PATCH] x86: Move CET control to _dl_x86_feature_control " H.J. Lu 1 sibling, 1 reply; 21+ messages in thread From: Florian Weimer @ 2020-05-16 17:27 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * 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 > <https://www.gnu.org/licenses/>. */ > > -/* 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? ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 2020-05-16 17:27 ` Florian Weimer @ 2020-05-16 23:44 ` H.J. Lu 2020-05-18 7:19 ` Florian Weimer 0 siblings, 1 reply; 21+ messages in thread From: H.J. Lu @ 2020-05-16 23:44 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 1620 bytes --] On Sat, May 16, 2020 at 10:27 AM Florian Weimer <fw@deneb.enyo.de> 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 > > <https://www.gnu.org/licenses/>. */ > > > > -/* 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? -- H.J. [-- Attachment #2: 0001-x86-Move-CET-control-to-_dl_x86_feature_control-BZ-2.patch --] [-- Type: text/x-patch, Size: 12927 bytes --] From cd9ccac3dbcb286de61f4123ead91bf893321c13 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> 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 <https://www.gnu.org/licenses/>. */ -/* 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 <dl-cet.h> -# include <cet-tunables.h> #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 <cet-tunables.h> 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 <libintl.h> #include <ldsodefs.h> #include <dl-cet.h> -#include <cet-tunables.h> /* GNU_PROPERTY_X86_FEATURE_1_IBT and GNU_PROPERTY_X86_FEATURE_1_SHSTK are defined in <elf.h>, 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 <cet-control.h> #include_next <ldsodefs.h> #endif -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 2020-05-16 23:44 ` [PATCH] x86: Move CET control to _dl_x86_feature_control " H.J. Lu @ 2020-05-18 7:19 ` Florian Weimer 2020-05-18 12:26 ` V2 " H.J. Lu 0 siblings, 1 reply; 21+ messages in thread From: Florian Weimer @ 2020-05-18 7:19 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: > Here is the updated patch to use bitfields. Thanks, I like this better. > 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 > <https://www.gnu.org/licenses/>. */ > > -/* 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 */ Missing ”.” at end of comment. > +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. */ Missing double space after “.”. > + permissive > +}; Given enum constantse are not not scoped and this is a widely-included header, I think you should include at least a “cet_” prefix in these enum constants. It's still not clear to me why CET control variables have to be in _rtld_global. Regular global variables would likely lead to clearer code, I think. _rtld_global is needed for read-write data that is shared with libc/libdl, and this does not seem to apply to CET control settings. Thanks, Florian ^ permalink raw reply [flat|nested] 21+ messages in thread
* V2 [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 2020-05-18 7:19 ` Florian Weimer @ 2020-05-18 12:26 ` H.J. Lu 2020-05-18 12:36 ` Florian Weimer 0 siblings, 1 reply; 21+ messages in thread From: H.J. Lu @ 2020-05-18 12:26 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 2708 bytes --] On Mon, May 18, 2020 at 12:19 AM Florian Weimer <fweimer@redhat.com> wrote: > > * H. J. Lu via Libc-alpha: > > > Here is the updated patch to use bitfields. > > Thanks, I like this better. > > > 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 > > <https://www.gnu.org/licenses/>. */ > > > > -/* 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 */ > > Missing ”.” at end of comment. Fixed. > > +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. */ > > Missing double space after “.”. Fixed. > > + permissive > > +}; > > Given enum constantse are not not scoped and this is a widely-included > header, I think you should include at least a “cet_” prefix in these > enum constants. Fixed. > It's still not clear to me why CET control variables have to be in > _rtld_global. Regular global variables would likely lead to clearer > code, I think. _rtld_global is needed for read-write data that is > shared with libc/libdl, and this does not seem to apply to CET control > settings. We don't have a pure local struct for rtld. _rtld_local is a hidden alias of _rtld_global: extern struct rtld_global _rtld_local __attribute__ ((alias ("_rtld_global"), visibility ("hidden"))); struct rtld_global is used to improve IP-relative access. It may not be important for x86-64. But it is still useful for i386 which doesn't have IP-relative addressing. Here is the updated patch. I combined Include <dl-procruntime.c> to get architecture specific initializer in rtld_global. OK for master? Thanks. -- H.J. [-- Attachment #2: 0001-x86-Move-CET-control-to-_dl_x86_feature_control-BZ-2.patch --] [-- Type: text/x-patch, Size: 13717 bytes --] From cf798745862c0461194f9676a2d1a5ce70dbc7a3 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Tue, 28 Apr 2020 10:05:25 -0700 Subject: [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 1. Include <dl-procruntime.c> to get architecture specific initializer in rtld_global. 2. Change _dl_x86_feature_1[2] to _dl_x86_feature_1. 3. 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]. --- elf/rtld.c | 2 + 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 | 43 +++++++++---------- sysdeps/x86/dl-procruntime.c | 22 +++++++++- sysdeps/x86/ldsodefs.h | 1 + 9 files changed, 81 insertions(+), 68 deletions(-) rename sysdeps/x86/{cet-tunables.h => cet-control.h} (60%) diff --git a/elf/rtld.c b/elf/rtld.c index 5ccc3c2dbb..882b070cc0 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) (except those which cannot be added for some reason). */ struct rtld_global _rtld_global = { + /* Get architecture specific initializer. */ +#include <dl-procruntime.c> /* Generally the default presumption without further information is an * executable stack but this is not true for all platforms. */ ._dl_stack_flags = DEFAULT_STACK_PERMS, 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 60% rename from sysdeps/x86/cet-tunables.h rename to sysdeps/x86/cet-control.h index 5e1e42df10..e163a7f55b 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 <https://www.gnu.org/licenses/>. */ -/* 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. */ + cet_elf_property = 0, + /* Always enable CET features. */ + cet_always_on, + /* Always disable CET features. */ + cet_always_off, + /* Enable CET features permissively. */ + cet_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..b6d5d9d4b1 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 <dl-cet.h> -# include <cet-tunables.h> #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 != cet_permissive + && GL(dl_x86_feature_control).shstk != cet_permissive) dl_cet_lock_cet (); } # endif diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c index 861bd7bcaa..38ad2c2e84 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 <cet-tunables.h> 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 = cet_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 = cet_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 = cet_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 = cet_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 = cet_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 = cet_permissive; } # endif #endif diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c index c7029f1b51..5524b66038 100644 --- a/sysdeps/x86/dl-cet.c +++ b/sysdeps/x86/dl-cet.c @@ -20,7 +20,6 @@ #include <libintl.h> #include <ldsodefs.h> #include <dl-cet.h> -#include <cet-tunables.h> /* GNU_PROPERTY_X86_FEATURE_1_IBT and GNU_PROPERTY_X86_FEATURE_1_SHSTK are defined in <elf.h>, which are only available for C sources. @@ -39,23 +38,23 @@ 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 == cet_always_on + && enable_shstk_type == cet_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 +64,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 != cet_always_off); bool enable_shstk = (shstk_enabled - && enable_shstk_type != CET_ALWAYS_OFF); + && enable_shstk_type != cet_always_off); if (program) { /* Enable IBT and SHSTK only if they are enabled in executable. @@ -76,10 +75,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 == cet_always_on || (m->l_cet & lc_ibt) != 0)); enable_shstk &= (HAS_CPU_FEATURE (SHSTK) - && (enable_shstk_type == CET_ALWAYS_ON + && (enable_shstk_type == cet_always_on || (m->l_cet & lc_shstk) != 0)); } @@ -111,7 +110,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 == cet_always_on || (l->l_cet & lc_ibt) != 0); if (!found_ibt_legacy && enable_ibt != ibt_enabled) { @@ -121,7 +120,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 == cet_always_on || (l->l_cet & lc_shstk) != 0); if (enable_shstk != shstk_enabled) { @@ -137,7 +136,7 @@ dl_cet_check (struct link_map *m, const char *program) { if (!program) { - if (enable_ibt_type != CET_PERMISSIVE) + if (enable_ibt_type != cet_permissive) { /* When IBT is enabled, we cannot dlopen a shared object without IBT. */ @@ -148,7 +147,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 != cet_permissive) { /* When SHSTK is enabled, we cannot dlopen a shared object without SHSTK. */ @@ -159,8 +158,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 != cet_permissive + && enable_shstk_type != cet_permissive) return; } @@ -190,7 +189,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 +198,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 != cet_permissive) && (!shstk_enabled - || enable_shstk_type != CET_PERMISSIVE)) + || enable_shstk_type != cet_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 5e39a38133..2c3e97952b 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -47,7 +47,27 @@ # 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 += { + .ibt = cet_elf_property, + .shstk = cet_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 <cet-control.h> #include_next <ldsodefs.h> #endif -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: V2 [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887] 2020-05-18 12:26 ` V2 " H.J. Lu @ 2020-05-18 12:36 ` Florian Weimer 0 siblings, 0 replies; 21+ messages in thread From: Florian Weimer @ 2020-05-18 12:36 UTC (permalink / raw) To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha * H. J. Lu: >> It's still not clear to me why CET control variables have to be in >> _rtld_global. Regular global variables would likely lead to clearer >> code, I think. _rtld_global is needed for read-write data that is >> shared with libc/libdl, and this does not seem to apply to CET control >> settings. > > We don't have a pure local struct for rtld. Yes, but we could use global variables. > _rtld_local is a hidden alias of _rtld_global: > > extern struct rtld_global _rtld_local > __attribute__ ((alias ("_rtld_global"), visibility ("hidden"))); > > struct rtld_global is used to improve IP-relative access. Are you sure? I think it's mainly for sharing data with libc and libdl through a single symbol. Data that's not shared should go into a regular global variable, I think. > It may not be important for x86-64. But it is still useful for i386 > which doesn't have IP-relative addressing. I don't think we use an alternative way to access _rtld_global or _rtld_global_ro on any target. Access always happens through a (hidden) variable. The patch looks okay to me now. Thanks! Florian ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global 2020-04-28 21:52 [PATCH 0/3] x86: Add --enable-cet=permissive H.J. Lu 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu @ 2020-04-28 21:52 ` H.J. Lu 2020-05-16 16:38 ` PING: " H.J. Lu 2020-05-16 17:51 ` Florian Weimer 2020-04-28 21:52 ` [PATCH 3/3] x86: Add --enable-cet=permissive H.J. Lu 2020-04-29 17:19 ` [PATCH 0/3] " Adhemerval Zanella 3 siblings, 2 replies; 21+ messages in thread From: H.J. Lu @ 2020-04-28 21:52 UTC (permalink / raw) To: libc-alpha Include <dl-procruntime.c> to get architecture specific initializer in rtld_global. --- elf/rtld.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/elf/rtld.c b/elf/rtld.c index 0016db86a7..44b9fb0b3c 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) (except those which cannot be added for some reason). */ struct rtld_global _rtld_global = { + /* Get architecture specific initializer. */ +#include <dl-procruntime.c> /* Generally the default presumption without further information is an * executable stack but this is not true for all platforms. */ ._dl_stack_flags = DEFAULT_STACK_PERMS, -- 2.25.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* PING: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global 2020-04-28 21:52 ` [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global H.J. Lu @ 2020-05-16 16:38 ` H.J. Lu 2020-05-16 17:51 ` Florian Weimer 1 sibling, 0 replies; 21+ messages in thread From: H.J. Lu @ 2020-05-16 16:38 UTC (permalink / raw) To: GNU C Library, Carlos O'Donell; +Cc: Florian Weimer On Tue, Apr 28, 2020 at 2:52 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > Include <dl-procruntime.c> to get architecture specific initializer in > rtld_global. > --- > elf/rtld.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/elf/rtld.c b/elf/rtld.c > index 0016db86a7..44b9fb0b3c 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) > (except those which cannot be added for some reason). */ > struct rtld_global _rtld_global = > { > + /* Get architecture specific initializer. */ > +#include <dl-procruntime.c> > /* Generally the default presumption without further information is an > * executable stack but this is not true for all platforms. */ > ._dl_stack_flags = DEFAULT_STACK_PERMS, > -- > 2.25.4 > PING. -- H.J. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global 2020-04-28 21:52 ` [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global H.J. Lu 2020-05-16 16:38 ` PING: " H.J. Lu @ 2020-05-16 17:51 ` Florian Weimer 2020-05-16 18:01 ` H.J. Lu 1 sibling, 1 reply; 21+ messages in thread From: Florian Weimer @ 2020-05-16 17:51 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: > Include <dl-procruntime.c> to get architecture specific initializer in > rtld_global. > --- > elf/rtld.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/elf/rtld.c b/elf/rtld.c > index 0016db86a7..44b9fb0b3c 100644 > --- a/elf/rtld.c > +++ b/elf/rtld.c > @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) > (except those which cannot be added for some reason). */ > struct rtld_global _rtld_global = > { > + /* Get architecture specific initializer. */ > +#include <dl-procruntime.c> > /* Generally the default presumption without further information is an > * executable stack but this is not true for all platforms. */ > ._dl_stack_flags = DEFAULT_STACK_PERMS, This patch does not build on its own. I'm surprised that it works in combination with the third patch. Is _rtld_global really the right place for this data? Is it even needed from outside the dynamic loader? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global 2020-05-16 17:51 ` Florian Weimer @ 2020-05-16 18:01 ` H.J. Lu 2020-05-16 18:07 ` Florian Weimer 0 siblings, 1 reply; 21+ messages in thread From: H.J. Lu @ 2020-05-16 18:01 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha On Sat, May 16, 2020 at 10:52 AM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu via Libc-alpha: > > > Include <dl-procruntime.c> to get architecture specific initializer in > > rtld_global. > > --- > > elf/rtld.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/elf/rtld.c b/elf/rtld.c > > index 0016db86a7..44b9fb0b3c 100644 > > --- a/elf/rtld.c > > +++ b/elf/rtld.c > > @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) > > (except those which cannot be added for some reason). */ > > struct rtld_global _rtld_global = > > { > > + /* Get architecture specific initializer. */ > > +#include <dl-procruntime.c> > > /* Generally the default presumption without further information is an > > * executable stack but this is not true for all platforms. */ > > ._dl_stack_flags = DEFAULT_STACK_PERMS, > > This patch does not build on its own. I'm surprised that it works in > combination with the third patch. It should build. > Is _rtld_global really the right place for this data? Is it even > needed from outside the dynamic loader? Yes, it matches: struct rtld_global { ... #include <dl-procruntime.c> ... }; in sysdeps/generic/ldsodefs.h. -- H.J. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global 2020-05-16 18:01 ` H.J. Lu @ 2020-05-16 18:07 ` Florian Weimer 2020-05-16 18:24 ` V2 [PATCH] " H.J. Lu 0 siblings, 1 reply; 21+ messages in thread From: Florian Weimer @ 2020-05-16 18:07 UTC (permalink / raw) To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha * H. J. Lu: > On Sat, May 16, 2020 at 10:52 AM Florian Weimer <fw@deneb.enyo.de> wrote: >> >> * H. J. Lu via Libc-alpha: >> >> > Include <dl-procruntime.c> to get architecture specific initializer in >> > rtld_global. >> > --- >> > elf/rtld.c | 2 ++ >> > 1 file changed, 2 insertions(+) >> > >> > diff --git a/elf/rtld.c b/elf/rtld.c >> > index 0016db86a7..44b9fb0b3c 100644 >> > --- a/elf/rtld.c >> > +++ b/elf/rtld.c >> > @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) >> > (except those which cannot be added for some reason). */ >> > struct rtld_global _rtld_global = >> > { >> > + /* Get architecture specific initializer. */ >> > +#include <dl-procruntime.c> >> > /* Generally the default presumption without further information is an >> > * executable stack but this is not true for all platforms. */ >> > ._dl_stack_flags = DEFAULT_STACK_PERMS, >> >> This patch does not build on its own. I'm surprised that it works in >> combination with the third patch. > > It should build. It doesn't if I do not apply the third patch. >> Is _rtld_global really the right place for this data? Is it even >> needed from outside the dynamic loader? > > Yes, it matches: > > struct rtld_global > { > ... > #include <dl-procruntime.c> > ... > }; > > in sysdeps/generic/ldsodefs.h. I mean conceptually—it's not clear to me why these fields have to be in _rtld_global. It makes things harder to maintain. Sorry, this is a more general question, it's not about the patch itself. ^ permalink raw reply [flat|nested] 21+ messages in thread
* V2 [PATCH] rtld: Get architecture specific initializer in rtld_global 2020-05-16 18:07 ` Florian Weimer @ 2020-05-16 18:24 ` H.J. Lu 0 siblings, 0 replies; 21+ messages in thread From: H.J. Lu @ 2020-05-16 18:24 UTC (permalink / raw) To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 1907 bytes --] On Sat, May 16, 2020 at 11:07 AM Florian Weimer <fw@deneb.enyo.de> wrote: > > * H. J. Lu: > > > On Sat, May 16, 2020 at 10:52 AM Florian Weimer <fw@deneb.enyo.de> wrote: > >> > >> * H. J. Lu via Libc-alpha: > >> > >> > Include <dl-procruntime.c> to get architecture specific initializer in > >> > rtld_global. > >> > --- > >> > elf/rtld.c | 2 ++ > >> > 1 file changed, 2 insertions(+) > >> > > >> > diff --git a/elf/rtld.c b/elf/rtld.c > >> > index 0016db86a7..44b9fb0b3c 100644 > >> > --- a/elf/rtld.c > >> > +++ b/elf/rtld.c > >> > @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) > >> > (except those which cannot be added for some reason). */ > >> > struct rtld_global _rtld_global = > >> > { > >> > + /* Get architecture specific initializer. */ > >> > +#include <dl-procruntime.c> > >> > /* Generally the default presumption without further information is an > >> > * executable stack but this is not true for all platforms. */ > >> > ._dl_stack_flags = DEFAULT_STACK_PERMS, > >> > >> This patch does not build on its own. I'm surprised that it works in > >> combination with the third patch. > > > > It should build. > > It doesn't if I do not apply the third patch. Fixed. > >> Is _rtld_global really the right place for this data? Is it even > >> needed from outside the dynamic loader? > > > > Yes, it matches: > > > > struct rtld_global > > { > > ... > > #include <dl-procruntime.c> > > ... > > }; > > > > in sysdeps/generic/ldsodefs.h. > > I mean conceptually—it's not clear to me why these fields have to be > in _rtld_global. It makes things harder to maintain. Sorry, this is > a more general question, it's not about the patch itself. What is the alternative? These fields are initialized very early and don't change after it. Here is the updated patch. OK for master? -- H.J. [-- Attachment #2: 0001-rtld-Get-architecture-specific-initializer-in-rtld_g.patch --] [-- Type: text/x-patch, Size: 1343 bytes --] From 83015ce9f4e56faa772bfab4f9717365af353ced Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Tue, 28 Apr 2020 10:05:25 -0700 Subject: [PATCH] rtld: Get architecture specific initializer in rtld_global Include <dl-procruntime.c> to get architecture specific initializer in rtld_global. --- elf/rtld.c | 2 ++ sysdeps/x86/dl-procruntime.c | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/elf/rtld.c b/elf/rtld.c index 5ccc3c2dbb..882b070cc0 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -315,6 +315,8 @@ rtld_hidden_def (_dl_starting_up) (except those which cannot be added for some reason). */ struct rtld_global _rtld_global = { + /* Get architecture specific initializer. */ +#include <dl-procruntime.c> /* Generally the default presumption without further information is an * executable stack but this is not true for all platforms. */ ._dl_stack_flags = DEFAULT_STACK_PERMS, diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c index 5e39a38133..88a7cac646 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -49,6 +49,11 @@ # else PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] # endif +# ifndef PROCINFO_DECL += { + 0, + } +# endif # if !defined SHARED || defined PROCINFO_DECL ; # else -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/3] x86: Add --enable-cet=permissive 2020-04-28 21:52 [PATCH 0/3] x86: Add --enable-cet=permissive H.J. Lu 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu 2020-04-28 21:52 ` [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global H.J. Lu @ 2020-04-28 21:52 ` H.J. Lu 2020-04-29 17:19 ` [PATCH 0/3] " Adhemerval Zanella 3 siblings, 0 replies; 21+ messages in thread From: H.J. Lu @ 2020-04-28 21:52 UTC (permalink / raw) To: libc-alpha When CET is enabled, it is an error to dlopen a non CET enabled shared library in CET enabled application. It may be desirable to make CET permissive, that is disable CET when dlopening a non CET enabled shared library. With the new --enable-cet=permissive configure option, CET is disabled when dlopening a non CET enabled shared library. Add DEFAULT_DL_X86_FEATURE_1_1 to config.h.in: /* The default value of _dl_x86_feature_1[1]. */ #define DEFAULT_DL_X86_FEATURE_1_1 0 --enable-cet changes it to /* The default value of _dl_x86_feature_1[1]. */ #define DEFAULT_DL_X86_FEATURE_1_1 ((CET_ELF_PROPERTY << CET_IBT_SHIFT) | (CET_ELF_PROPERTY << CET_SHSTK_SHIFT)) enables CET features based on ELF property note. --enable-cet=permissive it to /* The default value of _dl_x86_feature_1[1]. */ #define DEFAULT_DL_X86_FEATURE_1_1 ((CET_PERMISSIVE << CET_IBT_SHIFT) | (CET_PERMISSIVE << CET_SHSTK_SHIFT)) which enable CET features permissively. Update tst-cet-legacy-5a, tst-cet-legacy-5b, tst-cet-legacy-6a and tst-cet-legacy-6b to check --enable-cet and --enable-cet=permissive. --- INSTALL | 26 +++++++++++++++----------- config.h.in | 3 +++ manual/install.texi | 12 ++++++++---- sysdeps/unix/sysv/linux/x86/Makefile | 2 +- sysdeps/x86/Makefile | 18 +++++++++++++----- sysdeps/x86/configure | 21 +++++++++++---------- sysdeps/x86/configure.ac | 19 +++++++++---------- sysdeps/x86/dl-procruntime.c | 5 +++++ sysdeps/x86/tst-cet-legacy-5.c | 25 +++++++++++++++++-------- sysdeps/x86/tst-cet-legacy-6.c | 25 +++++++++++++++++-------- 10 files changed, 99 insertions(+), 57 deletions(-) diff --git a/INSTALL b/INSTALL index 242cb06f91..397537d189 100644 --- a/INSTALL +++ b/INSTALL @@ -123,20 +123,24 @@ if 'CFLAGS' is specified it must enable optimization. For example: executables (PIE) by default. '--enable-cet' +'--enable-cet=permissive' Enable Intel Control-flow Enforcement Technology (CET) support. - When the GNU C Library is built with '--enable-cet', the resulting - library is protected with indirect branch tracking (IBT) and shadow - stack (SHSTK). When CET is enabled, the GNU C Library is - compatible with all existing executables and shared libraries. - This feature is currently supported on i386, x86_64 and x32 with - GCC 8 and binutils 2.29 or later. Note that when CET is enabled, - the GNU C Library requires CPUs capable of multi-byte NOPs, like - x86-64 processors as well as Intel Pentium Pro or newer. + When the GNU C Library is built with '--enable-cet' or + '--enable-cet=permissive', the resulting library is protected with + indirect branch tracking (IBT) and shadow stack (SHSTK). When CET + is enabled, the GNU C Library is compatible with all existing + executables and shared libraries. This feature is currently + supported on i386, x86_64 and x32 with GCC 8 and binutils 2.29 or + later. Note that when CET is enabled, the GNU C Library requires + CPUs capable of multi-byte NOPs, like x86-64 processors as well as + Intel Pentium Pro or newer. With '--enable-cet', it is an error to + dlopen a non CET enabled shared library in CET enabled application. + With '--enable-cet=permissive', CET is disabled when dlopening a + non CET enabled shared library in CET enabled application. NOTE: '--enable-cet' has been tested for i686, x86_64 and x32 on - non-CET processors. '--enable-cet' has been tested for x86_64 and - x32 on CET SDVs, but Intel CET support hasn't been validated for - i686. + non-CET processors. '--enable-cet' has been tested for i686, + x86_64 and x32 on CET processors. '--disable-profile' Don't build libraries with profiling information. You may want to diff --git a/config.h.in b/config.h.in index dea43df438..93cda8838c 100644 --- a/config.h.in +++ b/config.h.in @@ -262,4 +262,7 @@ in i386 6 argument syscall issue). */ #define CAN_USE_REGISTER_ASM_EBP 0 +/* The default value of _dl_x86_feature_1[1]. */ +#define DEFAULT_DL_X86_FEATURE_1_1 0 + #endif diff --git a/manual/install.texi b/manual/install.texi index 71bf47cac6..f1359526d7 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -152,20 +152,24 @@ PIE. This option also implies that glibc programs and tests are created as dynamic position independent executables (PIE) by default. @item --enable-cet +@itemx --enable-cet=permissive Enable Intel Control-flow Enforcement Technology (CET) support. When -@theglibc{} is built with @option{--enable-cet}, the resulting library +@theglibc{} is built with @option{--enable-cet} or +@option{--enable-cet=permissive}, the resulting library is protected with indirect branch tracking (IBT) and shadow stack (SHSTK)@. When CET is enabled, @theglibc{} is compatible with all existing executables and shared libraries. This feature is currently supported on i386, x86_64 and x32 with GCC 8 and binutils 2.29 or later. Note that when CET is enabled, @theglibc{} requires CPUs capable of multi-byte NOPs, like x86-64 processors as well as Intel Pentium Pro or -newer. +newer. With @option{--enable-cet}, it is an error to dlopen a non CET +enabled shared library in CET enabled application. With +@option{--enable-cet=permissive}, CET is disabled when dlopening a +non CET enabled shared library in CET enabled application. NOTE: @option{--enable-cet} has been tested for i686, x86_64 and x32 on non-CET processors. @option{--enable-cet} has been tested for -x86_64 and x32 on CET SDVs, but Intel CET support hasn't been validated -for i686. +i686, x86_64 and x32 on CET processors. @item --disable-profile Don't build libraries with profiling information. You may want to use diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile index b23b532590..50fd018fa3 100644 --- a/sysdeps/unix/sysv/linux/x86/Makefile +++ b/sysdeps/unix/sysv/linux/x86/Makefile @@ -24,7 +24,7 @@ ifeq ($(subdir),setjmp) tests += tst-saved_mask-1 endif -ifeq ($(enable-cet),yes) +ifneq ($(enable-cet),no) ifeq ($(subdir),elf) tests += tst-cet-property-1 tst-cet-property-2 diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 4ffa699e5f..beab426f67 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -14,7 +14,7 @@ gen-as-const-headers += jmp_buf-ssp.sym sysdep_routines += __longjmp_cancel endif -ifeq ($(enable-cet),yes) +ifneq ($(enable-cet),no) ifeq ($(subdir),elf) sysdep-dl-routines += dl-cet @@ -42,13 +42,21 @@ CFLAGS-tst-cet-legacy-4.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-4a.c += -fcf-protection CFLAGS-tst-cet-legacy-4b.c += -fcf-protection CFLAGS-tst-cet-legacy-mod-4.c += -fcf-protection=none -CFLAGS-tst-cet-legacy-5a.c += -fcf-protection -CFLAGS-tst-cet-legacy-5b.c += -fcf-protection +CFLAGS-tst-cet-legacy-5a.c += -fcf-protection -mshstk +ifeq ($(enable-cet),permissive) +CPPFLAGS-tst-cet-legacy-5a.c += -DCET_IS_PERMISSIVE=1 +endif +CFLAGS-tst-cet-legacy-5b.c += -fcf-protection -mshstk +CPPFLAGS-tst-cet-legacy-5b.c += -DCET_DISABLED_BY_ENV=1 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-6a.c += -fcf-protection -mshstk +ifeq ($(enable-cet),permissive) +CPPFLAGS-tst-cet-legacy-6a.c += -DCET_IS_PERMISSIVE=1 +endif +CFLAGS-tst-cet-legacy-6b.c += -fcf-protection -mshstk +CPPFLAGS-tst-cet-legacy-6b.c += -DCET_DISABLED_BY_ENV=1 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 diff --git a/sysdeps/x86/configure b/sysdeps/x86/configure index b1ff281249..ad4dd412b4 100644 --- a/sysdeps/x86/configure +++ b/sysdeps/x86/configure @@ -1,7 +1,7 @@ # This file is generated from configure.ac by Autoconf. DO NOT EDIT! # Local configure fragment for sysdeps/x86. -if test x"$enable_cet" = xyes; then +if test $enable_cet != no; then # Check if CET can be enabled. { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether CET can be enabled" >&5 $as_echo_n "checking whether CET can be enabled... " >&6; } @@ -27,17 +27,11 @@ EOF fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_x86_cet_available" >&5 $as_echo "$libc_cv_x86_cet_available" >&6; } - if test $libc_cv_x86_cet_available = yes; then - enable_cet=yes - else - if test x"$enable_cet" = xdefault; then - enable_cet=no - else - as_fn_error $? "$CC doesn't support CET" "$LINENO" 5 - fi + if test $libc_cv_x86_cet_available != yes; then + as_fn_error $? "$CC doesn't support CET" "$LINENO" 5 fi fi -if test $enable_cet = yes; then +if test $enable_cet != no; then # Check if assembler supports CET. { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $AS supports CET" >&5 $as_echo_n "checking whether $AS supports CET... " >&6; } @@ -65,5 +59,12 @@ $as_echo "$libc_cv_x86_cet_as" >&6; } as_fn_error $? "$AS doesn't support CET" "$LINENO" 5 fi fi +if test $enable_cet = yes; then + $as_echo "#define DEFAULT_DL_X86_FEATURE_1_1 ((CET_ELF_PROPERTY << CET_IBT_SHIFT) | (CET_ELF_PROPERTY << CET_SHSTK_SHIFT))" >>confdefs.h + +elif test $enable_cet = permissive; then + $as_echo "#define DEFAULT_DL_X86_FEATURE_1_1 ((CET_PERMISSIVE << CET_IBT_SHIFT) | (CET_PERMISSIVE << CET_SHSTK_SHIFT))" >>confdefs.h + +fi config_vars="$config_vars enable-cet = $enable_cet" diff --git a/sysdeps/x86/configure.ac b/sysdeps/x86/configure.ac index a909b073af..b3ab3c83d8 100644 --- a/sysdeps/x86/configure.ac +++ b/sysdeps/x86/configure.ac @@ -1,7 +1,7 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory. # Local configure fragment for sysdeps/x86. -if test x"$enable_cet" = xyes; then +if test $enable_cet != no; then # Check if CET can be enabled. AC_CACHE_CHECK(whether CET can be enabled, libc_cv_x86_cet_available, [dnl @@ -16,17 +16,11 @@ EOF libc_cv_x86_cet_available=no fi rm -rf conftest*]) - if test $libc_cv_x86_cet_available = yes; then - enable_cet=yes - else - if test x"$enable_cet" = xdefault; then - enable_cet=no - else - AC_MSG_ERROR([$CC doesn't support CET]) - fi + if test $libc_cv_x86_cet_available != yes; then + AC_MSG_ERROR([$CC doesn't support CET]) fi fi -if test $enable_cet = yes; then +if test $enable_cet != no; then # Check if assembler supports CET. AC_CACHE_CHECK(whether $AS supports CET, libc_cv_x86_cet_as, [dnl @@ -43,4 +37,9 @@ EOF AC_MSG_ERROR([$AS doesn't support CET]) fi fi +if test $enable_cet = yes; then + AC_DEFINE(DEFAULT_DL_X86_FEATURE_1_1, ((CET_ELF_PROPERTY << CET_IBT_SHIFT) | (CET_ELF_PROPERTY << CET_SHSTK_SHIFT))) +elif test $enable_cet = permissive; then + AC_DEFINE(DEFAULT_DL_X86_FEATURE_1_1, ((CET_PERMISSIVE << CET_IBT_SHIFT) | (CET_PERMISSIVE << CET_SHSTK_SHIFT))) +fi LIBC_CONFIG_VAR([enable-cet], [$enable_cet]) diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c index 5e39a38133..d5be40294e 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -49,6 +49,11 @@ # else PROCINFO_CLASS unsigned int _dl_x86_feature_1[2] # endif +# ifndef PROCINFO_DECL += { + 0, DEFAULT_DL_X86_FEATURE_1_1 + } +# endif # if !defined SHARED || defined PROCINFO_DECL ; # else diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c index e2e95b6749..007b30029b 100644 --- a/sysdeps/x86/tst-cet-legacy-5.c +++ b/sysdeps/x86/tst-cet-legacy-5.c @@ -22,6 +22,14 @@ #include <stdlib.h> #include <stdbool.h> #include <string.h> +#include <x86intrin.h> +#include <support/check.h> + +#if defined CET_IS_PERMISSIVE || defined CET_DISABLED_BY_ENV +# define CET_MAYBE_DISABLED 1 +#else +# define CET_MAYBE_DISABLED 0 +#endif static void do_test_1 (const char *modname, bool fail) @@ -32,24 +40,25 @@ do_test_1 (const char *modname, bool fail) h = dlopen (modname, RTLD_LAZY); if (h == NULL) { + const char *err = dlerror (); if (fail) { - const char *err = dlerror (); if (strstr (err, "rebuild shared object with SHSTK support enabled") == NULL) - { - printf ("incorrect dlopen '%s' error: %s\n", modname, - err); - exit (1); - } + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); return; } - printf ("cannot open '%s': %s\n", modname, dlerror ()); - exit (1); + FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err); } + /* NB: dlopen should never fail on non-CET platforms. If SHSTK is + disabled, assuming IBT is also disabled. */ + bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED; + if (fail && cet_enabled) + FAIL_EXIT1 ("dlopen should have failed\n"); + fp = dlsym (h, "test"); if (fp == NULL) { diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c index bdbbb9075f..fdf798ee20 100644 --- a/sysdeps/x86/tst-cet-legacy-6.c +++ b/sysdeps/x86/tst-cet-legacy-6.c @@ -22,6 +22,14 @@ #include <stdlib.h> #include <stdbool.h> #include <string.h> +#include <x86intrin.h> +#include <support/check.h> + +#if defined CET_IS_PERMISSIVE || defined CET_DISABLED_BY_ENV +# define CET_MAYBE_DISABLED 1 +#else +# define CET_MAYBE_DISABLED 0 +#endif static void do_test_1 (const char *modname, bool fail) @@ -32,24 +40,25 @@ do_test_1 (const char *modname, bool fail) h = dlopen (modname, RTLD_LAZY); if (h == NULL) { + const char *err = dlerror (); if (fail) { - const char *err = dlerror (); if (strstr (err, "rebuild shared object with SHSTK support enabled") == NULL) - { - printf ("incorrect dlopen '%s' error: %s\n", modname, - err); - exit (1); - } + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); return; } - printf ("cannot open '%s': %s\n", modname, dlerror ()); - exit (1); + FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err); } + /* NB: dlopen should never fail on non-CET platforms. If SHSTK is + disabled, assuming IBT is also disabled. */ + bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED; + if (fail && cet_enabled) + FAIL_EXIT1 ("dlopen should have failed\n"); + fp = dlsym (h, "test"); if (fp == NULL) { -- 2.25.4 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] x86: Add --enable-cet=permissive 2020-04-28 21:52 [PATCH 0/3] x86: Add --enable-cet=permissive H.J. Lu ` (2 preceding siblings ...) 2020-04-28 21:52 ` [PATCH 3/3] x86: Add --enable-cet=permissive H.J. Lu @ 2020-04-29 17:19 ` Adhemerval Zanella 2020-04-29 20:29 ` H.J. Lu 2020-04-29 20:46 ` Florian Weimer 3 siblings, 2 replies; 21+ messages in thread From: Adhemerval Zanella @ 2020-04-29 17:19 UTC (permalink / raw) To: libc-alpha On 28/04/2020 18:52, H.J. Lu via Libc-alpha wrote: > When CET is enabled, it is an error to dlopen a non CET enabled shared > library in CET enabled application. It may be desirable to make CET > permissive, that is disable CET when dlopening a non CET enabled shared > library. With the new --enable-cet=permissive configure option, CET is > disabled when dlopening a non CET enabled shared library. Does not CET already provide a tunable to make it permissive? If the idea is to enable as de-facto for a distro bootstrap, why not make it default then? > > To support --enable-cet=permissive, CET_MAX is renamed to > CET_CONTROL_MASK and <dl-procruntime.c> is included in rtld.c to get > architecture specific initializer in rtld_global. > > H.J. Lu (3): > CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] > rtld: Get architecture specific initializer in rtld_global > x86: Add --enable-cet=permissive > > INSTALL | 26 +++++++++++-------- > config.h.in | 3 +++ > elf/rtld.c | 2 ++ > manual/install.texi | 12 ++++++--- > sysdeps/unix/sysv/linux/x86/Makefile | 2 +- > sysdeps/x86/Makefile | 18 +++++++++---- > sysdeps/x86/cet-tunables.h | 22 ++++++++++++++-- > sysdeps/x86/configure | 21 ++++++++------- > sysdeps/x86/configure.ac | 19 +++++++------- > sysdeps/x86/cpu-features.c | 7 +++-- > sysdeps/x86/cpu-tunables.c | 39 +++++++++++----------------- > sysdeps/x86/dl-cet.c | 6 ++--- > sysdeps/x86/dl-procruntime.c | 5 ++++ > sysdeps/x86/tst-cet-legacy-5.c | 25 ++++++++++++------ > sysdeps/x86/tst-cet-legacy-6.c | 25 ++++++++++++------ > 15 files changed, 141 insertions(+), 91 deletions(-) > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] x86: Add --enable-cet=permissive 2020-04-29 17:19 ` [PATCH 0/3] " Adhemerval Zanella @ 2020-04-29 20:29 ` H.J. Lu 2020-04-29 20:46 ` Florian Weimer 1 sibling, 0 replies; 21+ messages in thread From: H.J. Lu @ 2020-04-29 20:29 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: GNU C Library On Wed, Apr 29, 2020 at 11:27 AM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 28/04/2020 18:52, H.J. Lu via Libc-alpha wrote: > > When CET is enabled, it is an error to dlopen a non CET enabled shared > > library in CET enabled application. It may be desirable to make CET > > permissive, that is disable CET when dlopening a non CET enabled shared > > library. With the new --enable-cet=permissive configure option, CET is > > disabled when dlopening a non CET enabled shared library. > > Does not CET already provide a tunable to make it permissive? If the idea Yes, it can be controlled by a tunable. > is to enable as de-facto for a distro bootstrap, why not make it default > then? A distro can choose CET control default to permissive or enforced at build time. > > > > To support --enable-cet=permissive, CET_MAX is renamed to > > CET_CONTROL_MASK and <dl-procruntime.c> is included in rtld.c to get > > architecture specific initializer in rtld_global. > > > > H.J. Lu (3): > > CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] > > rtld: Get architecture specific initializer in rtld_global > > x86: Add --enable-cet=permissive > > > > INSTALL | 26 +++++++++++-------- > > config.h.in | 3 +++ > > elf/rtld.c | 2 ++ > > manual/install.texi | 12 ++++++--- > > sysdeps/unix/sysv/linux/x86/Makefile | 2 +- > > sysdeps/x86/Makefile | 18 +++++++++---- > > sysdeps/x86/cet-tunables.h | 22 ++++++++++++++-- > > sysdeps/x86/configure | 21 ++++++++------- > > sysdeps/x86/configure.ac | 19 +++++++------- > > sysdeps/x86/cpu-features.c | 7 +++-- > > sysdeps/x86/cpu-tunables.c | 39 +++++++++++----------------- > > sysdeps/x86/dl-cet.c | 6 ++--- > > sysdeps/x86/dl-procruntime.c | 5 ++++ > > sysdeps/x86/tst-cet-legacy-5.c | 25 ++++++++++++------ > > sysdeps/x86/tst-cet-legacy-6.c | 25 ++++++++++++------ > > 15 files changed, 141 insertions(+), 91 deletions(-) > > -- H.J. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] x86: Add --enable-cet=permissive 2020-04-29 17:19 ` [PATCH 0/3] " Adhemerval Zanella 2020-04-29 20:29 ` H.J. Lu @ 2020-04-29 20:46 ` Florian Weimer 2020-04-29 21:14 ` Adhemerval Zanella 1 sibling, 1 reply; 21+ messages in thread From: Florian Weimer @ 2020-04-29 20:46 UTC (permalink / raw) To: Adhemerval Zanella via Libc-alpha * Adhemerval Zanella via Libc-alpha: > On 28/04/2020 18:52, H.J. Lu via Libc-alpha wrote: >> When CET is enabled, it is an error to dlopen a non CET enabled shared >> library in CET enabled application. It may be desirable to make CET >> permissive, that is disable CET when dlopening a non CET enabled shared >> library. With the new --enable-cet=permissive configure option, CET is >> disabled when dlopening a non CET enabled shared library. > > Does not CET already provide a tunable to make it permissive? If the idea > is to enable as de-facto for a distro bootstrap, why not make it default > then? We currently do not have a way to set a tunable for SUID binaries. This means that it would be necessary to disable CET at the kernel or hypervisor level if the system depends on pre-CET NSS or PAM modules for its operation (or something else which is ultimately dlopen-based). ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/3] x86: Add --enable-cet=permissive 2020-04-29 20:46 ` Florian Weimer @ 2020-04-29 21:14 ` Adhemerval Zanella 2020-05-18 13:50 ` V2 [PATCH] " H.J. Lu 0 siblings, 1 reply; 21+ messages in thread From: Adhemerval Zanella @ 2020-04-29 21:14 UTC (permalink / raw) To: Florian Weimer, Adhemerval Zanella via Libc-alpha On 29/04/2020 17:46, Florian Weimer wrote: > * Adhemerval Zanella via Libc-alpha: > >> On 28/04/2020 18:52, H.J. Lu via Libc-alpha wrote: >>> When CET is enabled, it is an error to dlopen a non CET enabled shared >>> library in CET enabled application. It may be desirable to make CET >>> permissive, that is disable CET when dlopening a non CET enabled shared >>> library. With the new --enable-cet=permissive configure option, CET is >>> disabled when dlopening a non CET enabled shared library. >> >> Does not CET already provide a tunable to make it permissive? If the idea >> is to enable as de-facto for a distro bootstrap, why not make it default >> then? > > We currently do not have a way to set a tunable for SUID binaries. > > This means that it would be necessary to disable CET at the kernel or > hypervisor level if the system depends on pre-CET NSS or PAM modules > for its operation (or something else which is ultimately > dlopen-based). Doesn't disable CET on some modules defeat the very security effort? If so, why should it be a build option on glibc? ^ permalink raw reply [flat|nested] 21+ messages in thread
* V2 [PATCH] x86: Add --enable-cet=permissive 2020-04-29 21:14 ` Adhemerval Zanella @ 2020-05-18 13:50 ` H.J. Lu 2020-05-18 14:43 ` Florian Weimer 0 siblings, 1 reply; 21+ messages in thread From: H.J. Lu @ 2020-05-18 13:50 UTC (permalink / raw) To: Adhemerval Zanella; +Cc: Florian Weimer, Adhemerval Zanella via Libc-alpha [-- Attachment #1: Type: text/plain, Size: 1501 bytes --] On Wed, Apr 29, 2020 at 2:58 PM Adhemerval Zanella via Libc-alpha <libc-alpha@sourceware.org> wrote: > > > > On 29/04/2020 17:46, Florian Weimer wrote: > > * Adhemerval Zanella via Libc-alpha: > > > >> On 28/04/2020 18:52, H.J. Lu via Libc-alpha wrote: > >>> When CET is enabled, it is an error to dlopen a non CET enabled shared > >>> library in CET enabled application. It may be desirable to make CET > >>> permissive, that is disable CET when dlopening a non CET enabled shared > >>> library. With the new --enable-cet=permissive configure option, CET is > >>> disabled when dlopening a non CET enabled shared library. > >> > >> Does not CET already provide a tunable to make it permissive? If the idea > >> is to enable as de-facto for a distro bootstrap, why not make it default > >> then? > > > > We currently do not have a way to set a tunable for SUID binaries. > > > > This means that it would be necessary to disable CET at the kernel or > > hypervisor level if the system depends on pre-CET NSS or PAM modules > > for its operation (or something else which is ultimately > > dlopen-based). > > Doesn't disable CET on some modules defeat the very security effort? If so, > why should it be a build option on glibc? I consider it as a transitional build option for OSVs. OSV should use it only as the last resort. I expect we will remove this option a few years from now when all major pieces of OS are CET enabled. Here is the updated patch against today's master. Thanks. -- H.J. [-- Attachment #2: 0001-x86-Add-enable-cet-permissive.patch --] [-- Type: text/x-patch, Size: 14460 bytes --] From 03f28ce66d85455a328681bbd25a33a20977e989 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" <hjl.tools@gmail.com> Date: Mon, 27 Apr 2020 15:44:07 -0700 Subject: [PATCH] x86: Add --enable-cet=permissive When CET is enabled, it is an error to dlopen a non CET enabled shared library in CET enabled application. It may be desirable to make CET permissive, that is disable CET when dlopening a non CET enabled shared library. With the new --enable-cet=permissive configure option, CET is disabled when dlopening a non CET enabled shared library. Add DEFAULT_DL_X86_CET_CONTROL to config.h.in: /* The default value of x86 CET control. */ #define DEFAULT_DL_X86_CET_CONTROL cet_elf_property which enable CET features based on ELF property note. --enable-cet=permissive it to /* The default value of x86 CET control. */ #define DEFAULT_DL_X86_CET_CONTROL cet_permissive which enable CET features permissively. Update tst-cet-legacy-5a, tst-cet-legacy-5b, tst-cet-legacy-6a and tst-cet-legacy-6b to check --enable-cet and --enable-cet=permissive. --- INSTALL | 26 +++++++++++++++----------- config.h.in | 3 +++ manual/install.texi | 12 ++++++++---- sysdeps/unix/sysv/linux/x86/Makefile | 2 +- sysdeps/x86/Makefile | 18 +++++++++++++----- sysdeps/x86/configure | 21 +++++++++++---------- sysdeps/x86/configure.ac | 19 +++++++++---------- sysdeps/x86/dl-procruntime.c | 4 ++-- sysdeps/x86/tst-cet-legacy-5.c | 25 +++++++++++++++++-------- sysdeps/x86/tst-cet-legacy-6.c | 25 +++++++++++++++++-------- 10 files changed, 96 insertions(+), 59 deletions(-) diff --git a/INSTALL b/INSTALL index b7676d1c9f..62e78725f5 100644 --- a/INSTALL +++ b/INSTALL @@ -123,20 +123,24 @@ if 'CFLAGS' is specified it must enable optimization. For example: executables (PIE) by default. '--enable-cet' +'--enable-cet=permissive' Enable Intel Control-flow Enforcement Technology (CET) support. - When the GNU C Library is built with '--enable-cet', the resulting - library is protected with indirect branch tracking (IBT) and shadow - stack (SHSTK). When CET is enabled, the GNU C Library is - compatible with all existing executables and shared libraries. - This feature is currently supported on i386, x86_64 and x32 with - GCC 8 and binutils 2.29 or later. Note that when CET is enabled, - the GNU C Library requires CPUs capable of multi-byte NOPs, like - x86-64 processors as well as Intel Pentium Pro or newer. + When the GNU C Library is built with '--enable-cet' or + '--enable-cet=permissive', the resulting library is protected with + indirect branch tracking (IBT) and shadow stack (SHSTK). When CET + is enabled, the GNU C Library is compatible with all existing + executables and shared libraries. This feature is currently + supported on i386, x86_64 and x32 with GCC 8 and binutils 2.29 or + later. Note that when CET is enabled, the GNU C Library requires + CPUs capable of multi-byte NOPs, like x86-64 processors as well as + Intel Pentium Pro or newer. With '--enable-cet', it is an error to + dlopen a non CET enabled shared library in CET enabled application. + With '--enable-cet=permissive', CET is disabled when dlopening a + non CET enabled shared library in CET enabled application. NOTE: '--enable-cet' has been tested for i686, x86_64 and x32 on - non-CET processors. '--enable-cet' has been tested for x86_64 and - x32 on CET SDVs, but Intel CET support hasn't been validated for - i686. + non-CET processors. '--enable-cet' has been tested for i686, + x86_64 and x32 on CET processors. '--disable-profile' Don't build libraries with profiling information. You may want to diff --git a/config.h.in b/config.h.in index dea43df438..a63b1cc939 100644 --- a/config.h.in +++ b/config.h.in @@ -262,4 +262,7 @@ in i386 6 argument syscall issue). */ #define CAN_USE_REGISTER_ASM_EBP 0 +/* The default value of x86 CET control. */ +#define DEFAULT_DL_X86_CET_CONTROL cet_elf_property + #endif diff --git a/manual/install.texi b/manual/install.texi index f6d9d92317..c1e49a94fe 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -152,20 +152,24 @@ PIE. This option also implies that glibc programs and tests are created as dynamic position independent executables (PIE) by default. @item --enable-cet +@itemx --enable-cet=permissive Enable Intel Control-flow Enforcement Technology (CET) support. When -@theglibc{} is built with @option{--enable-cet}, the resulting library +@theglibc{} is built with @option{--enable-cet} or +@option{--enable-cet=permissive}, the resulting library is protected with indirect branch tracking (IBT) and shadow stack (SHSTK)@. When CET is enabled, @theglibc{} is compatible with all existing executables and shared libraries. This feature is currently supported on i386, x86_64 and x32 with GCC 8 and binutils 2.29 or later. Note that when CET is enabled, @theglibc{} requires CPUs capable of multi-byte NOPs, like x86-64 processors as well as Intel Pentium Pro or -newer. +newer. With @option{--enable-cet}, it is an error to dlopen a non CET +enabled shared library in CET enabled application. With +@option{--enable-cet=permissive}, CET is disabled when dlopening a +non CET enabled shared library in CET enabled application. NOTE: @option{--enable-cet} has been tested for i686, x86_64 and x32 on non-CET processors. @option{--enable-cet} has been tested for -x86_64 and x32 on CET SDVs, but Intel CET support hasn't been validated -for i686. +i686, x86_64 and x32 on CET processors. @item --disable-profile Don't build libraries with profiling information. You may want to use diff --git a/sysdeps/unix/sysv/linux/x86/Makefile b/sysdeps/unix/sysv/linux/x86/Makefile index b23b532590..50fd018fa3 100644 --- a/sysdeps/unix/sysv/linux/x86/Makefile +++ b/sysdeps/unix/sysv/linux/x86/Makefile @@ -24,7 +24,7 @@ ifeq ($(subdir),setjmp) tests += tst-saved_mask-1 endif -ifeq ($(enable-cet),yes) +ifneq ($(enable-cet),no) ifeq ($(subdir),elf) tests += tst-cet-property-1 tst-cet-property-2 diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index 4ffa699e5f..beab426f67 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -14,7 +14,7 @@ gen-as-const-headers += jmp_buf-ssp.sym sysdep_routines += __longjmp_cancel endif -ifeq ($(enable-cet),yes) +ifneq ($(enable-cet),no) ifeq ($(subdir),elf) sysdep-dl-routines += dl-cet @@ -42,13 +42,21 @@ CFLAGS-tst-cet-legacy-4.c += -fcf-protection=branch CFLAGS-tst-cet-legacy-4a.c += -fcf-protection CFLAGS-tst-cet-legacy-4b.c += -fcf-protection CFLAGS-tst-cet-legacy-mod-4.c += -fcf-protection=none -CFLAGS-tst-cet-legacy-5a.c += -fcf-protection -CFLAGS-tst-cet-legacy-5b.c += -fcf-protection +CFLAGS-tst-cet-legacy-5a.c += -fcf-protection -mshstk +ifeq ($(enable-cet),permissive) +CPPFLAGS-tst-cet-legacy-5a.c += -DCET_IS_PERMISSIVE=1 +endif +CFLAGS-tst-cet-legacy-5b.c += -fcf-protection -mshstk +CPPFLAGS-tst-cet-legacy-5b.c += -DCET_DISABLED_BY_ENV=1 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-6a.c += -fcf-protection -mshstk +ifeq ($(enable-cet),permissive) +CPPFLAGS-tst-cet-legacy-6a.c += -DCET_IS_PERMISSIVE=1 +endif +CFLAGS-tst-cet-legacy-6b.c += -fcf-protection -mshstk +CPPFLAGS-tst-cet-legacy-6b.c += -DCET_DISABLED_BY_ENV=1 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 diff --git a/sysdeps/x86/configure b/sysdeps/x86/configure index b1ff281249..81cc4e80d6 100644 --- a/sysdeps/x86/configure +++ b/sysdeps/x86/configure @@ -1,7 +1,7 @@ # This file is generated from configure.ac by Autoconf. DO NOT EDIT! # Local configure fragment for sysdeps/x86. -if test x"$enable_cet" = xyes; then +if test $enable_cet != no; then # Check if CET can be enabled. { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether CET can be enabled" >&5 $as_echo_n "checking whether CET can be enabled... " >&6; } @@ -27,17 +27,11 @@ EOF fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_x86_cet_available" >&5 $as_echo "$libc_cv_x86_cet_available" >&6; } - if test $libc_cv_x86_cet_available = yes; then - enable_cet=yes - else - if test x"$enable_cet" = xdefault; then - enable_cet=no - else - as_fn_error $? "$CC doesn't support CET" "$LINENO" 5 - fi + if test $libc_cv_x86_cet_available != yes; then + as_fn_error $? "$CC doesn't support CET" "$LINENO" 5 fi fi -if test $enable_cet = yes; then +if test $enable_cet != no; then # Check if assembler supports CET. { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $AS supports CET" >&5 $as_echo_n "checking whether $AS supports CET... " >&6; } @@ -65,5 +59,12 @@ $as_echo "$libc_cv_x86_cet_as" >&6; } as_fn_error $? "$AS doesn't support CET" "$LINENO" 5 fi fi +if test $enable_cet = yes; then + $as_echo "#define DEFAULT_DL_X86_CET_CONTROL cet_elf_property" >>confdefs.h + +elif test $enable_cet = permissive; then + $as_echo "#define DEFAULT_DL_X86_CET_CONTROL cet_permissive" >>confdefs.h + +fi config_vars="$config_vars enable-cet = $enable_cet" diff --git a/sysdeps/x86/configure.ac b/sysdeps/x86/configure.ac index a909b073af..8f3e1191f6 100644 --- a/sysdeps/x86/configure.ac +++ b/sysdeps/x86/configure.ac @@ -1,7 +1,7 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory. # Local configure fragment for sysdeps/x86. -if test x"$enable_cet" = xyes; then +if test $enable_cet != no; then # Check if CET can be enabled. AC_CACHE_CHECK(whether CET can be enabled, libc_cv_x86_cet_available, [dnl @@ -16,17 +16,11 @@ EOF libc_cv_x86_cet_available=no fi rm -rf conftest*]) - if test $libc_cv_x86_cet_available = yes; then - enable_cet=yes - else - if test x"$enable_cet" = xdefault; then - enable_cet=no - else - AC_MSG_ERROR([$CC doesn't support CET]) - fi + if test $libc_cv_x86_cet_available != yes; then + AC_MSG_ERROR([$CC doesn't support CET]) fi fi -if test $enable_cet = yes; then +if test $enable_cet != no; then # Check if assembler supports CET. AC_CACHE_CHECK(whether $AS supports CET, libc_cv_x86_cet_as, [dnl @@ -43,4 +37,9 @@ EOF AC_MSG_ERROR([$AS doesn't support CET]) fi fi +if test $enable_cet = yes; then + AC_DEFINE(DEFAULT_DL_X86_CET_CONTROL, cet_elf_property) +elif test $enable_cet = permissive; then + AC_DEFINE(DEFAULT_DL_X86_CET_CONTROL, cet_permissive) +fi LIBC_CONFIG_VAR([enable-cet], [$enable_cet]) diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c index 2c3e97952b..a3ec61b91e 100644 --- a/sysdeps/x86/dl-procruntime.c +++ b/sysdeps/x86/dl-procruntime.c @@ -65,8 +65,8 @@ PROCINFO_CLASS struct dl_x86_feature_control _dl_x86_feature_control # endif # ifndef PROCINFO_DECL = { - .ibt = cet_elf_property, - .shstk = cet_elf_property + .ibt = DEFAULT_DL_X86_CET_CONTROL, + .shstk = DEFAULT_DL_X86_CET_CONTROL, } # endif # if !defined SHARED || defined PROCINFO_DECL diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c index e2e95b6749..007b30029b 100644 --- a/sysdeps/x86/tst-cet-legacy-5.c +++ b/sysdeps/x86/tst-cet-legacy-5.c @@ -22,6 +22,14 @@ #include <stdlib.h> #include <stdbool.h> #include <string.h> +#include <x86intrin.h> +#include <support/check.h> + +#if defined CET_IS_PERMISSIVE || defined CET_DISABLED_BY_ENV +# define CET_MAYBE_DISABLED 1 +#else +# define CET_MAYBE_DISABLED 0 +#endif static void do_test_1 (const char *modname, bool fail) @@ -32,24 +40,25 @@ do_test_1 (const char *modname, bool fail) h = dlopen (modname, RTLD_LAZY); if (h == NULL) { + const char *err = dlerror (); if (fail) { - const char *err = dlerror (); if (strstr (err, "rebuild shared object with SHSTK support enabled") == NULL) - { - printf ("incorrect dlopen '%s' error: %s\n", modname, - err); - exit (1); - } + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); return; } - printf ("cannot open '%s': %s\n", modname, dlerror ()); - exit (1); + FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err); } + /* NB: dlopen should never fail on non-CET platforms. If SHSTK is + disabled, assuming IBT is also disabled. */ + bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED; + if (fail && cet_enabled) + FAIL_EXIT1 ("dlopen should have failed\n"); + fp = dlsym (h, "test"); if (fp == NULL) { diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c index bdbbb9075f..fdf798ee20 100644 --- a/sysdeps/x86/tst-cet-legacy-6.c +++ b/sysdeps/x86/tst-cet-legacy-6.c @@ -22,6 +22,14 @@ #include <stdlib.h> #include <stdbool.h> #include <string.h> +#include <x86intrin.h> +#include <support/check.h> + +#if defined CET_IS_PERMISSIVE || defined CET_DISABLED_BY_ENV +# define CET_MAYBE_DISABLED 1 +#else +# define CET_MAYBE_DISABLED 0 +#endif static void do_test_1 (const char *modname, bool fail) @@ -32,24 +40,25 @@ do_test_1 (const char *modname, bool fail) h = dlopen (modname, RTLD_LAZY); if (h == NULL) { + const char *err = dlerror (); if (fail) { - const char *err = dlerror (); if (strstr (err, "rebuild shared object with SHSTK support enabled") == NULL) - { - printf ("incorrect dlopen '%s' error: %s\n", modname, - err); - exit (1); - } + FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err); return; } - printf ("cannot open '%s': %s\n", modname, dlerror ()); - exit (1); + FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err); } + /* NB: dlopen should never fail on non-CET platforms. If SHSTK is + disabled, assuming IBT is also disabled. */ + bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED; + if (fail && cet_enabled) + FAIL_EXIT1 ("dlopen should have failed\n"); + fp = dlsym (h, "test"); if (fp == NULL) { -- 2.26.2 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: V2 [PATCH] x86: Add --enable-cet=permissive 2020-05-18 13:50 ` V2 [PATCH] " H.J. Lu @ 2020-05-18 14:43 ` Florian Weimer 0 siblings, 0 replies; 21+ messages in thread From: Florian Weimer @ 2020-05-18 14:43 UTC (permalink / raw) To: H.J. Lu via Libc-alpha * H. J. Lu via Libc-alpha: > + /* NB: dlopen should never fail on non-CET platforms. If SHSTK is > + disabled, assuming IBT is also disabled. */ > + bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED; > + if (fail && cet_enabled) > + FAIL_EXIT1 ("dlopen should have failed\n"); I think the multiple negatives are a bit difficult to understand here, but I think the logic is correct. (It's more obvious when looking at the test as a whole.) The patch looks okay to me. Thanks, Florian ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-05-18 14:43 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-28 21:52 [PATCH 0/3] x86: Add --enable-cet=permissive H.J. Lu 2020-04-28 21:52 ` [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887] H.J. Lu 2020-05-16 16:37 ` PING: " H.J. Lu 2020-05-16 17:27 ` Florian Weimer 2020-05-16 23:44 ` [PATCH] x86: Move CET control to _dl_x86_feature_control " H.J. Lu 2020-05-18 7:19 ` Florian Weimer 2020-05-18 12:26 ` V2 " H.J. Lu 2020-05-18 12:36 ` Florian Weimer 2020-04-28 21:52 ` [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global H.J. Lu 2020-05-16 16:38 ` PING: " H.J. Lu 2020-05-16 17:51 ` Florian Weimer 2020-05-16 18:01 ` H.J. Lu 2020-05-16 18:07 ` Florian Weimer 2020-05-16 18:24 ` V2 [PATCH] " H.J. Lu 2020-04-28 21:52 ` [PATCH 3/3] x86: Add --enable-cet=permissive H.J. Lu 2020-04-29 17:19 ` [PATCH 0/3] " Adhemerval Zanella 2020-04-29 20:29 ` H.J. Lu 2020-04-29 20:46 ` Florian Weimer 2020-04-29 21:14 ` Adhemerval Zanella 2020-05-18 13:50 ` V2 [PATCH] " H.J. Lu 2020-05-18 14:43 ` Florian Weimer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).