[PATCH v3 0/5] fix ifunc with static pie [BZ #27072]

[PATCH v3 0/5] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy]

v3:
- refactor tunables: move internals out of dl-tunables.h
- use generated max string length in the tunables list
  instead of magic values.

v2:
- check PI_STATIC_AND_HIDDEN for --enable-static-pie
- change string buffer sizes in the tunables
- fix env_alias == NULL logic in __tunables_init
- move __ehdr_start processing after self relocation


force pushed into nsz/bug27072 branch (with a gcc8 bug
workaround for aarch64)

Issues that are not addressed:
- tunables try to allocate memory even with non-suid exe.
  allocation is only needed for rewriting the GLIBC_TUNABLES
  env var. i think a case can be made that if anything there
  is TUNABLE_SECLEVEL_SXID_ERASE then this env var would be
  simply dropped.
- tunable list is not optimized for compactness.
- all symbols are forced hidden in libc.a, but i think lib*.a
  should do the same. (other than lib*_nonshared.a)

Szabolcs Nagy (5):
  configure: Require PI_STATIC_AND_HIDDEN for static pie
  Make libc symbols hidden in static PIE
  elf: Make the tunable struct definition internal only
  elf: Avoid RELATIVE relocs in __tunables_init
  csu: Move static pie self relocation later [BZ #27072]

 configure                | 14 +++++++++++++
 configure.ac             |  5 +++++
 csu/libc-start.c         | 44 +++++++++++++++++++++++-----------------
 elf/dl-tunable-types.h   | 42 +++++++++++++++++++++++++++++---------
 elf/dl-tunables.c        |  2 +-
 elf/dl-tunables.h        | 35 ++++++++------------------------
 include/libc-symbols.h   |  8 ++++++--
 scripts/gen-tunables.awk | 16 +++++++++++++--
 8 files changed, 105 insertions(+), 61 deletions(-)

-- 
2.17.1

[PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie

[Szabolcs Nagy]

The glibc static pie self relocation code relies on that local
symbols can be accessed without dynamic relocations in position
independent code.
---
 configure    | 14 ++++++++++++++
 configure.ac |  5 +++++
 2 files changed, 19 insertions(+)

diff --git a/configure b/configure
index 49f7b32b52..81fd116d87 100755
--- a/configure
+++ b/configure
@@ -6818,6 +6818,20 @@ if test "$static_pie" = yes; then
   if test "$libc_cv_no_dynamic_linker" != yes; then
     as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5
   fi
+
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#ifndef PI_STATIC_AND_HIDDEN
+# error static pie depends on PI_STATIC_AND_HIDDEN
+#endif
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+else
+  as_fn_error $? "the target does not support static pie" "$LINENO" 5
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
   # Default to PIE.
   libc_cv_pie_default=yes
   $as_echo "#define ENABLE_STATIC_PIE 1" >>confdefs.h
diff --git a/configure.ac b/configure.ac
index 341d4eeac2..8b7c99001d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1835,6 +1835,11 @@ if test "$static_pie" = yes; then
   if test "$libc_cv_no_dynamic_linker" != yes; then
     AC_MSG_ERROR([linker support for --no-dynamic-linker needed])
   fi
+
+  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef PI_STATIC_AND_HIDDEN
+# error static pie depends on PI_STATIC_AND_HIDDEN
+#endif]])], , AC_MSG_ERROR([the target does not support static pie]))
+
   # Default to PIE.
   libc_cv_pie_default=yes
   AC_DEFINE(ENABLE_STATIC_PIE)
-- 
2.17.1

[PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

Hidden matters with static PIE: extern symbol access in position
independent code usually involves GOT indirections which needs
RELATIVE relocs in a static linked PIE. Using So hidden avoids
indirections and RELATIVE relocs on targets that can access symbols
pc-relative.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
static libraries can use hidden visibility to avoid indirections,
however the test system links objects from libcrypt.a into dynamic
linked test binaries so hidden does not work there.  I think mixing
static and shared libc components in the same binary should not be
supported usage, but to be safe only use hidden in libc.a.

From -static-pie linked 'int main(){}' this shaves off 73 relative
relocs on aarch64 and reduces code size too.
---
 include/libc-symbols.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ea126ae70c..93e63ee889 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,13 +434,17 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
 #endif
 
+/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
+#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
+# pragma GCC visibility push(hidden)
+#endif
+
 #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
 
 #define attribute_relro __attribute__ ((section (".data.rel.ro")))
-- 
2.17.1

[PATCH v3 3/5] elf: Make the tunable struct definition internal only

[Szabolcs Nagy]

The representation of the tunables including type information and
the tunable list structure are only used in the implementation not
in the tunables api that is exposed to usage within glibc.

This patch moves the representation related definitions into the
existing dl-tunable-types.h and uses that only for implementation.

The tunable callback and related types are moved to dl-tunables.h
because they are part of the tunables api.

This reduces the details exposed in the tunables api so the internals
are easier to change.
---
 elf/dl-tunable-types.h   | 42 ++++++++++++++++++++++++++++++----------
 elf/dl-tunables.h        | 35 ++++++++-------------------------
 scripts/gen-tunables.awk |  4 +++-
 3 files changed, 43 insertions(+), 38 deletions(-)

diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index 8f6a383dcc..05d4958e1c 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -1,4 +1,4 @@
-/* Tunable type information.
+/* Internal representation of tunables.
 
    Copyright (C) 2016-2021 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
@@ -18,8 +18,14 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef _TUNABLE_TYPES_H_
-# define _TUNABLE_TYPES_H_
+#define _TUNABLE_TYPES_H_
+
+/* Note: This header is included in the generated dl-tunables-list.h and
+   only used internally in the tunables implementation in dl-tunables.c.  */
+
+#include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 
 typedef enum
 {
@@ -36,14 +42,6 @@ typedef struct
   int64_t max;
 } tunable_type_t;
 
-typedef union
-{
-  int64_t numval;
-  const char *strval;
-} tunable_val_t;
-
-typedef void (*tunable_callback_t) (tunable_val_t *);
-
 /* Security level for tunables.  This decides what to do with individual
    tunables for AT_SECURE binaries.  */
 typedef enum
@@ -58,5 +56,29 @@ typedef enum
   TUNABLE_SECLEVEL_NONE = 2,
 } tunable_seclevel_t;
 
+/* A tunable.  */
+struct _tunable
+{
+  const char *name;			/* Internal name of the tunable.  */
+  tunable_type_t type;			/* Data type of the tunable.  */
+  tunable_val_t val;			/* The value.  */
+  bool initialized;			/* Flag to indicate that the tunable is
+					   initialized.  */
+  tunable_seclevel_t security_level;	/* Specify the security level for the
+					   tunable with respect to AT_SECURE
+					   programs.  See description of
+					   tunable_seclevel_t to see a
+					   description of the values.
+
+					   Note that even if the tunable is
+					   read, it may not get used by the
+					   target module if the value is
+					   considered unsafe.  */
+  /* Compatibility elements.  */
+  const char *env_alias;		/* The compatibility environment
+					   variable name.  */
+};
+
+typedef struct _tunable tunable_t;
 
 #endif
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 518342a300..1773c7e254 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -21,8 +21,6 @@
 #ifndef _TUNABLES_H_
 #define _TUNABLES_H_
 
-#include <stdbool.h>
-
 #if !HAVE_TUNABLES
 static inline void
 __always_inline
@@ -31,34 +29,17 @@ __tunables_init (char **unused __attribute__ ((unused)))
   /* This is optimized out if tunables are not enabled.  */
 }
 #else
-
+# include <stdbool.h>
 # include <stddef.h>
-# include "dl-tunable-types.h"
+# include <stdint.h>
 
-/* A tunable.  */
-struct _tunable
+typedef union
 {
-  const char *name;			/* Internal name of the tunable.  */
-  tunable_type_t type;			/* Data type of the tunable.  */
-  tunable_val_t val;			/* The value.  */
-  bool initialized;			/* Flag to indicate that the tunable is
-					   initialized.  */
-  tunable_seclevel_t security_level;	/* Specify the security level for the
-					   tunable with respect to AT_SECURE
-					   programs.  See description of
-					   tunable_seclevel_t to see a
-					   description of the values.
-
-					   Note that even if the tunable is
-					   read, it may not get used by the
-					   target module if the value is
-					   considered unsafe.  */
-  /* Compatibility elements.  */
-  const char *env_alias;		/* The compatibility environment
-					   variable name.  */
-};
-
-typedef struct _tunable tunable_t;
+  int64_t numval;
+  const char *strval;
+} tunable_val_t;
+
+typedef void (*tunable_callback_t) (tunable_val_t *);
 
 /* Full name for a tunable is top_ns.tunable_ns.id.  */
 # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index 622199061a..cda12ef62e 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -156,8 +156,10 @@ END {
   }
   print "} tunable_id_t;\n"
 
-  # Finally, the tunable list.
   print "\n#ifdef TUNABLES_INTERNAL"
+  # Internal definitions.
+  print "# include \"dl-tunable-types.h\""
+  # Finally, the tunable list.
   print "static tunable_t tunable_list[] attribute_relro = {"
   for (tnm in types) {
     split (tnm, indices, SUBSEP);
-- 
2.17.1

[PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init

[Szabolcs Nagy]

With static pie linking pointers in the tunables list need
RELATIVE relocs since the absolute address is not known at link
time. We want to avoid relocations so the static pie self
relocation can be done after tunables are initialized.

This is a simple fix that embeds the tunable strings into the
tunable list instead of using pointers.  It is possible to have
a more compact representation of tunables with some additional
complexity in the generator and tunable parser logic.  Such
optimization will be useful if the list of tunables grows.

There is still an issue that tunables_strdup allocates and the
failure handling code path is sufficiently complex that it can
easily have RELATIVE relocations.  It is possible to avoid the
early allocation and only change environment variables in a
setuid exe after relocations are processed.  But that is a
bigger change and early failure is fatal anyway so it is not
as critical to fix right away.
---
 elf/dl-tunable-types.h   |  4 ++--
 elf/dl-tunables.c        |  2 +-
 scripts/gen-tunables.awk | 12 +++++++++++-
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
index 05d4958e1c..3fcc0806f5 100644
--- a/elf/dl-tunable-types.h
+++ b/elf/dl-tunable-types.h
@@ -59,7 +59,7 @@ typedef enum
 /* A tunable.  */
 struct _tunable
 {
-  const char *name;			/* Internal name of the tunable.  */
+  const char name[TUNABLE_NAME_MAX];	/* Internal name of the tunable.  */
   tunable_type_t type;			/* Data type of the tunable.  */
   tunable_val_t val;			/* The value.  */
   bool initialized;			/* Flag to indicate that the tunable is
@@ -75,7 +75,7 @@ struct _tunable
 					   target module if the value is
 					   considered unsafe.  */
   /* Compatibility elements.  */
-  const char *env_alias;		/* The compatibility environment
+  const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment
 					   variable name.  */
 };
 
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 9b4d737fb8..3845b2c04e 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -350,7 +350,7 @@ __tunables_init (char **envp)
 
 	  /* Skip over tunables that have either been set already or should be
 	     skipped.  */
-	  if (cur->initialized || cur->env_alias == NULL)
+	  if (cur->initialized || cur->env_alias[0] == '\0')
 	    continue;
 
 	  const char *name = cur->env_alias;
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index cda12ef62e..fa63e86d1a 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -12,6 +12,8 @@ BEGIN {
   tunable=""
   ns=""
   top_ns=""
+  max_name_len=0
+  max_alias_len=0
 }
 
 # Skip over blank lines and comments.
@@ -57,11 +59,14 @@ $1 == "}" {
       maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]]
     }
     if (!env_alias[top_ns,ns,tunable]) {
-      env_alias[top_ns,ns,tunable] = "NULL"
+      env_alias[top_ns,ns,tunable] = "{0}"
     }
     if (!security_level[top_ns,ns,tunable]) {
       security_level[top_ns,ns,tunable] = "SXID_ERASE"
     }
+    len = length(top_ns"."ns"."tunable)
+    if (len > max_name_len)
+      max_name_len = len
 
     tunable = ""
   }
@@ -109,6 +114,9 @@ $1 == "}" {
   }
   else if (attr == "env_alias") {
     env_alias[top_ns,ns,tunable] = sprintf("\"%s\"", val)
+    len = length(val)
+    if (len > max_alias_len)
+      max_alias_len = len
   }
   else if (attr == "security_level") {
     if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") {
@@ -158,6 +166,8 @@ END {
 
   print "\n#ifdef TUNABLES_INTERNAL"
   # Internal definitions.
+  print "# define TUNABLE_NAME_MAX " (max_name_len + 1)
+  print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1)
   print "# include \"dl-tunable-types.h\""
   # Finally, the tunable list.
   print "static tunable_t tunable_list[] attribute_relro = {"
-- 
2.17.1

[PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy]

IFUNC resolvers may depend on tunables and cpu feature setup so
move static pie self relocation after those.

It is hard to guarantee that the ealy startup code does not rely
on relocations so this is a bit fragile. It would be more robust
to handle RELATIVE relocs early and only IRELATIVE relocs later,
but the current relocation processing code cannot do that.

The early startup code before relocation processing includes

  _dl_aux_init (auxvec);
  __libc_init_secure ();
  __tunables_init (__environ);
  ARCH_INIT_CPU_FEATURES ();

These are simple enough that RELATIVE relocs can be avoided.

__ehdr_start may require RELATIVE relocation so it was moved
later, fortunately ehdr and phdr are not used in the early code.
---
 csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index db859c3bed..fb64cdb2c9 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   int result;
 
 #ifndef SHARED
-  _dl_relocate_static_pie ();
-
   char **ev = &argv[argc + 1];
 
   __environ = ev;
@@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   }
 #  endif
   _dl_aux_init (auxvec);
-  if (GL(dl_phdr) == NULL)
 # endif
-    {
-      /* Starting from binutils-2.23, the linker will define the
-         magic symbol __ehdr_start to point to our own ELF header
-         if it is visible in a segment that also includes the phdrs.
-         So we can set up _dl_phdr and _dl_phnum even without any
-         information from auxv.  */
-
-      extern const ElfW(Ehdr) __ehdr_start
-	__attribute__ ((weak, visibility ("hidden")));
-      if (&__ehdr_start != NULL)
-        {
-          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
-          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
-          GL(dl_phnum) = __ehdr_start.e_phnum;
-        }
-    }
 
   /* Initialize very early so that tunables can use it.  */
   __libc_init_secure ();
@@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 
   ARCH_INIT_CPU_FEATURES ();
 
+  /* Do static pie self relocation after tunables and cpu features
+     are setup for ifunc resolvers. Before this point relocations
+     must be avoided.  */
+  _dl_relocate_static_pie ();
+
   /* Perform IREL{,A} relocations.  */
   ARCH_SETUP_IREL ();
 
@@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
      hwcap and platform fields available in the TCB.  */
   ARCH_APPLY_IREL ();
 
+# ifdef HAVE_AUX_VECTOR
+  if (GL(dl_phdr) == NULL)
+# endif
+    {
+      /* Starting from binutils-2.23, the linker will define the
+         magic symbol __ehdr_start to point to our own ELF header
+         if it is visible in a segment that also includes the phdrs.
+         So we can set up _dl_phdr and _dl_phnum even without any
+         information from auxv.  */
+
+      extern const ElfW(Ehdr) __ehdr_start
+	__attribute__ ((weak, visibility ("hidden")));
+      if (&__ehdr_start != NULL)
+        {
+          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
+          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
+          GL(dl_phnum) = __ehdr_start.e_phnum;
+        }
+    }
+
   /* Set up the stack checker's canary.  */
   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
 # ifdef THREAD_SET_STACK_GUARD
-- 
2.17.1

Re: [PATCH v3 1/5] configure: Require PI_STATIC_AND_HIDDEN for static pie

[Adhemerval Zanella]

On 12/01/2021 14:21, Szabolcs Nagy via Libc-alpha wrote:
> The glibc static pie self relocation code relies on that local
> symbols can be accessed without dynamic relocations in position
> independent code.

LGTM with the nit below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  configure    | 14 ++++++++++++++
>  configure.ac |  5 +++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/configure b/configure
> index 49f7b32b52..81fd116d87 100755
> --- a/configure
> +++ b/configure
> @@ -6818,6 +6818,20 @@ if test "$static_pie" = yes; then
>    if test "$libc_cv_no_dynamic_linker" != yes; then
>      as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5
>    fi
> +
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +#ifndef PI_STATIC_AND_HIDDEN
> +# error static pie depends on PI_STATIC_AND_HIDDEN
> +#endif
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> +
> +else
> +  as_fn_error $? "the target does not support static pie" "$LINENO" 5
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> +
>    # Default to PIE.
>    libc_cv_pie_default=yes
>    $as_echo "#define ENABLE_STATIC_PIE 1" >>confdefs.h
> diff --git a/configure.ac b/configure.ac
> index 341d4eeac2..8b7c99001d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1835,6 +1835,11 @@ if test "$static_pie" = yes; then
>    if test "$libc_cv_no_dynamic_linker" != yes; then
>      AC_MSG_ERROR([linker support for --no-dynamic-linker needed])
>    fi
> +
> +  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef PI_STATIC_AND_HIDDEN
> +# error static pie depends on PI_STATIC_AND_HIDDEN
> +#endif]])], , AC_MSG_ERROR([the target does not support static pie]))
> +

I think we need a more explicit error message stating that the 
architecture does not support static pie, this might give the user 
the impression either a compiler options would fix it or, worse,
if it defines PI_STATIC_AND_HIDDEN on architecture configure.ac
it would fix it.  Maybe "the architecture does not support static pie".

>    # Default to PIE.
>    libc_cv_pie_default=yes
>    AC_DEFINE(ENABLE_STATIC_PIE)
> 

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu]

On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> IFUNC resolvers may depend on tunables and cpu feature setup so
> move static pie self relocation after those.
>
> It is hard to guarantee that the ealy startup code does not rely
> on relocations so this is a bit fragile. It would be more robust
> to handle RELATIVE relocs early and only IRELATIVE relocs later,
> but the current relocation processing code cannot do that.
>
> The early startup code before relocation processing includes
>
>   _dl_aux_init (auxvec);
>   __libc_init_secure ();
>   __tunables_init (__environ);
>   ARCH_INIT_CPU_FEATURES ();
>
> These are simple enough that RELATIVE relocs can be avoided.
>
> __ehdr_start may require RELATIVE relocation so it was moved
> later, fortunately ehdr and phdr are not used in the early code.
> ---
>  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
>  1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index db859c3bed..fb64cdb2c9 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    int result;
>
>  #ifndef SHARED
> -  _dl_relocate_static_pie ();
> -
>    char **ev = &argv[argc + 1];
>
>    __environ = ev;
> @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    }
>  #  endif
>    _dl_aux_init (auxvec);
> -  if (GL(dl_phdr) == NULL)
>  # endif
> -    {
> -      /* Starting from binutils-2.23, the linker will define the
> -         magic symbol __ehdr_start to point to our own ELF header
> -         if it is visible in a segment that also includes the phdrs.
> -         So we can set up _dl_phdr and _dl_phnum even without any
> -         information from auxv.  */
> -
> -      extern const ElfW(Ehdr) __ehdr_start
> -       __attribute__ ((weak, visibility ("hidden")));
> -      if (&__ehdr_start != NULL)
> -        {
> -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> -          GL(dl_phnum) = __ehdr_start.e_phnum;
> -        }
> -    }
>
>    /* Initialize very early so that tunables can use it.  */
>    __libc_init_secure ();
> @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>
>    ARCH_INIT_CPU_FEATURES ();
>
> +  /* Do static pie self relocation after tunables and cpu features
> +     are setup for ifunc resolvers. Before this point relocations
> +     must be avoided.  */
> +  _dl_relocate_static_pie ();
> +
>    /* Perform IREL{,A} relocations.  */
>    ARCH_SETUP_IREL ();
>
> @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>       hwcap and platform fields available in the TCB.  */
>    ARCH_APPLY_IREL ();
>
> +# ifdef HAVE_AUX_VECTOR
> +  if (GL(dl_phdr) == NULL)
> +# endif
> +    {
> +      /* Starting from binutils-2.23, the linker will define the
> +         magic symbol __ehdr_start to point to our own ELF header
> +         if it is visible in a segment that also includes the phdrs.
> +         So we can set up _dl_phdr and _dl_phnum even without any
> +         information from auxv.  */
> +
> +      extern const ElfW(Ehdr) __ehdr_start
> +       __attribute__ ((weak, visibility ("hidden")));
> +      if (&__ehdr_start != NULL)
> +        {
> +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> +          GL(dl_phnum) = __ehdr_start.e_phnum;
> +        }
> +    }
> +
>    /* Set up the stack checker's canary.  */
>    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
>  # ifdef THREAD_SET_STACK_GUARD
> --
> 2.17.1
>

LGTM.

Thanks.

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Using So hidden avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.
> ---
>  include/libc-symbols.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index ea126ae70c..93e63ee889 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -434,13 +434,17 @@ for linking")
>    strong_alias(real, name)
>  #endif
>
> -#if defined SHARED || defined LIBC_NONSHARED \
> -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> +#if defined SHARED || defined LIBC_NONSHARED
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden
>  #endif
>
> +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> +# pragma GCC visibility push(hidden)
> +#endif
> +
>  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
>
>  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> --
> 2.17.1
>

Unfortunately it doesn't work on i686 with --enable-static-pie:

/usr/local/bin/ld:
/export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
unsupported non-PIC call to IFUNC `strcmp'

Please make it opt-out.

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > Hidden matters with static PIE: extern symbol access in position
> > independent code usually involves GOT indirections which needs
> > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > indirections and RELATIVE relocs on targets that can access symbols
> > pc-relative.
> >
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > static libraries can use hidden visibility to avoid indirections,
> > however the test system links objects from libcrypt.a into dynamic
> > linked test binaries so hidden does not work there.  I think mixing
> > static and shared libc components in the same binary should not be
> > supported usage, but to be safe only use hidden in libc.a.
> >
> > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > relocs on aarch64 and reduces code size too.
> > ---
> >  include/libc-symbols.h | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > index ea126ae70c..93e63ee889 100644
> > --- a/include/libc-symbols.h
> > +++ b/include/libc-symbols.h
> > @@ -434,13 +434,17 @@ for linking")
> >    strong_alias(real, name)
> >  #endif
> >
> > -#if defined SHARED || defined LIBC_NONSHARED \
> > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > +#if defined SHARED || defined LIBC_NONSHARED
> >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> >  #else
> >  # define attribute_hidden
> >  #endif
> >
> > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > +# pragma GCC visibility push(hidden)
> > +#endif
> > +
> >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> >
> >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > --
> > 2.17.1
> >
>
> Unfortunately it doesn't work on i686 with --enable-static-pie:
>
> /usr/local/bin/ld:
> /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> unsupported non-PIC call to IFUNC `strcmp'
>
> Please make it opt-out.
>

See:

https://sourceware.org/bugzilla/show_bug.cgi?id=14961

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > Hidden matters with static PIE: extern symbol access in position
> > > independent code usually involves GOT indirections which needs
> > > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > > indirections and RELATIVE relocs on targets that can access symbols
> > > pc-relative.
> > >
> > > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > > static libraries can use hidden visibility to avoid indirections,
> > > however the test system links objects from libcrypt.a into dynamic
> > > linked test binaries so hidden does not work there.  I think mixing
> > > static and shared libc components in the same binary should not be
> > > supported usage, but to be safe only use hidden in libc.a.
> > >
> > > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > > relocs on aarch64 and reduces code size too.
> > > ---
> > >  include/libc-symbols.h | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > > index ea126ae70c..93e63ee889 100644
> > > --- a/include/libc-symbols.h
> > > +++ b/include/libc-symbols.h
> > > @@ -434,13 +434,17 @@ for linking")
> > >    strong_alias(real, name)
> > >  #endif
> > >
> > > -#if defined SHARED || defined LIBC_NONSHARED \
> > > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > > +#if defined SHARED || defined LIBC_NONSHARED
> > >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> > >  #else
> > >  # define attribute_hidden
> > >  #endif
> > >
> > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > +# pragma GCC visibility push(hidden)
> > > +#endif
> > > +
> > >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> > >
> > >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > > --
> > > 2.17.1
> > >
> >
> > Unfortunately it doesn't work on i686 with --enable-static-pie:
> >
> > /usr/local/bin/ld:
> > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > unsupported non-PIC call to IFUNC `strcmp'
> >
> > Please make it opt-out.
> >
>
> See:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=14961
>
> --
> H.J.

Testing:

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index 93e63ee889..f4dd735555 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -441,7 +441,8 @@ for linking")
 #endif

 /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
-#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
+#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+    && IS_IN (libc) && !defined LIBC_NONSHARED
 # pragma GCC visibility push(hidden)
 #endif


-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 12, 2021 at 9:26 AM Szabolcs Nagy via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > > >
> > > > Hidden matters with static PIE: extern symbol access in position
> > > > independent code usually involves GOT indirections which needs
> > > > RELATIVE relocs in a static linked PIE. Using So hidden avoids
> > > > indirections and RELATIVE relocs on targets that can access symbols
> > > > pc-relative.
> > > >
> > > > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > > > static libraries can use hidden visibility to avoid indirections,
> > > > however the test system links objects from libcrypt.a into dynamic
> > > > linked test binaries so hidden does not work there.  I think mixing
> > > > static and shared libc components in the same binary should not be
> > > > supported usage, but to be safe only use hidden in libc.a.
> > > >
> > > > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > > > relocs on aarch64 and reduces code size too.
> > > > ---
> > > >  include/libc-symbols.h | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> > > > index ea126ae70c..93e63ee889 100644
> > > > --- a/include/libc-symbols.h
> > > > +++ b/include/libc-symbols.h
> > > > @@ -434,13 +434,17 @@ for linking")
> > > >    strong_alias(real, name)
> > > >  #endif
> > > >
> > > > -#if defined SHARED || defined LIBC_NONSHARED \
> > > > -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> > > > +#if defined SHARED || defined LIBC_NONSHARED
> > > >  # define attribute_hidden __attribute__ ((visibility ("hidden")))
> > > >  #else
> > > >  # define attribute_hidden
> > > >  #endif
> > > >
> > > > +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > +#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +# pragma GCC visibility push(hidden)
> > > > +#endif
> > > > +
> > > >  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
> > > >
> > > >  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > Unfortunately it doesn't work on i686 with --enable-static-pie:
> > >
> > > /usr/local/bin/ld:
> > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > > unsupported non-PIC call to IFUNC `strcmp'
> > >
> > > Please make it opt-out.
> > >
> >
> > See:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> >
> > --
> > H.J.
>
> Testing:
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index 93e63ee889..f4dd735555 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -441,7 +441,8 @@ for linking")
>  #endif
>
>  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> +    && IS_IN (libc) && !defined LIBC_NONSHARED
>  # pragma GCC visibility push(hidden)
>  #endif
>

This works on i686.


-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

The 01/12/2021 17:19, H.J. Lu wrote:
> On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 12, 2021 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > Unfortunately it doesn't work on i686 with --enable-static-pie:
> > > >
> > > > /usr/local/bin/ld:
> > > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/libc.a(dl-lookup-direct.o):
> > > > unsupported non-PIC call to IFUNC `strcmp'
> > > >
> > > > Please make it opt-out.
> > > >
> > >
> > > See:
> > >
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > >
> >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> >  # pragma GCC visibility push(hidden)
> >  #endif
> >
> 
> This works on i686.

if it works then likely pie uses copy relocs on i686
so object symbols are always accessed locally instead
of via GOT. this sounds a bit fragile.

ideally there would be a way to mark object symbols
hidden but not functions for such targets.

i will add this change for now, thanks.

Re: [PATCH v3 3/5] elf: Make the tunable struct definition internal only

[Adhemerval Zanella]

On 12/01/2021 14:22, Szabolcs Nagy via Libc-alpha wrote:
> The representation of the tunables including type information and
> the tunable list structure are only used in the implementation not
> in the tunables api that is exposed to usage within glibc.
> 
> This patch moves the representation related definitions into the
> existing dl-tunable-types.h and uses that only for implementation.
> 
> The tunable callback and related types are moved to dl-tunables.h
> because they are part of the tunables api.
> 
> This reduces the details exposed in the tunables api so the internals
> are easier to change.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-tunable-types.h   | 42 ++++++++++++++++++++++++++++++----------
>  elf/dl-tunables.h        | 35 ++++++++-------------------------
>  scripts/gen-tunables.awk |  4 +++-
>  3 files changed, 43 insertions(+), 38 deletions(-)
> 
> diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
> index 8f6a383dcc..05d4958e1c 100644
> --- a/elf/dl-tunable-types.h
> +++ b/elf/dl-tunable-types.h
> @@ -1,4 +1,4 @@
> -/* Tunable type information.
> +/* Internal representation of tunables.
>  
>     Copyright (C) 2016-2021 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
> @@ -18,8 +18,14 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef _TUNABLE_TYPES_H_
> -# define _TUNABLE_TYPES_H_
> +#define _TUNABLE_TYPES_H_
> +
> +/* Note: This header is included in the generated dl-tunables-list.h and
> +   only used internally in the tunables implementation in dl-tunables.c.  */
> +
> +#include <stdbool.h>
>  #include <stddef.h>
> +#include <stdint.h>
>  
>  typedef enum
>  {
> @@ -36,14 +42,6 @@ typedef struct
>    int64_t max;
>  } tunable_type_t;
>  
> -typedef union
> -{
> -  int64_t numval;
> -  const char *strval;
> -} tunable_val_t;
> -
> -typedef void (*tunable_callback_t) (tunable_val_t *);
> -
>  /* Security level for tunables.  This decides what to do with individual
>     tunables for AT_SECURE binaries.  */
>  typedef enum
> @@ -58,5 +56,29 @@ typedef enum
>    TUNABLE_SECLEVEL_NONE = 2,
>  } tunable_seclevel_t;
>  
> +/* A tunable.  */
> +struct _tunable
> +{
> +  const char *name;			/* Internal name of the tunable.  */
> +  tunable_type_t type;			/* Data type of the tunable.  */
> +  tunable_val_t val;			/* The value.  */
> +  bool initialized;			/* Flag to indicate that the tunable is
> +					   initialized.  */
> +  tunable_seclevel_t security_level;	/* Specify the security level for the
> +					   tunable with respect to AT_SECURE
> +					   programs.  See description of
> +					   tunable_seclevel_t to see a
> +					   description of the values.
> +
> +					   Note that even if the tunable is
> +					   read, it may not get used by the
> +					   target module if the value is
> +					   considered unsafe.  */
> +  /* Compatibility elements.  */
> +  const char *env_alias;		/* The compatibility environment
> +					   variable name.  */
> +};
> +
> +typedef struct _tunable tunable_t;
>  
>  #endif

Ok.

> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> index 518342a300..1773c7e254 100644
> --- a/elf/dl-tunables.h
> +++ b/elf/dl-tunables.h
> @@ -21,8 +21,6 @@
>  #ifndef _TUNABLES_H_
>  #define _TUNABLES_H_
>  
> -#include <stdbool.h>
> -
>  #if !HAVE_TUNABLES
>  static inline void
>  __always_inline
> @@ -31,34 +29,17 @@ __tunables_init (char **unused __attribute__ ((unused)))
>    /* This is optimized out if tunables are not enabled.  */
>  }
>  #else
> -
> +# include <stdbool.h>
>  # include <stddef.h>
> -# include "dl-tunable-types.h"
> +# include <stdint.h>
>  
> -/* A tunable.  */
> -struct _tunable
> +typedef union
>  {
> -  const char *name;			/* Internal name of the tunable.  */
> -  tunable_type_t type;			/* Data type of the tunable.  */
> -  tunable_val_t val;			/* The value.  */
> -  bool initialized;			/* Flag to indicate that the tunable is
> -					   initialized.  */
> -  tunable_seclevel_t security_level;	/* Specify the security level for the
> -					   tunable with respect to AT_SECURE
> -					   programs.  See description of
> -					   tunable_seclevel_t to see a
> -					   description of the values.
> -
> -					   Note that even if the tunable is
> -					   read, it may not get used by the
> -					   target module if the value is
> -					   considered unsafe.  */
> -  /* Compatibility elements.  */
> -  const char *env_alias;		/* The compatibility environment
> -					   variable name.  */
> -};
> -
> -typedef struct _tunable tunable_t;
> +  int64_t numval;
> +  const char *strval;
> +} tunable_val_t;
> +
> +typedef void (*tunable_callback_t) (tunable_val_t *);
>  
>  /* Full name for a tunable is top_ns.tunable_ns.id.  */
>  # define TUNABLE_NAME_S(top,ns,id) #top "." #ns "." #id

Ok.

> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index 622199061a..cda12ef62e 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -156,8 +156,10 @@ END {
>    }
>    print "} tunable_id_t;\n"
>  
> -  # Finally, the tunable list.
>    print "\n#ifdef TUNABLES_INTERNAL"
> +  # Internal definitions.
> +  print "# include \"dl-tunable-types.h\""
> +  # Finally, the tunable list.
>    print "static tunable_t tunable_list[] attribute_relro = {"
>    for (tnm in types) {
>      split (tnm, indices, SUBSEP);
> 

Ok.

Re: [PATCH v3 4/5] elf: Avoid RELATIVE relocs in __tunables_init

[Adhemerval Zanella]

On 12/01/2021 14:22, Szabolcs Nagy via Libc-alpha wrote:
> With static pie linking pointers in the tunables list need
> RELATIVE relocs since the absolute address is not known at link
> time. We want to avoid relocations so the static pie self
> relocation can be done after tunables are initialized.
> 
> This is a simple fix that embeds the tunable strings into the
> tunable list instead of using pointers.  It is possible to have
> a more compact representation of tunables with some additional
> complexity in the generator and tunable parser logic.  Such
> optimization will be useful if the list of tunables grows.
> 
> There is still an issue that tunables_strdup allocates and the
> failure handling code path is sufficiently complex that it can
> easily have RELATIVE relocations.  It is possible to avoid the
> early allocation and only change environment variables in a
> setuid exe after relocations are processed.  But that is a
> bigger change and early failure is fatal anyway so it is not
> as critical to fix right away.

The _dl_fatal_printf is used quite earlier in the loader, does
still represent a potential failure? If so I think it would be
good to open a bug report to track it.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/dl-tunable-types.h   |  4 ++--
>  elf/dl-tunables.c        |  2 +-
>  scripts/gen-tunables.awk | 12 +++++++++++-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/elf/dl-tunable-types.h b/elf/dl-tunable-types.h
> index 05d4958e1c..3fcc0806f5 100644
> --- a/elf/dl-tunable-types.h
> +++ b/elf/dl-tunable-types.h
> @@ -59,7 +59,7 @@ typedef enum
>  /* A tunable.  */
>  struct _tunable
>  {
> -  const char *name;			/* Internal name of the tunable.  */
> +  const char name[TUNABLE_NAME_MAX];	/* Internal name of the tunable.  */
>    tunable_type_t type;			/* Data type of the tunable.  */
>    tunable_val_t val;			/* The value.  */
>    bool initialized;			/* Flag to indicate that the tunable is
> @@ -75,7 +75,7 @@ struct _tunable
>  					   target module if the value is
>  					   considered unsafe.  */
>    /* Compatibility elements.  */
> -  const char *env_alias;		/* The compatibility environment
> +  const char env_alias[TUNABLE_ALIAS_MAX]; /* The compatibility environment
>  					   variable name.  */
>  };
>  

Ok.

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index 9b4d737fb8..3845b2c04e 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -350,7 +350,7 @@ __tunables_init (char **envp)
>  
>  	  /* Skip over tunables that have either been set already or should be
>  	     skipped.  */
> -	  if (cur->initialized || cur->env_alias == NULL)
> +	  if (cur->initialized || cur->env_alias[0] == '\0')
>  	    continue;
>  
>  	  const char *name = cur->env_alias;

Ok.

> diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
> index cda12ef62e..fa63e86d1a 100644
> --- a/scripts/gen-tunables.awk
> +++ b/scripts/gen-tunables.awk
> @@ -12,6 +12,8 @@ BEGIN {
>    tunable=""
>    ns=""
>    top_ns=""
> +  max_name_len=0
> +  max_alias_len=0
>  }
>  
>  # Skip over blank lines and comments.
> @@ -57,11 +59,14 @@ $1 == "}" {
>        maxvals[top_ns,ns,tunable] = max_of[types[top_ns,ns,tunable]]
>      }
>      if (!env_alias[top_ns,ns,tunable]) {
> -      env_alias[top_ns,ns,tunable] = "NULL"
> +      env_alias[top_ns,ns,tunable] = "{0}"
>      }
>      if (!security_level[top_ns,ns,tunable]) {
>        security_level[top_ns,ns,tunable] = "SXID_ERASE"
>      }
> +    len = length(top_ns"."ns"."tunable)
> +    if (len > max_name_len)
> +      max_name_len = len
>  
>      tunable = ""
>    }
> @@ -109,6 +114,9 @@ $1 == "}" {
>    }
>    else if (attr == "env_alias") {
>      env_alias[top_ns,ns,tunable] = sprintf("\"%s\"", val)
> +    len = length(val)
> +    if (len > max_alias_len)
> +      max_alias_len = len
>    }
>    else if (attr == "security_level") {
>      if (val == "SXID_ERASE" || val == "SXID_IGNORE" || val == "NONE") {
> @@ -158,6 +166,8 @@ END {
>  
>    print "\n#ifdef TUNABLES_INTERNAL"
>    # Internal definitions.
> +  print "# define TUNABLE_NAME_MAX " (max_name_len + 1)
> +  print "# define TUNABLE_ALIAS_MAX " (max_alias_len + 1)
>    print "# include \"dl-tunable-types.h\""
>    # Finally, the tunable list.
>    print "static tunable_t tunable_list[] attribute_relro = {"
> 

Ok.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> The 01/12/2021 17:19, H.J. Lu wrote:
> > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > See:
> > > >
> > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > >
> > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > >  # pragma GCC visibility push(hidden)
> > >  #endif
> > >
> > 
> > This works on i686.

The series i plan to commit today is in nsz/bug27072 now,

This is the v4 of this patch:

Hidden matters with static PIE: extern symbol access in position
independent code usually involves GOT indirections which needs
RELATIVE relocs in a static linked PIE. Hidden visibility avoids
indirections and RELATIVE relocs on targets that can access symbols
pc-relative.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
static libraries can use hidden visibility to avoid indirections,
however the test system links objects from libcrypt.a into dynamic
linked test binaries so hidden does not work there.  I think mixing
static and shared libc components in the same binary should not be
supported usage, but to be safe only use hidden in libc.a.

There are targets (i686) where hidden visibility functions are
problematic in PIE code so hidden cannot be applied to all symbols.
Then static PIE requires extern object access without relocations
(e.g. by relying on copy relocations in shared libraries instead of
GOT access in PIE code). See bug 14961.

From -static-pie linked 'int main(){}' this shaves off 73 relative
relocs on aarch64 and reduces code size too.
---
 include/libc-symbols.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index ea126ae70c..f4dd735555 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,13 +434,18 @@ for linking")
   strong_alias(real, name)
 #endif
 
-#if defined SHARED || defined LIBC_NONSHARED \
-  || (BUILD_PIE_DEFAULT && IS_IN (libc))
+#if defined SHARED || defined LIBC_NONSHARED
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden
 #endif
 
+/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
+#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+    && IS_IN (libc) && !defined LIBC_NONSHARED
+# pragma GCC visibility push(hidden)
+#endif
+
 #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
 
 #define attribute_relro __attribute__ ((section (".data.rel.ro")))
-- 
2.17.1

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > The 01/12/2021 17:19, H.J. Lu wrote:
> > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > See:
> > > > >
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > >
> > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > >  # pragma GCC visibility push(hidden)
> > > >  #endif
> > > >
> > >
> > > This works on i686.
>
> The series i plan to commit today is in nsz/bug27072 now,
>
> This is the v4 of this patch:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> There are targets (i686) where hidden visibility functions are
> problematic in PIE code so hidden cannot be applied to all symbols.
> Then static PIE requires extern object access without relocations
> (e.g. by relying on copy relocations in shared libraries instead of
> GOT access in PIE code). See bug 14961.
>
> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.
> ---
>  include/libc-symbols.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/include/libc-symbols.h b/include/libc-symbols.h
> index ea126ae70c..f4dd735555 100644
> --- a/include/libc-symbols.h
> +++ b/include/libc-symbols.h
> @@ -434,13 +434,18 @@ for linking")
>    strong_alias(real, name)
>  #endif
>
> -#if defined SHARED || defined LIBC_NONSHARED \
> -  || (BUILD_PIE_DEFAULT && IS_IN (libc))
> +#if defined SHARED || defined LIBC_NONSHARED
>  # define attribute_hidden __attribute__ ((visibility ("hidden")))
>  #else
>  # define attribute_hidden
>  #endif
>
> +/* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> +    && IS_IN (libc) && !defined LIBC_NONSHARED
> +# pragma GCC visibility push(hidden)
> +#endif
> +
>  #define attribute_tls_model_ie __attribute__ ((tls_model ("initial-exec")))
>
>  #define attribute_relro __attribute__ ((section (".data.rel.ro")))
> --
> 2.17.1
>

This generates bad static PIE on i386.   This patch is needed:

diff --git a/include/libc-symbols.h b/include/libc-symbols.h
index f4dd735555..72276a5c48 100644
--- a/include/libc-symbols.h
+++ b/include/libc-symbols.h
@@ -434,7 +434,9 @@ for linking")
   strong_alias(real, name)
 #endif

-#if defined SHARED || defined LIBC_NONSHARED
+#if defined SHARED || defined LIBC_NONSHARED \
+  || !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
+  || (BUILD_PIE_DEFAULT && IS_IN (libc))
 # define attribute_hidden __attribute__ ((visibility ("hidden")))
 #else
 # define attribute_hidden

-- 
H.J.

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu]

On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > IFUNC resolvers may depend on tunables and cpu feature setup so
> > move static pie self relocation after those.
> >
> > It is hard to guarantee that the ealy startup code does not rely
> > on relocations so this is a bit fragile. It would be more robust
> > to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > but the current relocation processing code cannot do that.
> >
> > The early startup code before relocation processing includes
> >
> >   _dl_aux_init (auxvec);
> >   __libc_init_secure ();
> >   __tunables_init (__environ);
> >   ARCH_INIT_CPU_FEATURES ();
> >
> > These are simple enough that RELATIVE relocs can be avoided.
> >
> > __ehdr_start may require RELATIVE relocation so it was moved
> > later, fortunately ehdr and phdr are not used in the early code.
> > ---
> >  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
> >  1 file changed, 25 insertions(+), 19 deletions(-)
> >
> > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > index db859c3bed..fb64cdb2c9 100644
> > --- a/csu/libc-start.c
> > +++ b/csu/libc-start.c
> > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >    int result;
> >
> >  #ifndef SHARED
> > -  _dl_relocate_static_pie ();
> > -
> >    char **ev = &argv[argc + 1];
> >
> >    __environ = ev;
> > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >    }
> >  #  endif
> >    _dl_aux_init (auxvec);
> > -  if (GL(dl_phdr) == NULL)
> >  # endif
> > -    {
> > -      /* Starting from binutils-2.23, the linker will define the
> > -         magic symbol __ehdr_start to point to our own ELF header
> > -         if it is visible in a segment that also includes the phdrs.
> > -         So we can set up _dl_phdr and _dl_phnum even without any
> > -         information from auxv.  */
> > -
> > -      extern const ElfW(Ehdr) __ehdr_start
> > -       __attribute__ ((weak, visibility ("hidden")));
> > -      if (&__ehdr_start != NULL)
> > -        {
> > -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > -          GL(dl_phnum) = __ehdr_start.e_phnum;
> > -        }
> > -    }
> >
> >    /* Initialize very early so that tunables can use it.  */
> >    __libc_init_secure ();
> > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >
> >    ARCH_INIT_CPU_FEATURES ();
> >
> > +  /* Do static pie self relocation after tunables and cpu features
> > +     are setup for ifunc resolvers. Before this point relocations
> > +     must be avoided.  */
> > +  _dl_relocate_static_pie ();
> > +
> >    /* Perform IREL{,A} relocations.  */
> >    ARCH_SETUP_IREL ();
> >
> > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> >       hwcap and platform fields available in the TCB.  */
> >    ARCH_APPLY_IREL ();
> >
> > +# ifdef HAVE_AUX_VECTOR
> > +  if (GL(dl_phdr) == NULL)
> > +# endif
> > +    {
> > +      /* Starting from binutils-2.23, the linker will define the
> > +         magic symbol __ehdr_start to point to our own ELF header
> > +         if it is visible in a segment that also includes the phdrs.
> > +         So we can set up _dl_phdr and _dl_phnum even without any
> > +         information from auxv.  */
> > +
> > +      extern const ElfW(Ehdr) __ehdr_start
> > +       __attribute__ ((weak, visibility ("hidden")));
> > +      if (&__ehdr_start != NULL)
> > +        {
> > +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > +          GL(dl_phnum) = __ehdr_start.e_phnum;
> > +        }
> > +    }
> > +
> >    /* Set up the stack checker's canary.  */
> >    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> >  # ifdef THREAD_SET_STACK_GUARD
> > --
> > 2.17.1
> >
>
> LGTM.
>
> Thanks.
>

Unfortunately, this failed on i686:

(gdb) r
Starting program:
/export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln

Program received signal SIGSEGV, Segmentation fault.
0xefec0550 in ?? ()

Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1,
    argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>,
    fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc)
    at ../csu/libc-start.c:145
145   char **ev = &argv[argc + 1];
(gdb) next
147   __environ = ev;
(gdb)
151   __libc_stack_end = stack_end;
(gdb)
160     while (*evp++ != NULL)
(gdb)
165   _dl_aux_init (auxvec);
(gdb)
169   __libc_init_secure ();
(gdb)
171   __tunables_init (__environ);
(gdb)
173   ARCH_INIT_CPU_FEATURES ();
(gdb)
178   _dl_relocate_static_pie ();
(gdb)
181   ARCH_SETUP_IREL ();
(gdb)
184   ARCH_SETUP_TLS ();
(gdb)
203       if (&__ehdr_start != NULL)
(gdb)
212   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
(gdb)
223     DL_SYSDEP_OSCHECK (__libc_fatal);
(gdb)

Program received signal SIGSEGV, Segmentation fault.
0xefec0550 in ?? ()
(gdb)

-- 
H.J.

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu]

On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> > >
> > > IFUNC resolvers may depend on tunables and cpu feature setup so
> > > move static pie self relocation after those.
> > >
> > > It is hard to guarantee that the ealy startup code does not rely
> > > on relocations so this is a bit fragile. It would be more robust
> > > to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > > but the current relocation processing code cannot do that.
> > >
> > > The early startup code before relocation processing includes
> > >
> > >   _dl_aux_init (auxvec);
> > >   __libc_init_secure ();
> > >   __tunables_init (__environ);
> > >   ARCH_INIT_CPU_FEATURES ();
> > >
> > > These are simple enough that RELATIVE relocs can be avoided.
> > >
> > > __ehdr_start may require RELATIVE relocation so it was moved
> > > later, fortunately ehdr and phdr are not used in the early code.
> > > ---
> > >  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
> > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > > index db859c3bed..fb64cdb2c9 100644
> > > --- a/csu/libc-start.c
> > > +++ b/csu/libc-start.c
> > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > >    int result;
> > >
> > >  #ifndef SHARED
> > > -  _dl_relocate_static_pie ();
> > > -
> > >    char **ev = &argv[argc + 1];
> > >
> > >    __environ = ev;
> > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > >    }
> > >  #  endif
> > >    _dl_aux_init (auxvec);
> > > -  if (GL(dl_phdr) == NULL)
> > >  # endif
> > > -    {
> > > -      /* Starting from binutils-2.23, the linker will define the
> > > -         magic symbol __ehdr_start to point to our own ELF header
> > > -         if it is visible in a segment that also includes the phdrs.
> > > -         So we can set up _dl_phdr and _dl_phnum even without any
> > > -         information from auxv.  */
> > > -
> > > -      extern const ElfW(Ehdr) __ehdr_start
> > > -       __attribute__ ((weak, visibility ("hidden")));
> > > -      if (&__ehdr_start != NULL)
> > > -        {
> > > -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > -          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > -        }
> > > -    }
> > >
> > >    /* Initialize very early so that tunables can use it.  */
> > >    __libc_init_secure ();
> > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > >
> > >    ARCH_INIT_CPU_FEATURES ();
> > >
> > > +  /* Do static pie self relocation after tunables and cpu features
> > > +     are setup for ifunc resolvers. Before this point relocations
> > > +     must be avoided.  */
> > > +  _dl_relocate_static_pie ();
> > > +
> > >    /* Perform IREL{,A} relocations.  */
> > >    ARCH_SETUP_IREL ();
> > >
> > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > >       hwcap and platform fields available in the TCB.  */
> > >    ARCH_APPLY_IREL ();
> > >
> > > +# ifdef HAVE_AUX_VECTOR
> > > +  if (GL(dl_phdr) == NULL)
> > > +# endif
> > > +    {
> > > +      /* Starting from binutils-2.23, the linker will define the
> > > +         magic symbol __ehdr_start to point to our own ELF header
> > > +         if it is visible in a segment that also includes the phdrs.
> > > +         So we can set up _dl_phdr and _dl_phnum even without any
> > > +         information from auxv.  */
> > > +
> > > +      extern const ElfW(Ehdr) __ehdr_start
> > > +       __attribute__ ((weak, visibility ("hidden")));
> > > +      if (&__ehdr_start != NULL)
> > > +        {
> > > +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > +          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > +        }
> > > +    }
> > > +
> > >    /* Set up the stack checker's canary.  */
> > >    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> > >  # ifdef THREAD_SET_STACK_GUARD
> > > --
> > > 2.17.1
> > >
> >
> > LGTM.
> >
> > Thanks.
> >
>
> Unfortunately, this failed on i686:
>
> (gdb) r
> Starting program:
> /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln
>
> Program received signal SIGSEGV, Segmentation fault.
> 0xefec0550 in ?? ()
>
> Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1,
>     argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>,
>     fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc)
>     at ../csu/libc-start.c:145
> 145   char **ev = &argv[argc + 1];
> (gdb) next
> 147   __environ = ev;
> (gdb)
> 151   __libc_stack_end = stack_end;
> (gdb)
> 160     while (*evp++ != NULL)
> (gdb)
> 165   _dl_aux_init (auxvec);
> (gdb)
> 169   __libc_init_secure ();
> (gdb)
> 171   __tunables_init (__environ);
> (gdb)
> 173   ARCH_INIT_CPU_FEATURES ();
> (gdb)
> 178   _dl_relocate_static_pie ();
> (gdb)
> 181   ARCH_SETUP_IREL ();
> (gdb)
> 184   ARCH_SETUP_TLS ();
> (gdb)
> 203       if (&__ehdr_start != NULL)
> (gdb)
> 212   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> (gdb)
> 223     DL_SYSDEP_OSCHECK (__libc_fatal);
> (gdb)
>
> Program received signal SIGSEGV, Segmentation fault.
> 0xefec0550 in ?? ()
> (gdb)
>
> --
> H.J.

(gdb) si
uname () at ../sysdeps/unix/syscall-template.S:120
120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) si
0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) si
0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) si
0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
(gdb) si
0xefec0550 in ?? ()
(gdb) disass uname
Dump of assembler code for function uname:
   0xf7fba3a0 <+0>: mov    %ebx,%edx
   0xf7fba3a2 <+2>: mov    0x4(%esp),%ebx
   0xf7fba3a6 <+6>: mov    $0x7a,%eax
   0xf7fba3ab <+11>: call   *%gs:0x10  <<<<<<<<<<<< This may not be setup yet.
   0xf7fba3b2 <+18>: mov    %edx,%ebx
   0xf7fba3b4 <+20>: cmp    $0xfffff001,%eax
   0xf7fba3b9 <+25>: jae    0xf7f9efd0 <__syscall_error>
   0xf7fba3bf <+31>: ret
End of assembler dump.
(gdb)


-- 
H.J.

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu]

On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > > >
> > > > IFUNC resolvers may depend on tunables and cpu feature setup so
> > > > move static pie self relocation after those.
> > > >
> > > > It is hard to guarantee that the ealy startup code does not rely
> > > > on relocations so this is a bit fragile. It would be more robust
> > > > to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > > > but the current relocation processing code cannot do that.
> > > >
> > > > The early startup code before relocation processing includes
> > > >
> > > >   _dl_aux_init (auxvec);
> > > >   __libc_init_secure ();
> > > >   __tunables_init (__environ);
> > > >   ARCH_INIT_CPU_FEATURES ();
> > > >
> > > > These are simple enough that RELATIVE relocs can be avoided.
> > > >
> > > > __ehdr_start may require RELATIVE relocation so it was moved
> > > > later, fortunately ehdr and phdr are not used in the early code.
> > > > ---
> > > >  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
> > > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > > >
> > > > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > > > index db859c3bed..fb64cdb2c9 100644
> > > > --- a/csu/libc-start.c
> > > > +++ b/csu/libc-start.c
> > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > >    int result;
> > > >
> > > >  #ifndef SHARED
> > > > -  _dl_relocate_static_pie ();
> > > > -
> > > >    char **ev = &argv[argc + 1];
> > > >
> > > >    __environ = ev;
> > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > >    }
> > > >  #  endif
> > > >    _dl_aux_init (auxvec);
> > > > -  if (GL(dl_phdr) == NULL)
> > > >  # endif
> > > > -    {
> > > > -      /* Starting from binutils-2.23, the linker will define the
> > > > -         magic symbol __ehdr_start to point to our own ELF header
> > > > -         if it is visible in a segment that also includes the phdrs.
> > > > -         So we can set up _dl_phdr and _dl_phnum even without any
> > > > -         information from auxv.  */
> > > > -
> > > > -      extern const ElfW(Ehdr) __ehdr_start
> > > > -       __attribute__ ((weak, visibility ("hidden")));
> > > > -      if (&__ehdr_start != NULL)
> > > > -        {
> > > > -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > > -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > > -          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > > -        }
> > > > -    }
> > > >
> > > >    /* Initialize very early so that tunables can use it.  */
> > > >    __libc_init_secure ();
> > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > >
> > > >    ARCH_INIT_CPU_FEATURES ();
> > > >
> > > > +  /* Do static pie self relocation after tunables and cpu features
> > > > +     are setup for ifunc resolvers. Before this point relocations
> > > > +     must be avoided.  */
> > > > +  _dl_relocate_static_pie ();
> > > > +
> > > >    /* Perform IREL{,A} relocations.  */
> > > >    ARCH_SETUP_IREL ();
> > > >
> > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > >       hwcap and platform fields available in the TCB.  */
> > > >    ARCH_APPLY_IREL ();
> > > >
> > > > +# ifdef HAVE_AUX_VECTOR
> > > > +  if (GL(dl_phdr) == NULL)
> > > > +# endif
> > > > +    {
> > > > +      /* Starting from binutils-2.23, the linker will define the
> > > > +         magic symbol __ehdr_start to point to our own ELF header
> > > > +         if it is visible in a segment that also includes the phdrs.
> > > > +         So we can set up _dl_phdr and _dl_phnum even without any
> > > > +         information from auxv.  */
> > > > +
> > > > +      extern const ElfW(Ehdr) __ehdr_start
> > > > +       __attribute__ ((weak, visibility ("hidden")));
> > > > +      if (&__ehdr_start != NULL)
> > > > +        {
> > > > +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > > +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > > +          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > > +        }
> > > > +    }
> > > > +
> > > >    /* Set up the stack checker's canary.  */
> > > >    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> > > >  # ifdef THREAD_SET_STACK_GUARD
> > > > --
> > > > 2.17.1
> > > >
> > >
> > > LGTM.
> > >
> > > Thanks.
> > >
> >
> > Unfortunately, this failed on i686:
> >
> > (gdb) r
> > Starting program:
> > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0xefec0550 in ?? ()
> >
> > Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1,
> >     argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>,
> >     fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc)
> >     at ../csu/libc-start.c:145
> > 145   char **ev = &argv[argc + 1];
> > (gdb) next
> > 147   __environ = ev;
> > (gdb)
> > 151   __libc_stack_end = stack_end;
> > (gdb)
> > 160     while (*evp++ != NULL)
> > (gdb)
> > 165   _dl_aux_init (auxvec);
> > (gdb)
> > 169   __libc_init_secure ();
> > (gdb)
> > 171   __tunables_init (__environ);
> > (gdb)
> > 173   ARCH_INIT_CPU_FEATURES ();
> > (gdb)
> > 178   _dl_relocate_static_pie ();
> > (gdb)
> > 181   ARCH_SETUP_IREL ();
> > (gdb)
> > 184   ARCH_SETUP_TLS ();
> > (gdb)
> > 203       if (&__ehdr_start != NULL)
> > (gdb)
> > 212   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> > (gdb)
> > 223     DL_SYSDEP_OSCHECK (__libc_fatal);
> > (gdb)
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > 0xefec0550 in ?? ()
> > (gdb)
> >
> > --
> > H.J.
>
> (gdb) si
> uname () at ../sysdeps/unix/syscall-template.S:120
> 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> (gdb) si
> 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> (gdb) si
> 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> (gdb) si
> 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> (gdb) si
> 0xefec0550 in ?? ()
> (gdb) disass uname
> Dump of assembler code for function uname:
>    0xf7fba3a0 <+0>: mov    %ebx,%edx
>    0xf7fba3a2 <+2>: mov    0x4(%esp),%ebx
>    0xf7fba3a6 <+6>: mov    $0x7a,%eax
>    0xf7fba3ab <+11>: call   *%gs:0x10  <<<<<<<<<<<< This may not be setup yet.
>    0xf7fba3b2 <+18>: mov    %edx,%ebx
>    0xf7fba3b4 <+20>: cmp    $0xfffff001,%eax
>    0xf7fba3b9 <+25>: jae    0xf7f9efd0 <__syscall_error>
>    0xf7fba3bf <+31>: ret
> End of assembler dump.
> (gdb)
>
>
> --
> H.J.

GL(dl_sysinfo) points to the wrong address.  This may affect all
variables accessed
in _dl_aux_init.

-- 
H.J.

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[H.J. Lu]

On Thu, Jan 14, 2021 at 8:01 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > >
> > > > On Tue, Jan 12, 2021 at 9:27 AM Szabolcs Nagy via Libc-alpha
> > > > <libc-alpha@sourceware.org> wrote:
> > > > >
> > > > > IFUNC resolvers may depend on tunables and cpu feature setup so
> > > > > move static pie self relocation after those.
> > > > >
> > > > > It is hard to guarantee that the ealy startup code does not rely
> > > > > on relocations so this is a bit fragile. It would be more robust
> > > > > to handle RELATIVE relocs early and only IRELATIVE relocs later,
> > > > > but the current relocation processing code cannot do that.
> > > > >
> > > > > The early startup code before relocation processing includes
> > > > >
> > > > >   _dl_aux_init (auxvec);
> > > > >   __libc_init_secure ();
> > > > >   __tunables_init (__environ);
> > > > >   ARCH_INIT_CPU_FEATURES ();
> > > > >
> > > > > These are simple enough that RELATIVE relocs can be avoided.
> > > > >
> > > > > __ehdr_start may require RELATIVE relocation so it was moved
> > > > > later, fortunately ehdr and phdr are not used in the early code.
> > > > > ---
> > > > >  csu/libc-start.c | 44 +++++++++++++++++++++++++-------------------
> > > > >  1 file changed, 25 insertions(+), 19 deletions(-)
> > > > >
> > > > > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > > > > index db859c3bed..fb64cdb2c9 100644
> > > > > --- a/csu/libc-start.c
> > > > > +++ b/csu/libc-start.c
> > > > > @@ -142,8 +142,6 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > > >    int result;
> > > > >
> > > > >  #ifndef SHARED
> > > > > -  _dl_relocate_static_pie ();
> > > > > -
> > > > >    char **ev = &argv[argc + 1];
> > > > >
> > > > >    __environ = ev;
> > > > > @@ -165,24 +163,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > > >    }
> > > > >  #  endif
> > > > >    _dl_aux_init (auxvec);
> > > > > -  if (GL(dl_phdr) == NULL)
> > > > >  # endif
> > > > > -    {
> > > > > -      /* Starting from binutils-2.23, the linker will define the
> > > > > -         magic symbol __ehdr_start to point to our own ELF header
> > > > > -         if it is visible in a segment that also includes the phdrs.
> > > > > -         So we can set up _dl_phdr and _dl_phnum even without any
> > > > > -         information from auxv.  */
> > > > > -
> > > > > -      extern const ElfW(Ehdr) __ehdr_start
> > > > > -       __attribute__ ((weak, visibility ("hidden")));
> > > > > -      if (&__ehdr_start != NULL)
> > > > > -        {
> > > > > -          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > > > -          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > > > -          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > > > -        }
> > > > > -    }
> > > > >
> > > > >    /* Initialize very early so that tunables can use it.  */
> > > > >    __libc_init_secure ();
> > > > > @@ -191,6 +172,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > > >
> > > > >    ARCH_INIT_CPU_FEATURES ();
> > > > >
> > > > > +  /* Do static pie self relocation after tunables and cpu features
> > > > > +     are setup for ifunc resolvers. Before this point relocations
> > > > > +     must be avoided.  */
> > > > > +  _dl_relocate_static_pie ();
> > > > > +
> > > > >    /* Perform IREL{,A} relocations.  */
> > > > >    ARCH_SETUP_IREL ();
> > > > >
> > > > > @@ -202,6 +188,26 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> > > > >       hwcap and platform fields available in the TCB.  */
> > > > >    ARCH_APPLY_IREL ();
> > > > >
> > > > > +# ifdef HAVE_AUX_VECTOR
> > > > > +  if (GL(dl_phdr) == NULL)
> > > > > +# endif
> > > > > +    {
> > > > > +      /* Starting from binutils-2.23, the linker will define the
> > > > > +         magic symbol __ehdr_start to point to our own ELF header
> > > > > +         if it is visible in a segment that also includes the phdrs.
> > > > > +         So we can set up _dl_phdr and _dl_phnum even without any
> > > > > +         information from auxv.  */
> > > > > +
> > > > > +      extern const ElfW(Ehdr) __ehdr_start
> > > > > +       __attribute__ ((weak, visibility ("hidden")));
> > > > > +      if (&__ehdr_start != NULL)
> > > > > +        {
> > > > > +          assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
> > > > > +          GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> > > > > +          GL(dl_phnum) = __ehdr_start.e_phnum;
> > > > > +        }
> > > > > +    }
> > > > > +
> > > > >    /* Set up the stack checker's canary.  */
> > > > >    uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> > > > >  # ifdef THREAD_SET_STACK_GUARD
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > > > LGTM.
> > > >
> > > > Thanks.
> > > >
> > >
> > > Unfortunately, this failed on i686:
> > >
> > > (gdb) r
> > > Starting program:
> > > /export/build/gnu/tools-build/glibc-32bit-static-pie-gitlab/build-i686-linux/elf/sln
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0xefec0550 in ?? ()
> > >
> > > Breakpoint 1, __libc_start_main (main=0xf7f64760 <main>, argc=1,
> > >     argv=0xffffc704, init=0xf7f670e0 <__libc_csu_init>,
> > >     fini=0xf7f67190 <__libc_csu_fini>, rtld_fini=0x0, stack_end=0xffffc6fc)
> > >     at ../csu/libc-start.c:145
> > > 145   char **ev = &argv[argc + 1];
> > > (gdb) next
> > > 147   __environ = ev;
> > > (gdb)
> > > 151   __libc_stack_end = stack_end;
> > > (gdb)
> > > 160     while (*evp++ != NULL)
> > > (gdb)
> > > 165   _dl_aux_init (auxvec);
> > > (gdb)
> > > 169   __libc_init_secure ();
> > > (gdb)
> > > 171   __tunables_init (__environ);
> > > (gdb)
> > > 173   ARCH_INIT_CPU_FEATURES ();
> > > (gdb)
> > > 178   _dl_relocate_static_pie ();
> > > (gdb)
> > > 181   ARCH_SETUP_IREL ();
> > > (gdb)
> > > 184   ARCH_SETUP_TLS ();
> > > (gdb)
> > > 203       if (&__ehdr_start != NULL)
> > > (gdb)
> > > 212   uintptr_t stack_chk_guard = _dl_setup_stack_chk_guard (_dl_random);
> > > (gdb)
> > > 223     DL_SYSDEP_OSCHECK (__libc_fatal);
> > > (gdb)
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0xefec0550 in ?? ()
> > > (gdb)
> > >
> > > --
> > > H.J.
> >
> > (gdb) si
> > uname () at ../sysdeps/unix/syscall-template.S:120
> > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xefec0550 in ?? ()
> > (gdb) disass uname
> > Dump of assembler code for function uname:
> >    0xf7fba3a0 <+0>: mov    %ebx,%edx
> >    0xf7fba3a2 <+2>: mov    0x4(%esp),%ebx
> >    0xf7fba3a6 <+6>: mov    $0x7a,%eax
> >    0xf7fba3ab <+11>: call   *%gs:0x10  <<<<<<<<<<<< This may not be setup yet.
> >    0xf7fba3b2 <+18>: mov    %edx,%ebx
> >    0xf7fba3b4 <+20>: cmp    $0xfffff001,%eax
> >    0xf7fba3b9 <+25>: jae    0xf7f9efd0 <__syscall_error>
> >    0xf7fba3bf <+31>: ret
> > End of assembler dump.
> > (gdb)
> >
> >
> > --
> > H.J.
>
> GL(dl_sysinfo) points to the wrong address.  This may affect all
> variables accessed
> in _dl_aux_init.
>
> --
> H.J.

We need to make sure that there are no RELATIVE relocations before
_dl_relocate_static_pie is called.  The problems with i386 are

1.  All calls to IFUNC functions must go through PLT.
2.  Calls to hidden functions CANNOT go through PLT in PIE since
EBX used in PIE PLT may not be set up for local calls.

I think we should add a new attribute, attribute_hidden_ifunc
which should be defined as

1. __attribute__ ((visibility ("default"))) if in PIE on i386
2. __attribute__ ((visibility ("hidden"))) else

attribute_hidden_ifunc should be used on prototypes of all IFUNC
functions.  This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE.


-- 
H.J.

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy]

The 01/14/2021 08:01, H.J. Lu wrote:
> On Thu, Jan 14, 2021 at 7:52 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > On Thu, Jan 14, 2021 at 7:49 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > On Tue, Jan 12, 2021 at 2:55 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > (gdb)
> > > 223     DL_SYSDEP_OSCHECK (__libc_fatal);
> > > (gdb)
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > > 0xefec0550 in ?? ()
> > > (gdb)
> > >
> > (gdb) si
> > uname () at ../sysdeps/unix/syscall-template.S:120
> > 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3a2 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3a6 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xf7fba3ab 120 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
> > (gdb) si
> > 0xefec0550 in ?? ()
> > (gdb) disass uname
> > Dump of assembler code for function uname:
> >    0xf7fba3a0 <+0>: mov    %ebx,%edx
> >    0xf7fba3a2 <+2>: mov    0x4(%esp),%ebx
> >    0xf7fba3a6 <+6>: mov    $0x7a,%eax
> >    0xf7fba3ab <+11>: call   *%gs:0x10  <<<<<<<<<<<< This may not be setup yet.
> >    0xf7fba3b2 <+18>: mov    %edx,%ebx
> >    0xf7fba3b4 <+20>: cmp    $0xfffff001,%eax
> >    0xf7fba3b9 <+25>: jae    0xf7f9efd0 <__syscall_error>
> >    0xf7fba3bf <+31>: ret
> > End of assembler dump.
> > (gdb)
> 
> GL(dl_sysinfo) points to the wrong address.  This may affect all
> variables accessed
> in _dl_aux_init.

so is GL(dl_sysinfo_*) accessed via a GOT entry which
require relocations or is this some other problem?
it's not clear to me from this description, but

the hidden visibility was added to avoid the GOT problem,
without that it won't work, unless i686 has some
magic to avoid GOT access for extern objects in PIE
(which i thought it might have because of copy relocs).

the solution is to ensure object symbols are hidden
but functions aren't (so functions use the normal PIC
call abi on i686 which is compatible with PIE ifunc)
and hope that there are no extern function address
computations in the early start code.

but i don't see an easy way to do that (other than
maintaining manual annotations either on object or
function declarations).

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy]

The 01/14/2021 08:26, H.J. Lu wrote:
> 
> We need to make sure that there are no RELATIVE relocations before
> _dl_relocate_static_pie is called.  The problems with i386 are
> 
> 1.  All calls to IFUNC functions must go through PLT.
> 2.  Calls to hidden functions CANNOT go through PLT in PIE since
> EBX used in PIE PLT may not be set up for local calls.
> 
> I think we should add a new attribute, attribute_hidden_ifunc
> which should be defined as
> 
> 1. __attribute__ ((visibility ("default"))) if in PIE on i386
> 2. __attribute__ ((visibility ("hidden"))) else
> 
> attribute_hidden_ifunc should be used on prototypes of all IFUNC
> functions.  This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE.

so is it enough to declare ifuncs with such attribute?

e.g. would it work if memcpy is default visibility
in PIE libc.a but user code is static linking that
with non-pie caller?

do we have a way to track which functions may be
defined as ifunc? should we do that manually? or add
the attribute to every extern function declaration
within the libc?

Re: [PATCH v3 5/5] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy]

The 01/14/2021 17:19, Szabolcs Nagy via Libc-alpha wrote:
> The 01/14/2021 08:26, H.J. Lu wrote:
> > 
> > We need to make sure that there are no RELATIVE relocations before
> > _dl_relocate_static_pie is called.  The problems with i386 are
> > 
> > 1.  All calls to IFUNC functions must go through PLT.
> > 2.  Calls to hidden functions CANNOT go through PLT in PIE since
> > EBX used in PIE PLT may not be set up for local calls.
> > 
> > I think we should add a new attribute, attribute_hidden_ifunc
> > which should be defined as
> > 
> > 1. __attribute__ ((visibility ("default"))) if in PIE on i386
> > 2. __attribute__ ((visibility ("hidden"))) else
> > 
> > attribute_hidden_ifunc should be used on prototypes of all IFUNC
> > functions.  This is similar to NO_HIDDEN_EXTERN_FUNC_IN_PIE.
> 
> so is it enough to declare ifuncs with such attribute?
> 
> e.g. would it work if memcpy is default visibility
> in PIE libc.a but user code is static linking that
> with non-pie caller?

hm no, i think the only inconsistency that can happen
is if an ifunc function is marked hidden in non-pie
libc.a, but user calls it with default visibility,
but that is not a problem i guess.

maybe this works:

target gives a list of ifunc declarations with explicit
visibility attribute (default vis for i686 PIE) in a
header that is pre-included very early everywhere so later
declarations keep the explicit visibility instead of the
one specified by the gcc pragma. (but i don't yet see how
to have the right prototypes in an early declaration)

> 
> do we have a way to track which functions may be
> defined as ifunc? should we do that manually? or add
> the attribute to every extern function declaration
> within the libc?

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]

On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > The 01/12/2021 17:19, H.J. Lu wrote:
> > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > See:
> > > > >
> > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > >
> > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > >  # pragma GCC visibility push(hidden)
> > > >  #endif
> > > >
> > >
> > > This works on i686.
>
> The series i plan to commit today is in nsz/bug27072 now,
>
> This is the v4 of this patch:
>
> Hidden matters with static PIE: extern symbol access in position
> independent code usually involves GOT indirections which needs
> RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> indirections and RELATIVE relocs on targets that can access symbols
> pc-relative.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> static libraries can use hidden visibility to avoid indirections,
> however the test system links objects from libcrypt.a into dynamic
> linked test binaries so hidden does not work there.  I think mixing
> static and shared libc components in the same binary should not be
> supported usage, but to be safe only use hidden in libc.a.
>
> There are targets (i686) where hidden visibility functions are
> problematic in PIE code so hidden cannot be applied to all symbols.
> Then static PIE requires extern object access without relocations
> (e.g. by relying on copy relocations in shared libraries instead of
> GOT access in PIE code). See bug 14961.

It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
additional patches to make static PIE to work on i386 and x86-64.
I am enclosing my patches.  Please include them in your patch set.

> From -static-pie linked 'int main(){}' this shaves off 73 relative
> relocs on aarch64 and reduces code size too.


-- 
H.J.

[-- Attachment #2: 0001-libmvec-Add-extra-test-objs-to-test-extras.patch --]
[-- Type: text/x-patch, Size: 1695 bytes --]

From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Jan 2021 18:39:24 -0800
Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras

Add extra-test-objs to test-extras so that they are compiled with
-DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.
---
 sysdeps/x86_64/fpu/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
index a4ff2723a8..9a4bdd075c 100644
--- a/sysdeps/x86_64/fpu/Makefile
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
 tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \
 	 test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \
 	 test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2
+test-extras += test-double-libmvec-sincos-avx-main \
+		   test-double-libmvec-sincos-avx2-main \
+		   test-double-libmvec-sincos-main \
+		   test-float-libmvec-sincosf-avx-main \
+		   test-float-libmvec-sincosf-avx2-main \
+		   test-float-libmvec-sincosf-main
 extra-test-objs += test-double-libmvec-sincos-avx-main.o \
 		   test-double-libmvec-sincos-avx2-main.o \
 		   test-double-libmvec-sincos-main.o \
@@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512))
 libmvec-tests += double-vlen8 float-vlen16
 tests += test-double-libmvec-sincos-avx512 \
 	 test-float-libmvec-sincosf-avx512
+test-extras += test-double-libmvec-sincos-avx512-main \
+	       test-float-libmvec-sincosf-avx512-main
 extra-test-objs += test-double-libmvec-sincos-avx512-main.o \
 		   test-float-libmvec-sincosf-avx512-main.o
 
-- 
2.29.2


[-- Attachment #3: 0002-Make-all-symbols-used-by-_dl_relocate_static_pie-hid.patch --]
[-- Type: text/x-patch, Size: 1006 bytes --]

From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Jan 2021 16:40:43 -0800
Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden

On i386, all calls to IFUNC functions must go through PLT and calls to
hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
may not be set up for local calls to hidden IFUNC functions.

Even if we can't make all libc symbols hidden for static PIE on i386, we
must make all symbols used by _dl_relocate_static_pie hidden.
---
 elf/dl-reloc-static-pie.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index a8d964061e..cc34c2d2fe 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if ENABLE_STATIC_PIE
+#pragma GCC visibility push(hidden)
 #include <unistd.h>
 #include <ldsodefs.h>
 #include "dynamic-link.h"
-- 
2.29.2


[-- Attachment #4: 0003-i386-Call-_dl_aux_init-after-relocating-static-PIE.patch --]
[-- Type: text/x-patch, Size: 17002 bytes --]

From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Jan 2021 13:26:29 -0800
Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE

In i386 static PIE, we need to call _dl_aux_init after relocating static
PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in
sync.  Also in i386 static PIE, since __libc_init_secure is called before
ARCH_SETUP_TLS, it must use "int $0x80" for system calls.  Update
__libc_init_secure to use __geteuid_startup, __getuid_startup,
__getegid_startup and __getgid_startup.
---
 config.h.in                                         |  3 +++
 csu/libc-start.c                                    |  8 ++++++++
 elf/enbl-secure.c                                   |  4 ++--
 include/unistd.h                                    |  4 ++++
 sysdeps/i386/configure                              |  3 +++
 sysdeps/i386/configure.ac                           |  4 ++++
 sysdeps/mach/hurd/getegid.c                         |  1 +
 sysdeps/mach/hurd/geteuid.c                         |  1 +
 sysdeps/mach/hurd/getgid.c                          |  1 +
 sysdeps/mach/hurd/getuid.c                          |  1 +
 sysdeps/unix/syscalls.list                          |  4 ++--
 sysdeps/unix/sysv/linux/arm/syscalls.list           |  8 ++++----
 sysdeps/unix/sysv/linux/i386/Makefile               |  8 ++++++++
 sysdeps/unix/sysv/linux/i386/getegid-startup.S      | 12 ++++++++++++
 sysdeps/unix/sysv/linux/i386/geteuid-startup.S      | 12 ++++++++++++
 sysdeps/unix/sysv/linux/i386/getgid-startup.S       | 12 ++++++++++++
 sysdeps/unix/sysv/linux/i386/getuid-startup.S       | 12 ++++++++++++
 sysdeps/unix/sysv/linux/m68k/syscalls.list          |  8 ++++----
 sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list  |  8 ++++----
 sysdeps/unix/sysv/linux/sh/syscalls.list            |  8 ++++----
 sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list |  8 ++++----
 sysdeps/unix/sysv/linux/syscalls.list               |  4 ++--
 22 files changed, 108 insertions(+), 26 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/i386/getegid-startup.S
 create mode 100644 sysdeps/unix/sysv/linux/i386/geteuid-startup.S
 create mode 100644 sysdeps/unix/sysv/linux/i386/getgid-startup.S
 create mode 100644 sysdeps/unix/sysv/linux/i386/getuid-startup.S

diff --git a/config.h.in b/config.h.in
index 947feeff36..70d3cbde0f 100644
--- a/config.h.in
+++ b/config.h.in
@@ -94,6 +94,9 @@
    and does not need relocations.  */
 #undef	PI_STATIC_AND_HIDDEN
 
+/* Define if _dl_aux_init should be called after relocating static PIE.  */
+#undef CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE
+
 /* Define this to disable the 'hidden_proto' et al macros in
    include/libc-symbols.h that avoid PLT slots in PIE.  */
 #undef  NO_HIDDEN_EXTERN_FUNC_IN_PIE
diff --git a/csu/libc-start.c b/csu/libc-start.c
index fb64cdb2c9..e82c9473c8 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -162,7 +162,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
     auxvec = (ElfW(auxv_t) *) evp;
   }
 #  endif
+#  if !(BUILD_PIE_DEFAULT && defined HAVE_AUX_VECTOR \
+	&& defined CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE)
   _dl_aux_init (auxvec);
+#  endif
 # endif
 
   /* Initialize very early so that tunables can use it.  */
@@ -177,6 +180,11 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
      must be avoided.  */
   _dl_relocate_static_pie ();
 
+# if BUILD_PIE_DEFAULT && defined HAVE_AUX_VECTOR \
+     && defined CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE
+  _dl_aux_init (auxvec);
+# endif
+
   /* Perform IREL{,A} relocations.  */
   ARCH_SETUP_IREL ();
 
diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
index bc8c5e96d2..2fadf96eed 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -31,6 +31,6 @@ void
 __libc_init_secure (void)
 {
   if (__libc_enable_secure_decided == 0)
-    __libc_enable_secure = (__geteuid () != __getuid ()
-			    || __getegid () != __getgid ());
+    __libc_enable_secure = (__geteuid_startup () != __getuid_startup ()
+			    || __getegid_startup () != __getgid_startup ());
 }
diff --git a/include/unistd.h b/include/unistd.h
index 54becbc9eb..67fe75f1e0 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -113,9 +113,13 @@ libc_hidden_proto (__getpid)
 extern __pid_t __getppid (void);
 extern __pid_t __setsid (void) attribute_hidden;
 extern __uid_t __getuid (void) attribute_hidden;
+extern __uid_t __getuid_startup (void) attribute_hidden;
 extern __uid_t __geteuid (void) attribute_hidden;
+extern __uid_t __geteuid_startup (void) attribute_hidden;
 extern __gid_t __getgid (void) attribute_hidden;
+extern __gid_t __getgid_startup (void) attribute_hidden;
 extern __gid_t __getegid (void) attribute_hidden;
+extern __gid_t __getegid_startup (void) attribute_hidden;
 extern int __getgroups (int __size, __gid_t __list[]) attribute_hidden;
 libc_hidden_proto (__getpgid)
 extern int __group_member (__gid_t __gid) attribute_hidden;
diff --git a/sysdeps/i386/configure b/sysdeps/i386/configure
index 90c63caf35..60f7afd66b 100644
--- a/sysdeps/i386/configure
+++ b/sysdeps/i386/configure
@@ -113,6 +113,9 @@ fi
 $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
 
 
+$as_echo "#define CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE 1" >>confdefs.h
+
+
 if test x"$multi_arch" != xno; then
   $as_echo "#define NO_HIDDEN_EXTERN_FUNC_IN_PIE 1" >>confdefs.h
 
diff --git a/sysdeps/i386/configure.ac b/sysdeps/i386/configure.ac
index 6d2068d2b3..0a7f233de8 100644
--- a/sysdeps/i386/configure.ac
+++ b/sysdeps/i386/configure.ac
@@ -72,6 +72,10 @@ dnl It is always possible to access static and hidden symbols in an
 dnl position independent way.
 AC_DEFINE(PI_STATIC_AND_HIDDEN)
 
+dnl Call _dl_aux_init after relocating static PIE so that symbol addresses
+dnl in _dl_aux_init and ARCH_SETUP_TLS are in sync.
+AC_DEFINE(CALL_DL_AUX_INIT_AFTER_RELOCATING_STATIC_PIE)
+
 dnl When multi-arch is enabled, all external functions must be called
 dnl via PIC PLT in PIE, which requires setting up EBX register.
 if test x"$multi_arch" != xno; then
diff --git a/sysdeps/mach/hurd/getegid.c b/sysdeps/mach/hurd/getegid.c
index 5a3db22746..da860d3544 100644
--- a/sysdeps/mach/hurd/getegid.c
+++ b/sysdeps/mach/hurd/getegid.c
@@ -54,3 +54,4 @@ __getegid (void)
 }
 
 weak_alias (__getegid, getegid)
+weak_alias (__getegid, __getegid_startup)
diff --git a/sysdeps/mach/hurd/geteuid.c b/sysdeps/mach/hurd/geteuid.c
index a7af5a9d0d..eecdb7383b 100644
--- a/sysdeps/mach/hurd/geteuid.c
+++ b/sysdeps/mach/hurd/geteuid.c
@@ -54,3 +54,4 @@ __geteuid (void)
 }
 
 weak_alias (__geteuid, geteuid)
+weak_alias (__geteuid, __geteuid_startup)
diff --git a/sysdeps/mach/hurd/getgid.c b/sysdeps/mach/hurd/getgid.c
index aa13884a8f..4f9414fee7 100644
--- a/sysdeps/mach/hurd/getgid.c
+++ b/sysdeps/mach/hurd/getgid.c
@@ -51,3 +51,4 @@ __getgid (void)
 }
 
 weak_alias (__getgid, getgid)
+weak_alias (__getgid, __getgid_startup)
diff --git a/sysdeps/mach/hurd/getuid.c b/sysdeps/mach/hurd/getuid.c
index fc4a441569..7a70d05bf8 100644
--- a/sysdeps/mach/hurd/getuid.c
+++ b/sysdeps/mach/hurd/getuid.c
@@ -51,3 +51,4 @@ __getuid (void)
 }
 
 weak_alias (__getuid, getuid)
+weak_alias (__getuid, __getuid_startup)
diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
index 7209c24110..6850bc5913 100644
--- a/sysdeps/unix/syscalls.list
+++ b/sysdeps/unix/syscalls.list
@@ -21,7 +21,7 @@ fcntl		-	fcntl		Ci:iiF	__libc_fcntl	__fcntl fcntl
 fstatfs		-	fstatfs		i:ip	__fstatfs	fstatfs
 ftruncate	-	ftruncate	i:ii	__ftruncate	ftruncate
 getdomain	-	getdomainname	i:si	getdomainname
-getgid		-	getgid		Ei:	__getgid	getgid
+getgid		-	getgid		Ei:	__getgid	getgid		__getgid_startup
 getgroups	-	getgroups	i:ip	__getgroups	getgroups
 gethostid	-	gethostid	i:	gethostid
 gethostname	-	gethostname	i:bn	__gethostname	gethostname
@@ -31,7 +31,7 @@ getpriority	-	getpriority	i:ii	__getpriority	getpriority
 getrlimit	-	getrlimit	i:ip	__getrlimit	getrlimit
 getsockname	-	getsockname	i:ibN	__getsockname	getsockname
 getsockopt	-	getsockopt	i:iiiBN	getsockopt
-getuid		-	getuid		Ei:	__getuid	getuid
+getuid		-	getuid		Ei:	__getuid	getuid		__getuid_startup
 ioctl		-	ioctl		i:iiI	__ioctl		ioctl
 kill		-	kill		i:ii	__kill		kill
 link		-	link		i:ss	__link		link
diff --git a/sysdeps/unix/sysv/linux/arm/syscalls.list b/sysdeps/unix/sysv/linux/arm/syscalls.list
index 13441f7eb4..4b5b1f1960 100644
--- a/sysdeps/unix/sysv/linux/arm/syscalls.list
+++ b/sysdeps/unix/sysv/linux/arm/syscalls.list
@@ -4,10 +4,10 @@ chown		-	chown32		i:sii	__chown		chown
 lchown		-	lchown32	i:sii	__lchown	lchown
 fchown		-	fchown32	i:iii	__fchown	fchown
 
-getegid		-	getegid32	Ei:	__getegid	getegid
-geteuid		-	geteuid32	Ei:	__geteuid	geteuid
-getgid		-	getgid32	Ei:	__getgid	getgid
-getuid		-	getuid32	Ei:	__getuid	getuid
+getegid		-	getegid32	Ei:	__getegid	getegid		__getegid_startup
+geteuid		-	geteuid32	Ei:	__geteuid	geteuid		__getegid_startup
+getgid		-	getgid32	Ei:	__getgid	getgid		__getgid_startup
+getuid		-	getuid32	Ei:	__getuid	getuid		__getuid_startup
 getresgid	-	getresgid32	i:ppp	__getresgid	getresgid
 getresuid	-	getresuid32	i:ppp	__getresuid	getresuid
 getgroups	-	getgroups32	i:ip	__getgroups	getgroups
diff --git a/sysdeps/unix/sysv/linux/i386/Makefile b/sysdeps/unix/sysv/linux/i386/Makefile
index da716e2c1b..e3732bc1a8 100644
--- a/sysdeps/unix/sysv/linux/i386/Makefile
+++ b/sysdeps/unix/sysv/linux/i386/Makefile
@@ -23,6 +23,14 @@ libpthread-sysdep_routines += libc-do-syscall
 libpthread-shared-only-routines += libc-do-syscall
 endif
 
+ifeq ($(subdir),posix)
+sysdep_routines += \
+  getegid-startup \
+  geteuid-startup \
+  getgid-startup \
+  getuid-startup
+endif
+
 ifeq ($(subdir),stdlib)
 gen-as-const-headers += ucontext_i.sym
 endif
diff --git a/sysdeps/unix/sysv/linux/i386/getegid-startup.S b/sysdeps/unix/sysv/linux/i386/getegid-startup.S
new file mode 100644
index 0000000000..4e2cb01361
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/i386/getegid-startup.S
@@ -0,0 +1,12 @@
+#ifndef SHARED
+/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
+# define I386_USE_SYSENTER 0
+# define SYSCALL_NAME getegid32
+# define SYSCALL_NARGS 0
+# define SYSCALL_ULONG_ARG_1 0
+# define SYSCALL_ULONG_ARG_2 0
+# define SYSCALL_SYMBOL __getegid_startup
+# define SYSCALL_NOERRNO 1
+# define SYSCALL_ERRVAL 0
+# include <syscall-template.S>
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/geteuid-startup.S b/sysdeps/unix/sysv/linux/i386/geteuid-startup.S
new file mode 100644
index 0000000000..ed97bdb3e1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/i386/geteuid-startup.S
@@ -0,0 +1,12 @@
+#ifndef SHARED
+/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
+# define I386_USE_SYSENTER 0
+# define SYSCALL_NAME geteuid32
+# define SYSCALL_NARGS 0
+# define SYSCALL_ULONG_ARG_1 0
+# define SYSCALL_ULONG_ARG_2 0
+# define SYSCALL_SYMBOL __geteuid_startup
+# define SYSCALL_NOERRNO 1
+# define SYSCALL_ERRVAL 0
+# include <syscall-template.S>
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/getgid-startup.S b/sysdeps/unix/sysv/linux/i386/getgid-startup.S
new file mode 100644
index 0000000000..dd75b3f2c2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/i386/getgid-startup.S
@@ -0,0 +1,12 @@
+#ifndef SHARED
+/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
+# define I386_USE_SYSENTER 0
+# define SYSCALL_NAME getgid32
+# define SYSCALL_NARGS 0
+# define SYSCALL_ULONG_ARG_1 0
+# define SYSCALL_ULONG_ARG_2 0
+# define SYSCALL_SYMBOL __getgid_startup
+# define SYSCALL_NOERRNO 1
+# define SYSCALL_ERRVAL 0
+# include <syscall-template.S>
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/getuid-startup.S b/sysdeps/unix/sysv/linux/i386/getuid-startup.S
new file mode 100644
index 0000000000..51fed82ca2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/i386/getuid-startup.S
@@ -0,0 +1,12 @@
+#ifndef SHARED
+/* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
+# define I386_USE_SYSENTER 0
+# define SYSCALL_NAME getuid32
+# define SYSCALL_NARGS 0
+# define SYSCALL_ULONG_ARG_1 0
+# define SYSCALL_ULONG_ARG_2 0
+# define SYSCALL_SYMBOL __getuid_startup
+# define SYSCALL_NOERRNO 1
+# define SYSCALL_ERRVAL 0
+# include <syscall-template.S>
+#endif
diff --git a/sysdeps/unix/sysv/linux/m68k/syscalls.list b/sysdeps/unix/sysv/linux/m68k/syscalls.list
index 55a377b841..529b548ce5 100644
--- a/sysdeps/unix/sysv/linux/m68k/syscalls.list
+++ b/sysdeps/unix/sysv/linux/m68k/syscalls.list
@@ -4,10 +4,10 @@ chown		-	chown32		i:sii	__chown		chown
 lchown		-	lchown32	i:sii	__lchown	lchown
 fchown		-	fchown32	i:iii	__fchown	fchown
 
-getegid		-	getegid32	Ei:	__getegid	getegid
-geteuid		-	geteuid32	Ei:	__geteuid	geteuid
-getgid		-	getgid32	Ei:	__getgid	getgid
-getuid		-	getuid32	Ei:	__getuid	getuid
+getegid		-	getegid32	Ei:	__getegid	getegid		__getegid_startup
+geteuid		-	geteuid32	Ei:	__geteuid	geteuid		__geteuid_startup
+getgid		-	getgid32	Ei:	__getgid	getgid		__getgid_startup
+getuid		-	getuid32	Ei:	__getuid	getuid		__getuid_startup
 getresgid	-	getresgid32	i:ppp	__getresgid	getresgid
 getresuid	-	getresuid32	i:ppp	__getresuid	getresuid
 getgroups	-	getgroups32	i:ip	__getgroups	getgroups
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list b/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list
index 300b13dd01..66253a20ff 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/syscalls.list
@@ -4,10 +4,10 @@ chown		-	chown32		i:sii	__chown		chown@@GLIBC_2.1
 lchown		-	lchown32	i:sii	__lchown	lchown@@GLIBC_2.0 chown@GLIBC_2.0
 fchown		-	fchown32	i:iii	__fchown	fchown
 
-getegid		-	getegid32	Ei:	__getegid	getegid
-geteuid		-	geteuid32	Ei:	__geteuid	geteuid
-getgid		-	getgid32	Ei:	__getgid	getgid
-getuid		-	getuid32	Ei:	__getuid	getuid
+getegid		-	getegid32	Ei:	__getegid	getegid		__getegid_startup
+geteuid		-	geteuid32	Ei:	__geteuid	geteuid		__geteuid_startup
+getgid		-	getgid32	Ei:	__getgid	getgid		__getgid_startup
+getuid		-	getuid32	Ei:	__getuid	getuid		__getuid_startup
 getresgid	-	getresgid32	i:ppp	__getresgid	getresgid
 getresuid	-	getresuid32	i:ppp	__getresuid	getresuid
 getgroups	-	getgroups32	i:ip	__getgroups	getgroups
diff --git a/sysdeps/unix/sysv/linux/sh/syscalls.list b/sysdeps/unix/sysv/linux/sh/syscalls.list
index 32badd1ee0..603f324f60 100644
--- a/sysdeps/unix/sysv/linux/sh/syscalls.list
+++ b/sysdeps/unix/sysv/linux/sh/syscalls.list
@@ -4,10 +4,10 @@ chown		-	chown32		i:sii	__chown		chown
 lchown		-	lchown32	i:sii	__lchown	lchown
 fchown		-	fchown32	i:iii	__fchown	fchown
 
-getegid		-	getegid32	Ei:	__getegid	getegid
-geteuid		-	geteuid32	Ei:	__geteuid	geteuid
-getgid		-	getgid32	Ei:	__getgid	getgid
-getuid		-	getuid32	Ei:	__getuid	getuid
+getegid		-	getegid32	Ei:	__getegid	getegid		__getegid_startup
+geteuid		-	geteuid32	Ei:	__geteuid	geteuid		__geteuid_startup
+getgid		-	getgid32	Ei:	__getgid	getgid		__getgid_startup
+getuid		-	getuid32	Ei:	__getuid	getuid		__getuid_startup
 getresgid	-	getresgid32	i:ppp	__getresgid	getresgid
 getresuid	-	getresuid32	i:ppp	__getresuid	getresuid
 getgroups	-	getgroups32	i:ip	__getgroups	getgroups
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list b/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list
index 0b6095ffab..672e7f7254 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/syscalls.list
@@ -4,10 +4,10 @@ chown		-	chown32		i:sii	__chown		chown
 lchown		-	lchown32	i:sii	__lchown	lchown
 fchown		-	fchown32	i:iii	__fchown	fchown
 
-getegid		-	getegid32	Ei:	__getegid	getegid
-geteuid		-	geteuid32	Ei:	__geteuid	geteuid
-getgid		-	getgid32	Ei:	__getgid	getgid
-getuid		-	getuid32	Ei:	__getuid	getuid
+getegid		-	getegid32	Ei:	__getegid	getegid		__getegid_startup
+geteuid		-	geteuid32	Ei:	__geteuid	geteuid		__geteuid_startup
+getgid		-	getgid32	Ei:	__getgid	getgid		__getgid_startup
+getuid		-	getuid32	Ei:	__getuid	getuid		__getuid_startup
 getresuid	-	getresuid32	3	getresuid
 getresgid	-	getresgid32	3	getresgid
 getgroups	-	getgroups32	i:ip	__getgroups	getgroups
diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
index 01ec2bfa95..b1b8940772 100644
--- a/sysdeps/unix/sysv/linux/syscalls.list
+++ b/sysdeps/unix/sysv/linux/syscalls.list
@@ -14,8 +14,8 @@ execve		-	execve		i:spp	__execve	execve
 flock		-	flock		i:ii	__flock		flock
 get_kernel_syms	EXTRA	get_kernel_syms	i:p	__compat_get_kernel_syms	get_kernel_syms@GLIBC_2.0:GLIBC_2.23
 getpid          -       getpid          Ei:     __getpid        getpid
-getegid		-	getegid		Ei:	__getegid	getegid
-geteuid		-	geteuid		Ei:	__geteuid	geteuid
+getegid		-	getegid		Ei:	__getegid	getegid	__getegid_startup
+geteuid		-	geteuid		Ei:	__geteuid	geteuid	__geteuid_startup
 getpgid		-	getpgid		i:i	__getpgid	getpgid
 getpgrp		-	getpgrp		Ei:	getpgrp
 getppid		-	getppid		Ei:	__getppid	getppid
-- 
2.29.2


[-- Attachment #5: 0004-x86-Check-ifunc-resolver-with-CPU_FEATURE_USABLE-BZ-.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From e289c42f8b40b76bb029779a6cb73c739f1f9c72 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 12 Jan 2021 14:41:10 -0800
Subject: [PATCH 4/4] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ
 #27072]

Check ifunc resolver with CPU_FEATURE_USABLE in dynamic and static
executables to verify that CPUID features are initialized early in
static PIE.
---
 sysdeps/x86/Makefile                 |   7 ++
 sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-1.c        | 115 +++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index adaa2a92cd..996ff46dbc 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -9,6 +9,13 @@ sysdep_headers += sys/platform/x86.h
 tests += tst-get-cpu-features tst-get-cpu-features-static \
 	 tst-cpu-features-cpuinfo tst-cpu-features-supports
 tests-static += tst-get-cpu-features-static
+ifeq (yes,$(have-ifunc))
+tests += \
+  tst-ifunc-isa-1 \
+  tst-ifunc-isa-1-static
+tests-static += \
+  tst-ifunc-isa-1-static
+endif
 ifeq (yes,$(enable-x86-isa-level))
 tests += tst-isa-level-1
 modules-names += tst-isa-level-mod-1-baseline \
diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
new file mode 100644
index 0000000000..0e94f6119b
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-1.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
new file mode 100644
index 0000000000..b3bc2a55a2
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1.c
@@ -0,0 +1,115 @@
+/* Check ifunc with CPU_FEATURE_USABLE.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+
+enum isa
+{
+  none,
+  sse2,
+  sse4_2,
+  avx,
+  avx2,
+  avx512f
+};
+
+enum isa
+get_isa (void)
+{
+  if (CPU_FEATURE_USABLE (AVX512F))
+    return avx512f;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return avx;
+  if (CPU_FEATURE_USABLE (SSE4_2))
+    return sse4_2;
+  if (CPU_FEATURE_USABLE (SSE2))
+    return sse2;
+  return none;
+}
+
+static int
+isa_sse2 (void)
+{
+  return sse2;
+}
+
+static int
+isa_sse4_2 (void)
+{
+  return sse4_2;
+}
+
+static int
+isa_avx (void)
+{
+  return avx;
+}
+
+static int
+isa_avx2 (void)
+{
+  return avx2;
+}
+
+static int
+isa_avx512f (void)
+{
+  return avx512f;
+}
+
+static int
+isa_none (void)
+{
+  return none;
+}
+
+int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
+
+void *
+foo_ifunc (void)
+{
+  switch (get_isa ())
+    {
+    case avx512f:
+      return isa_avx512f;
+    case avx2:
+      return isa_avx2;
+    case avx:
+      return isa_avx;
+    case sse4_2:
+      return isa_sse4_2;
+    case sse2:
+      return isa_sse2;
+    default:
+      break;
+    }
+  return isa_none;
+}
+
+static int
+do_test (void)
+{
+  enum isa value = foo ();
+  enum isa expected = get_isa ();
+  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
-- 
2.29.2

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Thu, Jan 14, 2021 at 7:36 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > > The 01/12/2021 17:19, H.J. Lu wrote:
> > > > On Tue, Jan 12, 2021 at 4:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > On Tue, Jan 12, 2021 at 4:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > > > > > See:
> > > > > >
> > > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=14961
> > > > > >
> > > > >  /* Mark all symbols hidden in static PIE libc to avoid GOT indirections.  */
> > > > > -#if BUILD_PIE_DEFAULT && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > > +#if BUILD_PIE_DEFAULT && !defined NO_HIDDEN_EXTERN_FUNC_IN_PIE \
> > > > > +    && IS_IN (libc) && !defined LIBC_NONSHARED
> > > > >  # pragma GCC visibility push(hidden)
> > > > >  #endif
> > > > >
> > > >
> > > > This works on i686.
> >
> > The series i plan to commit today is in nsz/bug27072 now,
> >
> > This is the v4 of this patch:
> >
> > Hidden matters with static PIE: extern symbol access in position
> > independent code usually involves GOT indirections which needs
> > RELATIVE relocs in a static linked PIE. Hidden visibility avoids
> > indirections and RELATIVE relocs on targets that can access symbols
> > pc-relative.
> >
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > static libraries can use hidden visibility to avoid indirections,
> > however the test system links objects from libcrypt.a into dynamic
> > linked test binaries so hidden does not work there.  I think mixing
> > static and shared libc components in the same binary should not be
> > supported usage, but to be safe only use hidden in libc.a.
> >
> > There are targets (i686) where hidden visibility functions are
> > problematic in PIE code so hidden cannot be applied to all symbols.
> > Then static PIE requires extern object access without relocations
> > (e.g. by relying on copy relocations in shared libraries instead of
> > GOT access in PIE code). See bug 14961.
>
> It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> additional patches to make static PIE to work on i386 and x86-64.
> I am enclosing my patches.  Please include them in your patch set.
>
> > From -static-pie linked 'int main(){}' this shaves off 73 relative
> > relocs on aarch64 and reduces code size too.
>
>
> --
> H.J.

commit c5ffa46591550d945b009f0e3bcf66603d48ac0b
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Jan 14 13:26:29 2021 -0800

    i386: Call _dl_aux_init after relocating static PIE

is too complicated.  I will submit a simple version.

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

The 01/14/2021 19:36, H.J. Lu wrote:
> On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > There are targets (i686) where hidden visibility functions are
> > problematic in PIE code so hidden cannot be applied to all symbols.
> > Then static PIE requires extern object access without relocations
> > (e.g. by relying on copy relocations in shared libraries instead of
> > GOT access in PIE code). See bug 14961.
> 
> It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> additional patches to make static PIE to work on i386 and x86-64.
> I am enclosing my patches.  Please include them in your patch set.

it is about avoiding GOT for object access, which copy
relocations can do. hidden does it better, but you said
that does not work on i686 so i removed it (assuming
you know the implications: every pie object access must
be local and copy relocated in shared libraries)

morally all symbols should be hidden in static pie
because everything is local (the code is only linked
into static exectuables). this is useful outside the
start code too to avoid GOT indirections.

i686 does not want to set up EBX for hidden extern calls,
which is needed for ifuncs, so making everything hidden
does not work.

options:

(1) fix extern hidden pie calls on i686 (by making
    them the same as default vis pie calls so ifuncs
    work).

(2) annotate ifuncs (avoid hidden for them, ifuncs
    cannot appear in early start code anyway because
    of IRELATIVE): this can be difficult to maintain.

(3) annotate early object accesses to be hidden so
    RELATIVE relocs are avoided. (most targets want
    all objects to be hidden, but this solves bug
    27072 without causing problems on i686)

(4) make pie always use copy relocations on i686.
    (and then no hidden annotation is needed, object
    access is always local in pie).

my patches assumed (4), but that seems to not work so
i think doing (3) is reasonable: you either need a few
carefully placed 'pragma GCC visibility push(hidden)'
or an attribute_hidden_pie_data on object declarations
that may be used by the early start code.

> From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 18:39:24 -0800
> Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras
> 
> Add extra-test-objs to test-extras so that they are compiled with
> -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.

this makes sense.

> From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 16:40:43 -0800
> Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden
> 
> On i386, all calls to IFUNC functions must go through PLT and calls to
> hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
> may not be set up for local calls to hidden IFUNC functions.
> 
> Even if we can't make all libc symbols hidden for static PIE on i386, we
> must make all symbols used by _dl_relocate_static_pie hidden.
> ---
>  elf/dl-reloc-static-pie.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> index a8d964061e..cc34c2d2fe 100644
> --- a/elf/dl-reloc-static-pie.c
> +++ b/elf/dl-reloc-static-pie.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #if ENABLE_STATIC_PIE
> +#pragma GCC visibility push(hidden)

yes, this is option (3). you will also need it in _dl_aux_init
and __libc_init_secure and __tunables_init.

> From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Thu, 14 Jan 2021 13:26:29 -0800
> Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE
> 
> In i386 static PIE, we need to call _dl_aux_init after relocating static
> PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in
> sync.  Also in i386 static PIE, since __libc_init_secure is called before
> ARCH_SETUP_TLS, it must use "int $0x80" for system calls.  Update
> __libc_init_secure to use __geteuid_startup, __getuid_startup,
> __getegid_startup and __getgid_startup.

the syscall part i understand, but auxv vs tls i don't:

i thought you only need to ensure that objects are hidden
visibility in _dl_aux_init.

i think the dependency order is:

1 auxv
2 libc_secure
3 tunables
4 cpu features
5 self reloc
6 setup _dl_phdr from __ehdr_start
7 setup tls

i got 6 wrong in my patch: setup tls can use _dl_phdr,
i will fix it.

moving auxv a bit later is possible (if you don't mind
syscalls in libc_secure and nothing requires it in
cpu features), but i don't see how that's related to
tls.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/14/2021 19:36, H.J. Lu wrote:
> > On Thu, Jan 14, 2021 at 3:18 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > The 01/13/2021 09:50, Szabolcs Nagy via Libc-alpha wrote:
> > > There are targets (i686) where hidden visibility functions are
> > > problematic in PIE code so hidden cannot be applied to all symbols.
> > > Then static PIE requires extern object access without relocations
> > > (e.g. by relying on copy relocations in shared libraries instead of
> > > GOT access in PIE code). See bug 14961.
> >
> > It isn't about copy relocations.  It is IFUNC, PLT and PIE.   I needed
> > additional patches to make static PIE to work on i386 and x86-64.
> > I am enclosing my patches.  Please include them in your patch set.
>
> it is about avoiding GOT for object access, which copy
> relocations can do. hidden does it better, but you said
> that does not work on i686 so i removed it (assuming
> you know the implications: every pie object access must
> be local and copy relocated in shared libraries)
>
> morally all symbols should be hidden in static pie
> because everything is local (the code is only linked
> into static exectuables). this is useful outside the
> start code too to avoid GOT indirections.
>
> i686 does not want to set up EBX for hidden extern calls,
> which is needed for ifuncs, so making everything hidden
> does not work.
>
> options:
>
> (1) fix extern hidden pie calls on i686 (by making
>     them the same as default vis pie calls so ifuncs
>     work).
>
> (2) annotate ifuncs (avoid hidden for them, ifuncs
>     cannot appear in early start code anyway because
>     of IRELATIVE): this can be difficult to maintain.
>
> (3) annotate early object accesses to be hidden so
>     RELATIVE relocs are avoided. (most targets want
>     all objects to be hidden, but this solves bug
>     27072 without causing problems on i686)
>
> (4) make pie always use copy relocations on i686.
>     (and then no hidden annotation is needed, object
>     access is always local in pie).

Linker doesn't generate copy relocations for static PIE.
The problem is GOT indirections which require RELATIVE
relocations.

> my patches assumed (4), but that seems to not work so
> i think doing (3) is reasonable: you either need a few
> carefully placed 'pragma GCC visibility push(hidden)'
> or an attribute_hidden_pie_data on object declarations
> that may be used by the early start code.
>
> > From 15488890220a8c580689e6f78a38847853b78850 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 18:39:24 -0800
> > Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras
> >
> > Add extra-test-objs to test-extras so that they are compiled with
> > -DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.
>
> this makes sense.
>
> > From e1e10cd6bd52d9061f138f49b35d4939e1cd5692 Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 16:40:43 -0800
> > Subject: [PATCH 2/4] Make all symbols used by _dl_relocate_static_pie hidden
> >
> > On i386, all calls to IFUNC functions must go through PLT and calls to
> > hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
> > may not be set up for local calls to hidden IFUNC functions.
> >
> > Even if we can't make all libc symbols hidden for static PIE on i386, we
> > must make all symbols used by _dl_relocate_static_pie hidden.
> > ---
> >  elf/dl-reloc-static-pie.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> > index a8d964061e..cc34c2d2fe 100644
> > --- a/elf/dl-reloc-static-pie.c
> > +++ b/elf/dl-reloc-static-pie.c
> > @@ -17,6 +17,7 @@
> >     <https://www.gnu.org/licenses/>.  */
> >
> >  #if ENABLE_STATIC_PIE
> > +#pragma GCC visibility push(hidden)
>
> yes, this is option (3). you will also need it in _dl_aux_init
> and __libc_init_secure and __tunables_init.

I will try it.

> > From c5ffa46591550d945b009f0e3bcf66603d48ac0b Mon Sep 17 00:00:00 2001
> > From: "H.J. Lu" <hjl.tools@gmail.com>
> > Date: Thu, 14 Jan 2021 13:26:29 -0800
> > Subject: [PATCH 3/4] i386: Call _dl_aux_init after relocating static PIE
> >
> > In i386 static PIE, we need to call _dl_aux_init after relocating static
> > PIE so that symbol addresses in _dl_aux_init and ARCH_SETUP_TLS are in
> > sync.  Also in i386 static PIE, since __libc_init_secure is called before
> > ARCH_SETUP_TLS, it must use "int $0x80" for system calls.  Update
> > __libc_init_secure to use __geteuid_startup, __getuid_startup,
> > __getegid_startup and __getgid_startup.
>
> the syscall part i understand, but auxv vs tls i don't:
>
> i thought you only need to ensure that objects are hidden
> visibility in _dl_aux_init.
>
> i think the dependency order is:
>
> 1 auxv
> 2 libc_secure
> 3 tunables
> 4 cpu features
> 5 self reloc
> 6 setup _dl_phdr from __ehdr_start
> 7 setup tls
>
> i got 6 wrong in my patch: setup tls can use _dl_phdr,
> i will fix it.

On i386, setup tls uses auxv.  But there are GOT
indirections in _dl_aux_init () which require RELATIVE
relocations.

> moving auxv a bit later is possible (if you don't mind
> syscalls in libc_secure and nothing requires it in
> cpu features), but i don't see how that's related to
> tls.


-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

The 01/15/2021 05:43, H.J. Lu wrote:
> On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > options:
> >
> > (1) fix extern hidden pie calls on i686 (by making
> >     them the same as default vis pie calls so ifuncs
> >     work).
> >
> > (2) annotate ifuncs (avoid hidden for them, ifuncs
> >     cannot appear in early start code anyway because
> >     of IRELATIVE): this can be difficult to maintain.
> >
> > (3) annotate early object accesses to be hidden so
> >     RELATIVE relocs are avoided. (most targets want
> >     all objects to be hidden, but this solves bug
> >     27072 without causing problems on i686)
> >
> > (4) make pie always use copy relocations on i686.
> >     (and then no hidden annotation is needed, object
> >     access is always local in pie).
...
> > > --- a/elf/dl-reloc-static-pie.c
> > > +++ b/elf/dl-reloc-static-pie.c
> > > @@ -17,6 +17,7 @@
> > >     <https://www.gnu.org/licenses/>.  */
> > >
> > >  #if ENABLE_STATIC_PIE
> > > +#pragma GCC visibility push(hidden)
> >
> > yes, this is option (3). you will also need it in _dl_aux_init
> > and __libc_init_secure and __tunables_init.
> 
> I will try it.

the naive way does not seem to work:

_dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
the former needs hidden the latter does not and calls rawmemchr
which is ifunc on i686.

i think the easiest fix is to move those two functions into
separate files. (ideally we would have a small set of files
that are involved in the start code before self relocation)

now i realized that there is another option:

(5) remove all ifuncs from i686 libc.

i assume there are not many users who care about i686 performance.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/15/2021 05:43, H.J. Lu wrote:
> > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > options:
> > >
> > > (1) fix extern hidden pie calls on i686 (by making
> > >     them the same as default vis pie calls so ifuncs
> > >     work).
> > >
> > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > >     cannot appear in early start code anyway because
> > >     of IRELATIVE): this can be difficult to maintain.
> > >
> > > (3) annotate early object accesses to be hidden so
> > >     RELATIVE relocs are avoided. (most targets want
> > >     all objects to be hidden, but this solves bug
> > >     27072 without causing problems on i686)
> > >
> > > (4) make pie always use copy relocations on i686.
> > >     (and then no hidden annotation is needed, object
> > >     access is always local in pie).
> ...
> > > > --- a/elf/dl-reloc-static-pie.c
> > > > +++ b/elf/dl-reloc-static-pie.c
> > > > @@ -17,6 +17,7 @@
> > > >     <https://www.gnu.org/licenses/>.  */
> > > >
> > > >  #if ENABLE_STATIC_PIE
> > > > +#pragma GCC visibility push(hidden)
> > >
> > > yes, this is option (3). you will also need it in _dl_aux_init
> > > and __libc_init_secure and __tunables_init.
> >
> > I will try it.
>
> the naive way does not seem to work:
>
> _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> the former needs hidden the latter does not and calls rawmemchr
> which is ifunc on i686.
>
> i think the easiest fix is to move those two functions into
> separate files. (ideally we would have a small set of files
> that are involved in the start code before self relocation)
>
> now i realized that there is another option:
>
> (5) remove all ifuncs from i686 libc.
>
> i assume there are not many users who care about i686 performance.

I don't know if this will work on i686.  Since i386 doesn't have IP relative
addressing, we can't remove all RELATIVE relocations.  We need to
call _dl_aux_init again after relocating PIE.  I don't know what other symbols
are affected.  My current patches are on users/hjl/pr27072/master branch:

https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/15/2021 05:43, H.J. Lu wrote:
> > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > options:
> > > >
> > > > (1) fix extern hidden pie calls on i686 (by making
> > > >     them the same as default vis pie calls so ifuncs
> > > >     work).
> > > >
> > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > >     cannot appear in early start code anyway because
> > > >     of IRELATIVE): this can be difficult to maintain.
> > > >
> > > > (3) annotate early object accesses to be hidden so
> > > >     RELATIVE relocs are avoided. (most targets want
> > > >     all objects to be hidden, but this solves bug
> > > >     27072 without causing problems on i686)
> > > >
> > > > (4) make pie always use copy relocations on i686.
> > > >     (and then no hidden annotation is needed, object
> > > >     access is always local in pie).
> > ...
> > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > @@ -17,6 +17,7 @@
> > > > >     <https://www.gnu.org/licenses/>.  */
> > > > >
> > > > >  #if ENABLE_STATIC_PIE
> > > > > +#pragma GCC visibility push(hidden)
> > > >
> > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > and __libc_init_secure and __tunables_init.
> > >
> > > I will try it.
> >
> > the naive way does not seem to work:
> >
> > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > the former needs hidden the latter does not and calls rawmemchr
> > which is ifunc on i686.
> >
> > i think the easiest fix is to move those two functions into
> > separate files. (ideally we would have a small set of files
> > that are involved in the start code before self relocation)
> >
> > now i realized that there is another option:
> >
> > (5) remove all ifuncs from i686 libc.
> >
> > i assume there are not many users who care about i686 performance.
>
> I don't know if this will work on i686.  Since i386 doesn't have IP relative
> addressing, we can't remove all RELATIVE relocations.  We need to
> call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> are affected.  My current patches are on users/hjl/pr27072/master branch:
>
> https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
>

The problem is

#ifdef NEED_DL_SYSINFO
/* Needed for improved syscall handling on at least x86/Linux.  */
uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
#endif

We can initialize it in _dl_aux_init instead.

I am testing this set of patches on top of yours on i686 and x86-64.

-- 
H.J.

[-- Attachment #2: 0004-x86-Check-ifunc-resolver-with-CPU_FEATURE_USABLE-BZ-.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From 5f7eeb0aa5251849ac516e027ea5ec8dfeec5727 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 12 Jan 2021 14:41:10 -0800
Subject: [PATCH 4/4] x86: Check ifunc resolver with CPU_FEATURE_USABLE [BZ
 #27072]

Check ifunc resolver with CPU_FEATURE_USABLE in dynamic and static
executables to verify that CPUID features are initialized early in
static PIE.
---
 sysdeps/x86/Makefile                 |   7 ++
 sysdeps/x86/tst-ifunc-isa-1-static.c |   1 +
 sysdeps/x86/tst-ifunc-isa-1.c        | 115 +++++++++++++++++++++++++++
 3 files changed, 123 insertions(+)
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1-static.c
 create mode 100644 sysdeps/x86/tst-ifunc-isa-1.c

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index adaa2a92cd..996ff46dbc 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -9,6 +9,13 @@ sysdep_headers += sys/platform/x86.h
 tests += tst-get-cpu-features tst-get-cpu-features-static \
 	 tst-cpu-features-cpuinfo tst-cpu-features-supports
 tests-static += tst-get-cpu-features-static
+ifeq (yes,$(have-ifunc))
+tests += \
+  tst-ifunc-isa-1 \
+  tst-ifunc-isa-1-static
+tests-static += \
+  tst-ifunc-isa-1-static
+endif
 ifeq (yes,$(enable-x86-isa-level))
 tests += tst-isa-level-1
 modules-names += tst-isa-level-mod-1-baseline \
diff --git a/sysdeps/x86/tst-ifunc-isa-1-static.c b/sysdeps/x86/tst-ifunc-isa-1-static.c
new file mode 100644
index 0000000000..0e94f6119b
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1-static.c
@@ -0,0 +1 @@
+#include "tst-ifunc-isa-1.c"
diff --git a/sysdeps/x86/tst-ifunc-isa-1.c b/sysdeps/x86/tst-ifunc-isa-1.c
new file mode 100644
index 0000000000..b3bc2a55a2
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc-isa-1.c
@@ -0,0 +1,115 @@
+/* Check ifunc with CPU_FEATURE_USABLE.
+   Copyright (C) 2021 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+#include <sys/platform/x86.h>
+
+enum isa
+{
+  none,
+  sse2,
+  sse4_2,
+  avx,
+  avx2,
+  avx512f
+};
+
+enum isa
+get_isa (void)
+{
+  if (CPU_FEATURE_USABLE (AVX512F))
+    return avx512f;
+  if (CPU_FEATURE_USABLE (AVX2))
+    return avx2;
+  if (CPU_FEATURE_USABLE (AVX))
+    return avx;
+  if (CPU_FEATURE_USABLE (SSE4_2))
+    return sse4_2;
+  if (CPU_FEATURE_USABLE (SSE2))
+    return sse2;
+  return none;
+}
+
+static int
+isa_sse2 (void)
+{
+  return sse2;
+}
+
+static int
+isa_sse4_2 (void)
+{
+  return sse4_2;
+}
+
+static int
+isa_avx (void)
+{
+  return avx;
+}
+
+static int
+isa_avx2 (void)
+{
+  return avx2;
+}
+
+static int
+isa_avx512f (void)
+{
+  return avx512f;
+}
+
+static int
+isa_none (void)
+{
+  return none;
+}
+
+int foo (void) __attribute__ ((ifunc ("foo_ifunc")));
+
+void *
+foo_ifunc (void)
+{
+  switch (get_isa ())
+    {
+    case avx512f:
+      return isa_avx512f;
+    case avx2:
+      return isa_avx2;
+    case avx:
+      return isa_avx;
+    case sse4_2:
+      return isa_sse4_2;
+    case sse2:
+      return isa_sse2;
+    default:
+      break;
+    }
+  return isa_none;
+}
+
+static int
+do_test (void)
+{
+  enum isa value = foo ();
+  enum isa expected = get_isa ();
+  return value == expected ? EXIT_SUCCESS : EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
-- 
2.29.2


[-- Attachment #3: 0003-Use-startup.h-in-__libc_init_secure.patch --]
[-- Type: text/x-patch, Size: 3377 bytes --]

From 24676232cd5485fc8b4d9707eb81ce0ba7e65f4a Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 15 Jan 2021 06:46:12 -0800
Subject: [PATCH 3/4] Use <startup.h> in __libc_init_secure

Since __libc_init_secure is called before ARCH_SETUP_TLS, it must use
"int $0x80" for system calls in i386 static PIE.  Add startup_getuid,
startup_geteuid, startup_getgid and startup_getegid to <startup.h>.
Update __libc_init_secure to use them.
---
 elf/enbl-secure.c                      |  6 +++---
 sysdeps/generic/startup.h              | 26 +++++++++++++++++++++++
 sysdeps/unix/sysv/linux/i386/startup.h | 29 ++++++++++++++++++++++++--
 3 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
index dba39a123e..3b1dd9795f 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -22,7 +22,7 @@
 #if BUILD_PIE_DEFAULT
 # pragma GCC visibility push(hidden)
 #endif
-#include <unistd.h>
+#include <startup.h>
 #include <libc-internal.h>
 
 /* If nonzero __libc_enable_secure is already set.  */
@@ -34,6 +34,6 @@ void
 __libc_init_secure (void)
 {
   if (__libc_enable_secure_decided == 0)
-    __libc_enable_secure = (__geteuid () != __getuid ()
-			    || __getegid () != __getgid ());
+    __libc_enable_secure = (startup_geteuid () != startup_getuid ()
+			    || startup_getegid () != startup_getgid ());
 }
diff --git a/sysdeps/generic/startup.h b/sysdeps/generic/startup.h
index 56c899a65e..04f20cde47 100644
--- a/sysdeps/generic/startup.h
+++ b/sysdeps/generic/startup.h
@@ -19,5 +19,31 @@
 /* Targets should override this file if the default definitions below
    will not work correctly very early before TLS is initialized.  */
 
+#include <unistd.h>
+
 /* Use macro instead of inline function to avoid including <stdio.h>.  */
 #define _startup_fatal(message) __libc_fatal ((message))
+
+static inline uid_t
+startup_getuid (void)
+{
+  return __getuid ();
+}
+
+static inline uid_t
+startup_geteuid (void)
+{
+  return __geteuid ();
+}
+
+static inline gid_t
+startup_getgid (void)
+{
+  return __getgid ();
+}
+
+static inline gid_t
+startup_getegid (void)
+{
+  return __getegid ();
+}
diff --git a/sysdeps/unix/sysv/linux/i386/startup.h b/sysdeps/unix/sysv/linux/i386/startup.h
index 3eb4cc43a2..dee7a4f1d3 100644
--- a/sysdeps/unix/sysv/linux/i386/startup.h
+++ b/sysdeps/unix/sysv/linux/i386/startup.h
@@ -17,11 +17,12 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if BUILD_PIE_DEFAULT
-# include <abort-instr.h>
-
 /* Can't use "call *%gs:SYSINFO_OFFSET" during statup in static PIE.  */
 # define I386_USE_SYSENTER 0
 
+# include <sysdep.h>
+# include <abort-instr.h>
+
 __attribute__ ((__noreturn__))
 static inline void
 _startup_fatal (const char *message __attribute__ ((unused)))
@@ -31,6 +32,30 @@ _startup_fatal (const char *message __attribute__ ((unused)))
   ABORT_INSTRUCTION;
   __builtin_unreachable ();
 }
+
+static inline uid_t
+startup_getuid (void)
+{
+  return (uid_t) INTERNAL_SYSCALL_CALL (getuid32);
+}
+
+static inline uid_t
+startup_geteuid (void)
+{
+  return (uid_t) INTERNAL_SYSCALL_CALL (geteuid32);
+}
+
+static inline gid_t
+startup_getgid (void)
+{
+  return (gid_t) INTERNAL_SYSCALL_CALL (getgid32);
+}
+
+static inline gid_t
+startup_getegid (void)
+{
+  return (gid_t) INTERNAL_SYSCALL_CALL (getegid32);
+}
 #else
 # include_next <startup.h>
 #endif
-- 
2.29.2


[-- Attachment #4: 0001-libmvec-Add-extra-test-objs-to-test-extras.patch --]
[-- Type: text/x-patch, Size: 1695 bytes --]

From aa3d762e41610bbf5c016a219021f055b8b46337 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Jan 2021 18:39:24 -0800
Subject: [PATCH 1/4] libmvec: Add extra-test-objs to test-extras

Add extra-test-objs to test-extras so that they are compiled with
-DMODULE_NAME=testsuite instead of -DMODULE_NAME=libc.
---
 sysdeps/x86_64/fpu/Makefile | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
index a4ff2723a8..9a4bdd075c 100644
--- a/sysdeps/x86_64/fpu/Makefile
+++ b/sysdeps/x86_64/fpu/Makefile
@@ -31,6 +31,12 @@ libmvec-tests += double-vlen2 double-vlen4 double-vlen4-avx2 \
 tests += test-double-libmvec-sincos test-double-libmvec-sincos-avx \
 	 test-double-libmvec-sincos-avx2 test-float-libmvec-sincosf \
 	 test-float-libmvec-sincosf-avx test-float-libmvec-sincosf-avx2
+test-extras += test-double-libmvec-sincos-avx-main \
+		   test-double-libmvec-sincos-avx2-main \
+		   test-double-libmvec-sincos-main \
+		   test-float-libmvec-sincosf-avx-main \
+		   test-float-libmvec-sincosf-avx2-main \
+		   test-float-libmvec-sincosf-main
 extra-test-objs += test-double-libmvec-sincos-avx-main.o \
 		   test-double-libmvec-sincos-avx2-main.o \
 		   test-double-libmvec-sincos-main.o \
@@ -66,6 +72,8 @@ ifeq (yes,$(config-cflags-avx512))
 libmvec-tests += double-vlen8 float-vlen16
 tests += test-double-libmvec-sincos-avx512 \
 	 test-float-libmvec-sincosf-avx512
+test-extras += test-double-libmvec-sincos-avx512-main \
+	       test-float-libmvec-sincosf-avx512-main
 extra-test-objs += test-double-libmvec-sincos-avx512-main.o \
 		   test-float-libmvec-sincosf-avx512-main.o
 
-- 
2.29.2


[-- Attachment #5: 0002-Make-all-symbols-used-before-and-by-_dl_relocate_sta.patch --]
[-- Type: text/x-patch, Size: 3312 bytes --]

From 52dd650d7b231fe20634afe3e261e67f33b66869 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 14 Jan 2021 16:40:43 -0800
Subject: [PATCH 2/4] Make all symbols used before and by
 _dl_relocate_static_pie hidden

On i386, all calls to IFUNC functions must go through PLT and calls to
hidden functions CANNOT go through PLT in PIE since EBX used in PIE PLT
may not be set up for local calls to hidden IFUNC functions.

Even if we can't make all libc symbols hidden for static PIE on i386, we
must make all symbols used before and by _dl_relocate_static_pie hidden.

In static PIE, set the default _dl_sysinfo in _dl_aux_init, instead of
using the RELATIVE relocation to intialize it.
---
 elf/dl-reloc-static-pie.c |  1 +
 elf/dl-support.c          | 16 +++++++++++++++-
 elf/dl-tunables.c         |  3 +++
 elf/enbl-secure.c         |  3 +++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index a8d964061e..cc34c2d2fe 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if ENABLE_STATIC_PIE
+#pragma GCC visibility push(hidden)
 #include <unistd.h>
 #include <ldsodefs.h>
 #include "dynamic-link.h"
diff --git a/elf/dl-support.c b/elf/dl-support.c
index 9d468d5a4b..4fbf33c982 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -19,6 +19,10 @@
 /* This file defines some things that for the dynamic linker are defined in
    rtld.c and dl-sysdep.c in ways appropriate to bootstrap dynamic linking.  */
 
+#include <string.h>
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <errno.h>
 #include <libintl.h>
 #include <stdlib.h>
@@ -194,7 +198,12 @@ struct dl_scope_free_list *_dl_scope_free_list;
 
 #ifdef NEED_DL_SYSINFO
 /* Needed for improved syscall handling on at least x86/Linux.  */
-uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
+uintptr_t _dl_sysinfo
+/* NB: Avoid RELATIVE relocation in static PIE.  */
+# ifndef BUILD_PIE_DEFAULT
+  = DL_SYSINFO_DEFAULT
+# endif
+;
 #endif
 #ifdef NEED_DL_SYSINFO_DSO
 /* Address of the ELF headers in the vsyscall page.  */
@@ -232,6 +241,11 @@ _dl_aux_init (ElfW(auxv_t) *av)
   uid_t uid = 0;
   gid_t gid = 0;
 
+#if defined NEED_DL_SYSINFO && BUILD_PIE_DEFAULT
+  /* NB: Avoid RELATIVE relocation in static PIE.  */
+  GL(dl_sysinfo) = DL_SYSINFO_DEFAULT;
+#endif
+
   _dl_auxv = av;
   for (; av->a_type != AT_NULL; ++av)
     switch (av->a_type)
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index e44476f204..060956902d 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -18,6 +18,9 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <startup.h>
 #include <stdint.h>
 #include <stdbool.h>
diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
index bc8c5e96d2..dba39a123e 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -19,6 +19,9 @@
 /* This file is used in the static libc.  For the shared library,
    dl-sysdep.c defines and initializes __libc_enable_secure.  */
 
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <unistd.h>
 #include <libc-internal.h>
 
-- 
2.29.2

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > >
> > > The 01/15/2021 05:43, H.J. Lu wrote:
> > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > options:
> > > > >
> > > > > (1) fix extern hidden pie calls on i686 (by making
> > > > >     them the same as default vis pie calls so ifuncs
> > > > >     work).
> > > > >
> > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > > >     cannot appear in early start code anyway because
> > > > >     of IRELATIVE): this can be difficult to maintain.
> > > > >
> > > > > (3) annotate early object accesses to be hidden so
> > > > >     RELATIVE relocs are avoided. (most targets want
> > > > >     all objects to be hidden, but this solves bug
> > > > >     27072 without causing problems on i686)
> > > > >
> > > > > (4) make pie always use copy relocations on i686.
> > > > >     (and then no hidden annotation is needed, object
> > > > >     access is always local in pie).
> > > ...
> > > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > > @@ -17,6 +17,7 @@
> > > > > >     <https://www.gnu.org/licenses/>.  */
> > > > > >
> > > > > >  #if ENABLE_STATIC_PIE
> > > > > > +#pragma GCC visibility push(hidden)
> > > > >
> > > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > > and __libc_init_secure and __tunables_init.
> > > >
> > > > I will try it.
> > >
> > > the naive way does not seem to work:
> > >
> > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > > the former needs hidden the latter does not and calls rawmemchr
> > > which is ifunc on i686.
> > >
> > > i think the easiest fix is to move those two functions into
> > > separate files. (ideally we would have a small set of files
> > > that are involved in the start code before self relocation)
> > >
> > > now i realized that there is another option:
> > >
> > > (5) remove all ifuncs from i686 libc.
> > >
> > > i assume there are not many users who care about i686 performance.
> >
> > I don't know if this will work on i686.  Since i386 doesn't have IP relative
> > addressing, we can't remove all RELATIVE relocations.  We need to
> > call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> > are affected.  My current patches are on users/hjl/pr27072/master branch:
> >
> > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
> >
>
> The problem is
>
> #ifdef NEED_DL_SYSINFO
> /* Needed for improved syscall handling on at least x86/Linux.  */
> uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> #endif
>
> We can initialize it in _dl_aux_init instead.
>
> I am testing this set of patches on top of yours on i686 and x86-64.
>

They worked and they passed build-many-glibcs.py.

-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[H.J. Lu]

On Fri, Jan 15, 2021 at 4:41 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Jan 15, 2021 at 2:42 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Jan 15, 2021 at 7:28 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > On Fri, Jan 15, 2021 at 6:27 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > >
> > > > The 01/15/2021 05:43, H.J. Lu wrote:
> > > > > On Fri, Jan 15, 2021 at 3:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> > > > > > options:
> > > > > >
> > > > > > (1) fix extern hidden pie calls on i686 (by making
> > > > > >     them the same as default vis pie calls so ifuncs
> > > > > >     work).
> > > > > >
> > > > > > (2) annotate ifuncs (avoid hidden for them, ifuncs
> > > > > >     cannot appear in early start code anyway because
> > > > > >     of IRELATIVE): this can be difficult to maintain.
> > > > > >
> > > > > > (3) annotate early object accesses to be hidden so
> > > > > >     RELATIVE relocs are avoided. (most targets want
> > > > > >     all objects to be hidden, but this solves bug
> > > > > >     27072 without causing problems on i686)
> > > > > >
> > > > > > (4) make pie always use copy relocations on i686.
> > > > > >     (and then no hidden annotation is needed, object
> > > > > >     access is always local in pie).
> > > > ...
> > > > > > > --- a/elf/dl-reloc-static-pie.c
> > > > > > > +++ b/elf/dl-reloc-static-pie.c
> > > > > > > @@ -17,6 +17,7 @@
> > > > > > >     <https://www.gnu.org/licenses/>.  */
> > > > > > >
> > > > > > >  #if ENABLE_STATIC_PIE
> > > > > > > +#pragma GCC visibility push(hidden)
> > > > > >
> > > > > > yes, this is option (3). you will also need it in _dl_aux_init
> > > > > > and __libc_init_secure and __tunables_init.
> > > > >
> > > > > I will try it.
> > > >
> > > > the naive way does not seem to work:
> > > >
> > > > _dl_support.c has _dl_aux_init as well as _dl_non_dynamic_init,
> > > > the former needs hidden the latter does not and calls rawmemchr
> > > > which is ifunc on i686.
> > > >
> > > > i think the easiest fix is to move those two functions into
> > > > separate files. (ideally we would have a small set of files
> > > > that are involved in the start code before self relocation)
> > > >
> > > > now i realized that there is another option:
> > > >
> > > > (5) remove all ifuncs from i686 libc.
> > > >
> > > > i assume there are not many users who care about i686 performance.
> > >
> > > I don't know if this will work on i686.  Since i386 doesn't have IP relative
> > > addressing, we can't remove all RELATIVE relocations.  We need to
> > > call _dl_aux_init again after relocating PIE.  I don't know what other symbols
> > > are affected.  My current patches are on users/hjl/pr27072/master branch:
> > >
> > > https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master
> > >
> >
> > The problem is
> >
> > #ifdef NEED_DL_SYSINFO
> > /* Needed for improved syscall handling on at least x86/Linux.  */
> > uintptr_t _dl_sysinfo = DL_SYSINFO_DEFAULT;
> > #endif
> >
> > We can initialize it in _dl_aux_init instead.
> >
> > I am testing this set of patches on top of yours on i686 and x86-64.
> >
>
> They worked and they passed build-many-glibcs.py.
>

I combined my patches, including 4 testcases, with yours in the right
order here:

https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master


-- 
H.J.

Re: [PATCH v3 2/5] Make libc symbols hidden in static PIE

[Szabolcs Nagy]

The 01/16/2021 05:18, H.J. Lu wrote:
> I combined my patches, including 4 testcases, with yours in the right
> order here:
> 
> https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr27072/master

thanks.

i moved things around a bit: if we have fine grain marking in
files that need hidden then we can just use that instead of
making everything hidden, this needed hidden in a few more
files.

i also moved the _dl_sysinfo bit into a separate patch.
i'm sending a v4 of the series.