[PATCH 0/3] x86: Add --enable-cet=permissive

[PATCH 0/3] x86: Add --enable-cet=permissive

[H.J. Lu]

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

[PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887]

[H.J. Lu]

_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

[PATCH 2/3] rtld: Get architecture specific initializer in rtld_global

[H.J. Lu]

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

[PATCH 3/3] x86: Add --enable-cet=permissive

[H.J. Lu]

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

Re: [PATCH 0/3] x86: Add --enable-cet=permissive

[Adhemerval Zanella]

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(-)
> 

Re: [PATCH 0/3] x86: Add --enable-cet=permissive

[H.J. Lu]

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.

Re: [PATCH 0/3] x86: Add --enable-cet=permissive

[Florian Weimer]

* 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).

Re: [PATCH 0/3] x86: Add --enable-cet=permissive

[Adhemerval Zanella]

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?

PING: [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887]

[H.J. Lu]

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.

PING: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global

[H.J. Lu]

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.

Re: [PATCH 1/3] CET: Rename CET_MAX to CET_CONTROL_MASK [BZ #25887]

[Florian Weimer]

* 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?

Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global

[Florian Weimer]

* 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?

Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global

[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.

> 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.

Re: [PATCH 2/3] rtld: Get architecture specific initializer in rtld_global

[Florian Weimer]

* 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.

V2 [PATCH] rtld: Get architecture specific initializer in rtld_global

[H.J. Lu]

[-- 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

[PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887]

[H.J. Lu]

[-- 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

Re: [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887]

[Florian Weimer]

* 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

V2 [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887]

[H.J. Lu]

[-- 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

Re: V2 [PATCH] x86: Move CET control to _dl_x86_feature_control [BZ #25887]

[Florian Weimer]

* 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

V2 [PATCH] x86: Add --enable-cet=permissive

[H.J. Lu]

[-- 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

Re: V2 [PATCH] x86: Add --enable-cet=permissive

[Florian Weimer]

* 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