[PATCH v5 0/7] fix ifunc with static pie [BZ #27072]

[PATCH v5 0/7] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

v5:
- reordered patches.
- config check SUPPORT_STATIC_PIE (instead of PI_STATIC_AND_HIDDEN)
- fix __ehdr_start differently: just drop weak.
- update commit messages for hidden visibility changes.

v4:
- added patches from H.J.Lu:
	- x86: fix libmvec tests
	- x86: fix syscalls in libc_enable_secure
	- x86: avoid relative reloc for _dl_sysinfo
	- x86: add ifunc test
- i386 cannot mark all symbols hidden, so use fine grain
  marking in files that participate in early code before
  static pie self relocation.
- the patch that makes all libc symbols hidden is still
  included: it is now only an optimization for non-i386
  targets.

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.

Tested on aarch64, i686 and x86_64 with and without the last
patch (some of the make check is still running though).

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, that would simplify this significantly).
- __sbrk only needs the hidden visibility magic because of
  tunables, ideally we would not do allocations before self
  relocation.
- tunable list data structure is not optimized for compactness.

Szabolcs Nagy (7):
  elf: Make the tunable struct definition internal only
  elf: Avoid RELATIVE relocs in __tunables_init
  configure: Check for static PIE support
  csu: Avoid weak ref for __ehdr_start in static PIE
  Use hidden visibility for early static PIE code
  csu: Move static pie self relocation later [BZ #27072]
  Make libc symbols hidden in static PIE

 config.h.in                                  |  3 ++
 configure                                    | 13 ++++++
 configure.ac                                 |  4 ++
 csu/libc-start.c                             | 15 ++++++-
 elf/dl-reloc-static-pie.c                    |  2 +
 elf/dl-support.c                             |  6 +++
 elf/dl-tunable-types.h                       | 42 +++++++++++++++-----
 elf/dl-tunables.c                            |  6 ++-
 elf/dl-tunables.h                            | 35 ++++------------
 elf/enbl-secure.c                            |  4 ++
 include/libc-symbols.h                       |  9 ++++-
 misc/sbrk.c                                  |  4 ++
 scripts/gen-tunables.awk                     | 16 +++++++-
 sysdeps/aarch64/configure                    |  4 ++
 sysdeps/aarch64/configure.ac                 |  3 ++
 sysdeps/i386/configure                       |  3 ++
 sysdeps/i386/configure.ac                    |  3 ++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c |  5 +++
 sysdeps/x86/libc-start.c                     |  5 +++
 sysdeps/x86_64/configure                     |  3 ++
 sysdeps/x86_64/configure.ac                  |  3 ++
 21 files changed, 144 insertions(+), 44 deletions(-)

-- 
2.17.1

[PATCH v5 1/7] elf: Make the tunable struct definition internal only

[Szabolcs Nagy via Libc-alpha]

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.

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
diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index dfa16c1977..971376ba8d 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 v5 2/7] elf: Avoid RELATIVE relocs in __tunables_init

[Szabolcs Nagy via Libc-alpha]

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. This is bug 27181.

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.  */
 };
 
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index 33be00e447..e44476f204 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -351,7 +351,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 v5 3/7] configure: Check for static PIE support

[Szabolcs Nagy via Libc-alpha]

Add SUPPORT_STATIC_PIE that targets can define if they support
static PIE. This requires PI_STATIC_AND_HIDDEN support and various
linker features as described in

  commit 9d7a3741c9e59eba87fb3ca6b9f979befce07826
  Add --enable-static-pie configure option to build static PIE [BZ #19574]

Currently defined on x86_64, i386 and aarch64 where static PIE is
known to work.
---
 config.h.in                  |  3 +++
 configure                    | 13 +++++++++++++
 configure.ac                 |  4 ++++
 sysdeps/aarch64/configure    |  4 ++++
 sysdeps/aarch64/configure.ac |  3 +++
 sysdeps/i386/configure       |  3 +++
 sysdeps/i386/configure.ac    |  3 +++
 sysdeps/x86_64/configure     |  3 +++
 sysdeps/x86_64/configure.ac  |  3 +++
 9 files changed, 39 insertions(+)

diff --git a/config.h.in b/config.h.in
index 947feeff36..06ee8ae26a 100644
--- a/config.h.in
+++ b/config.h.in
@@ -259,6 +259,9 @@
 /* Build glibc with tunables support.  */
 #define HAVE_TUNABLES 0
 
+/* Define if static PIE is supported.  */
+#undef SUPPORT_STATIC_PIE
+
 /* Define if static PIE is enabled.  */
 #define ENABLE_STATIC_PIE 0
 
diff --git a/configure b/configure
index 49f7b32b52..1dc3af60b4 100755
--- a/configure
+++ b/configure
@@ -6814,6 +6814,19 @@ libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
 
 
 if test "$static_pie" = yes; then
+  # Check target support for static PIE
+  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+#ifndef SUPPORT_STATIC_PIE
+# error static PIE is not supported
+#endif
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+
+else
+  as_fn_error $? "the architecture does not support static PIE" "$LINENO" 5
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
   # The linker must support --no-dynamic-linker.
   if test "$libc_cv_no_dynamic_linker" != yes; then
     as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5
diff --git a/configure.ac b/configure.ac
index 341d4eeac2..dfebb8a7cc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1831,6 +1831,10 @@ libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
 AC_SUBST(libc_cv_multidir)
 
 if test "$static_pie" = yes; then
+  # Check target support for static PIE
+  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef SUPPORT_STATIC_PIE
+# error static PIE is not supported
+#endif]])], , AC_MSG_ERROR([the architecture does not support static PIE]))
   # The linker must support --no-dynamic-linker.
   if test "$libc_cv_no_dynamic_linker" != yes; then
     AC_MSG_ERROR([linker support for --no-dynamic-linker needed])
diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
index 5f5f3cc44c..83c3a23e44 100644
--- a/sysdeps/aarch64/configure
+++ b/sysdeps/aarch64/configure
@@ -6,6 +6,10 @@
 $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
 
 
+# Static PIE is supported.
+$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
+
+
 # We check to see if the compiler and flags are
 # selecting the big endian ABI and if they are then
 # we set libc_cv_aarch64_be to yes which causes
diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
index 180a16a29f..66f755078a 100644
--- a/sysdeps/aarch64/configure.ac
+++ b/sysdeps/aarch64/configure.ac
@@ -5,6 +5,9 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
 # The exception is -mcmodel=large which is unsupported with PIC/PIE.
 AC_DEFINE(PI_STATIC_AND_HIDDEN)
 
+# Static PIE is supported.
+AC_DEFINE(SUPPORT_STATIC_PIE)
+
 # We check to see if the compiler and flags are
 # selecting the big endian ABI and if they are then
 # we set libc_cv_aarch64_be to yes which causes
diff --git a/sysdeps/i386/configure b/sysdeps/i386/configure
index 90c63caf35..bb482ca16c 100644
--- a/sysdeps/i386/configure
+++ b/sysdeps/i386/configure
@@ -117,3 +117,6 @@ if test x"$multi_arch" != xno; then
   $as_echo "#define NO_HIDDEN_EXTERN_FUNC_IN_PIE 1" >>confdefs.h
 
 fi
+
+$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
+
diff --git a/sysdeps/i386/configure.ac b/sysdeps/i386/configure.ac
index 6d2068d2b3..5e43eb0adf 100644
--- a/sysdeps/i386/configure.ac
+++ b/sysdeps/i386/configure.ac
@@ -77,3 +77,6 @@ dnl via PIC PLT in PIE, which requires setting up EBX register.
 if test x"$multi_arch" != xno; then
   AC_DEFINE(NO_HIDDEN_EXTERN_FUNC_IN_PIE)
 fi
+
+dnl Static PIE is supported.
+AC_DEFINE(SUPPORT_STATIC_PIE)
diff --git a/sysdeps/x86_64/configure b/sysdeps/x86_64/configure
index 84f82c2406..198554d788 100644
--- a/sysdeps/x86_64/configure
+++ b/sysdeps/x86_64/configure
@@ -143,5 +143,8 @@ fi
 $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
 
 
+$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
+
+
 test -n "$critic_missing" && as_fn_error $? "
 *** $critic_missing" "$LINENO" 5
diff --git a/sysdeps/x86_64/configure.ac b/sysdeps/x86_64/configure.ac
index cdaba0c075..ec776274af 100644
--- a/sysdeps/x86_64/configure.ac
+++ b/sysdeps/x86_64/configure.ac
@@ -82,5 +82,8 @@ dnl It is always possible to access static and hidden symbols in an
 dnl position independent way.
 AC_DEFINE(PI_STATIC_AND_HIDDEN)
 
+dnl Static PIE is supported.
+AC_DEFINE(SUPPORT_STATIC_PIE)
+
 test -n "$critic_missing" && AC_MSG_ERROR([
 *** $critic_missing])
-- 
2.17.1

Re: [PATCH v5 3/7] configure: Check for static PIE support

[Adhemerval Zanella via Libc-alpha]

On 20/01/2021 12:30, Szabolcs Nagy via Libc-alpha wrote:
> Add SUPPORT_STATIC_PIE that targets can define if they support
> static PIE. This requires PI_STATIC_AND_HIDDEN support and various
> linker features as described in
> 
>   commit 9d7a3741c9e59eba87fb3ca6b9f979befce07826
>   Add --enable-static-pie configure option to build static PIE [BZ #19574]
> 
> Currently defined on x86_64, i386 and aarch64 where static PIE is
> known to work.

LGTM, thanks.

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

> ---
>  config.h.in                  |  3 +++
>  configure                    | 13 +++++++++++++
>  configure.ac                 |  4 ++++
>  sysdeps/aarch64/configure    |  4 ++++
>  sysdeps/aarch64/configure.ac |  3 +++
>  sysdeps/i386/configure       |  3 +++
>  sysdeps/i386/configure.ac    |  3 +++
>  sysdeps/x86_64/configure     |  3 +++
>  sysdeps/x86_64/configure.ac  |  3 +++
>  9 files changed, 39 insertions(+)
> 
> diff --git a/config.h.in b/config.h.in
> index 947feeff36..06ee8ae26a 100644
> --- a/config.h.in
> +++ b/config.h.in
> @@ -259,6 +259,9 @@
>  /* Build glibc with tunables support.  */
>  #define HAVE_TUNABLES 0
>  
> +/* Define if static PIE is supported.  */
> +#undef SUPPORT_STATIC_PIE
> +
>  /* Define if static PIE is enabled.  */
>  #define ENABLE_STATIC_PIE 0
>  

Ok.

> diff --git a/configure b/configure
> index 49f7b32b52..1dc3af60b4 100755
> --- a/configure
> +++ b/configure
> @@ -6814,6 +6814,19 @@ libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
>  
>  
>  if test "$static_pie" = yes; then
> +  # Check target support for static PIE
> +  cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +#ifndef SUPPORT_STATIC_PIE
> +# error static PIE is not supported
> +#endif
> +_ACEOF
> +if ac_fn_c_try_compile "$LINENO"; then :
> +
> +else
> +  as_fn_error $? "the architecture does not support static PIE" "$LINENO" 5
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
>    # The linker must support --no-dynamic-linker.
>    if test "$libc_cv_no_dynamic_linker" != yes; then
>      as_fn_error $? "linker support for --no-dynamic-linker needed" "$LINENO" 5


> diff --git a/configure.ac b/configure.ac
> index 341d4eeac2..dfebb8a7cc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1831,6 +1831,10 @@ libc_cv_multidir=`${CC-cc} $CFLAGS $CPPFLAGS -print-multi-directory`
>  AC_SUBST(libc_cv_multidir)
>  
>  if test "$static_pie" = yes; then
> +  # Check target support for static PIE
> +  AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#ifndef SUPPORT_STATIC_PIE
> +# error static PIE is not supported
> +#endif]])], , AC_MSG_ERROR([the architecture does not support static PIE]))
>    # The linker must support --no-dynamic-linker.
>    if test "$libc_cv_no_dynamic_linker" != yes; then
>      AC_MSG_ERROR([linker support for --no-dynamic-linker needed])

Ok.

> diff --git a/sysdeps/aarch64/configure b/sysdeps/aarch64/configure
> index 5f5f3cc44c..83c3a23e44 100644
> --- a/sysdeps/aarch64/configure
> +++ b/sysdeps/aarch64/configure
> @@ -6,6 +6,10 @@
>  $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
>  
>  
> +# Static PIE is supported.
> +$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
> +
> +
>  # We check to see if the compiler and flags are
>  # selecting the big endian ABI and if they are then
>  # we set libc_cv_aarch64_be to yes which causes

Skip since it is autogenerated.

> diff --git a/sysdeps/aarch64/configure.ac b/sysdeps/aarch64/configure.ac
> index 180a16a29f..66f755078a 100644
> --- a/sysdeps/aarch64/configure.ac
> +++ b/sysdeps/aarch64/configure.ac
> @@ -5,6 +5,9 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory.
>  # The exception is -mcmodel=large which is unsupported with PIC/PIE.
>  AC_DEFINE(PI_STATIC_AND_HIDDEN)
>  
> +# Static PIE is supported.
> +AC_DEFINE(SUPPORT_STATIC_PIE)
> +
>  # We check to see if the compiler and flags are
>  # selecting the big endian ABI and if they are then
>  # we set libc_cv_aarch64_be to yes which causes

Ok.

> diff --git a/sysdeps/i386/configure b/sysdeps/i386/configure
> index 90c63caf35..bb482ca16c 100644
> --- a/sysdeps/i386/configure
> +++ b/sysdeps/i386/configure
> @@ -117,3 +117,6 @@ if test x"$multi_arch" != xno; then
>    $as_echo "#define NO_HIDDEN_EXTERN_FUNC_IN_PIE 1" >>confdefs.h
>  
>  fi
> +
> +$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
> +

Skip since it is autogenerated.

> diff --git a/sysdeps/i386/configure.ac b/sysdeps/i386/configure.ac
> index 6d2068d2b3..5e43eb0adf 100644
> --- a/sysdeps/i386/configure.ac
> +++ b/sysdeps/i386/configure.ac
> @@ -77,3 +77,6 @@ dnl via PIC PLT in PIE, which requires setting up EBX register.
>  if test x"$multi_arch" != xno; then
>    AC_DEFINE(NO_HIDDEN_EXTERN_FUNC_IN_PIE)
>  fi
> +
> +dnl Static PIE is supported.
> +AC_DEFINE(SUPPORT_STATIC_PIE)

Ok.

> diff --git a/sysdeps/x86_64/configure b/sysdeps/x86_64/configure
> index 84f82c2406..198554d788 100644
> --- a/sysdeps/x86_64/configure
> +++ b/sysdeps/x86_64/configure
> @@ -143,5 +143,8 @@ fi
>  $as_echo "#define PI_STATIC_AND_HIDDEN 1" >>confdefs.h
>  
>  
> +$as_echo "#define SUPPORT_STATIC_PIE 1" >>confdefs.h
> +
> +
>  test -n "$critic_missing" && as_fn_error $? "
>  *** $critic_missing" "$LINENO" 5

Skip since it is autogenerated.

> diff --git a/sysdeps/x86_64/configure.ac b/sysdeps/x86_64/configure.ac
> index cdaba0c075..ec776274af 100644
> --- a/sysdeps/x86_64/configure.ac
> +++ b/sysdeps/x86_64/configure.ac
> @@ -82,5 +82,8 @@ dnl It is always possible to access static and hidden symbols in an
>  dnl position independent way.
>  AC_DEFINE(PI_STATIC_AND_HIDDEN)
>  
> +dnl Static PIE is supported.
> +AC_DEFINE(SUPPORT_STATIC_PIE)
> +
>  test -n "$critic_missing" && AC_MSG_ERROR([
>  *** $critic_missing])
> 

Ok.

[PATCH v5 4/7] csu: Avoid weak ref for __ehdr_start in static PIE

[Szabolcs Nagy via Libc-alpha]

All linkers support __ehdr_start that support static PIE linking,
so there is no need to check for its presence via a weak reference.

This avoids a RELATIVE relocation in static PIE startup code on some
targets.

With non-PIE static linking the weak ref check is kept in case the
linker does not support __ehdr_start.
---
 csu/libc-start.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index db859c3bed..5b9ce1d158 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -175,8 +175,12 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
          information from auxv.  */
 
       extern const ElfW(Ehdr) __ehdr_start
+# if BUILD_PIE_DEFAULT
+	__attribute__ ((visibility ("hidden")));
+# else
 	__attribute__ ((weak, visibility ("hidden")));
       if (&__ehdr_start != NULL)
+# endif
         {
           assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
           GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
-- 
2.17.1

Re: [PATCH v5 4/7] csu: Avoid weak ref for __ehdr_start in static PIE

[H.J. Lu via Libc-alpha]

On Wed, Jan 20, 2021 at 7:34 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> All linkers support __ehdr_start that support static PIE linking,
> so there is no need to check for its presence via a weak reference.
>
> This avoids a RELATIVE relocation in static PIE startup code on some
> targets.
>
> With non-PIE static linking the weak ref check is kept in case the
> linker does not support __ehdr_start.
> ---
>  csu/libc-start.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index db859c3bed..5b9ce1d158 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -175,8 +175,12 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>           information from auxv.  */
>
>        extern const ElfW(Ehdr) __ehdr_start
> +# if BUILD_PIE_DEFAULT
> +       __attribute__ ((visibility ("hidden")));
> +# else
>         __attribute__ ((weak, visibility ("hidden")));
>        if (&__ehdr_start != NULL)
> +# endif
>          {
>            assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
>            GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> --
> 2.17.1
>

LGTM.  Please wait for Adhemerval approval.

-- 
H.J.

Re: [PATCH v5 4/7] csu: Avoid weak ref for __ehdr_start in static PIE

[Adhemerval Zanella via Libc-alpha]

On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha wrote:
> All linkers support __ehdr_start that support static PIE linking,
> so there is no need to check for its presence via a weak reference.
> 
> This avoids a RELATIVE relocation in static PIE startup code on some
> targets.
> 
> With non-PIE static linking the weak ref check is kept in case the
> linker does not support __ehdr_start.

LGTM, thanks.

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

> ---
>  csu/libc-start.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index db859c3bed..5b9ce1d158 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -175,8 +175,12 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>           information from auxv.  */
>  
>        extern const ElfW(Ehdr) __ehdr_start
> +# if BUILD_PIE_DEFAULT
> +	__attribute__ ((visibility ("hidden")));
> +# else
>  	__attribute__ ((weak, visibility ("hidden")));
>        if (&__ehdr_start != NULL)
> +# endif
>          {
>            assert (__ehdr_start.e_phentsize == sizeof *GL(dl_phdr));
>            GL(dl_phdr) = (const void *) &__ehdr_start + __ehdr_start.e_phoff;
> 

[PATCH v5 5/7] Use hidden visibility for early static PIE code

[Szabolcs Nagy via Libc-alpha]

Extern symbol access in position independent code usually involves GOT
indirection which needs RELATIVE reloc in a static linked PIE. (On
some targets this is avoided e.g. because the linker can relax a GOT
access to a pc-relative access, but this is not generally true.) Code
that runs before static PIE self relocation must avoid relying on
dynamic relocations which can be ensured by using hidden visibility.
However we cannot just make all symbols 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.

This patch aims to make symbol references hidden in code that is used
before and by _dl_relocate_static_pie when building a static PIE libc.
Note: for an object that is used in the startup code, its references
and definition may not have consistent visibility: it is only forced
hidden in the startup code.

This is needed for fixing bug 27072.

Co-authored-by: H.J. Lu <hjl.tools@gmail.com>
---
 csu/libc-start.c                             | 4 ++++
 elf/dl-reloc-static-pie.c                    | 2 ++
 elf/dl-support.c                             | 6 ++++++
 elf/dl-tunables.c                            | 4 ++++
 elf/enbl-secure.c                            | 4 ++++
 misc/sbrk.c                                  | 4 ++++
 sysdeps/unix/sysv/linux/aarch64/libc-start.c | 5 +++++
 sysdeps/x86/libc-start.c                     | 5 +++++
 8 files changed, 34 insertions(+)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 5b9ce1d158..a2f6e12728 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -15,6 +15,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
index a8d964061e..d5bd2f31e9 100644
--- a/elf/dl-reloc-static-pie.c
+++ b/elf/dl-reloc-static-pie.c
@@ -17,6 +17,8 @@
    <https://www.gnu.org/licenses/>.  */
 
 #if ENABLE_STATIC_PIE
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# 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 2434c470c7..7abb65d8e3 100644
--- a/elf/dl-support.c
+++ b/elf/dl-support.c
@@ -19,6 +19,12 @@
 /* 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>
+/* Mark symbols hidden in static PIE for early self relocation to work.
+   Note: string.h may have ifuncs which cannot be hidden on i686.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <errno.h>
 #include <libintl.h>
 #include <stdlib.h>
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
index e44476f204..b1a50b8469 100644
--- a/elf/dl-tunables.c
+++ b/elf/dl-tunables.c
@@ -18,6 +18,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#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 5dcf649626..9e47526bd3 100644
--- a/elf/enbl-secure.c
+++ b/elf/enbl-secure.c
@@ -19,6 +19,10 @@
 /* This file is used in the static libc.  For the shared library,
    dl-sysdep.c defines and initializes __libc_enable_secure.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <startup.h>
 #include <libc-internal.h>
 
diff --git a/misc/sbrk.c b/misc/sbrk.c
index 99b3fb517e..95800b32aa 100644
--- a/misc/sbrk.c
+++ b/misc/sbrk.c
@@ -15,6 +15,10 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+#if BUILD_PIE_DEFAULT
+# pragma GCC visibility push(hidden)
+#endif
 #include <errno.h>
 #include <libc-internal.h>
 #include <stdbool.h>
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
index f816f04ee1..e1604a6ed0 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
@@ -17,6 +17,11 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef SHARED
+
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# if BUILD_PIE_DEFAULT
+#  pragma GCC visibility push(hidden)
+# endif
 # include <ldsodefs.h>
 # include <cpu-features.c>
 
diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
index 4bbd7d555b..d30aec2aa1 100644
--- a/sysdeps/x86/libc-start.c
+++ b/sysdeps/x86/libc-start.c
@@ -16,6 +16,11 @@
    <https://www.gnu.org/licenses/>.  */
 
 #ifndef SHARED
+
+/* Mark symbols hidden in static PIE for early self relocation to work.  */
+# if BUILD_PIE_DEFAULT
+#  pragma GCC visibility push(hidden)
+# endif
 /* Define I386_USE_SYSENTER to support syscall during startup in static
    PIE.  */
 # include <startup.h>
-- 
2.17.1

Re: [PATCH v5 5/7] Use hidden visibility for early static PIE code

[Adhemerval Zanella via Libc-alpha]

On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha wrote:
> Extern symbol access in position independent code usually involves GOT
> indirection which needs RELATIVE reloc in a static linked PIE. (On
> some targets this is avoided e.g. because the linker can relax a GOT
> access to a pc-relative access, but this is not generally true.) Code
> that runs before static PIE self relocation must avoid relying on
> dynamic relocations which can be ensured by using hidden visibility.
> However we cannot just make all symbols 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.
> 
> This patch aims to make symbol references hidden in code that is used
> before and by _dl_relocate_static_pie when building a static PIE libc.
> Note: for an object that is used in the startup code, its references
> and definition may not have consistent visibility: it is only forced
> hidden in the startup code.
> 
> This is needed for fixing bug 27072.
> 
> Co-authored-by: H.J. Lu <hjl.tools@gmail.com>

LGTM, thanks.

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

> ---
>  csu/libc-start.c                             | 4 ++++
>  elf/dl-reloc-static-pie.c                    | 2 ++
>  elf/dl-support.c                             | 6 ++++++
>  elf/dl-tunables.c                            | 4 ++++
>  elf/enbl-secure.c                            | 4 ++++
>  misc/sbrk.c                                  | 4 ++++
>  sysdeps/unix/sysv/linux/aarch64/libc-start.c | 5 +++++
>  sysdeps/x86/libc-start.c                     | 5 +++++
>  8 files changed, 34 insertions(+)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 5b9ce1d158..a2f6e12728 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -15,6 +15,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <assert.h>
>  #include <stdlib.h>
>  #include <stdio.h>

Ok.

> diff --git a/elf/dl-reloc-static-pie.c b/elf/dl-reloc-static-pie.c
> index a8d964061e..d5bd2f31e9 100644
> --- a/elf/dl-reloc-static-pie.c
> +++ b/elf/dl-reloc-static-pie.c
> @@ -17,6 +17,8 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #if ENABLE_STATIC_PIE
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# pragma GCC visibility push(hidden)
>  #include <unistd.h>
>  #include <ldsodefs.h>
>  #include "dynamic-link.h"

Ok.

> diff --git a/elf/dl-support.c b/elf/dl-support.c
> index 2434c470c7..7abb65d8e3 100644
> --- a/elf/dl-support.c
> +++ b/elf/dl-support.c
> @@ -19,6 +19,12 @@
>  /* 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>
> +/* Mark symbols hidden in static PIE for early self relocation to work.
> +   Note: string.h may have ifuncs which cannot be hidden on i686.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <errno.h>
>  #include <libintl.h>
>  #include <stdlib.h>

Ok.

> diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> index e44476f204..b1a50b8469 100644
> --- a/elf/dl-tunables.c
> +++ b/elf/dl-tunables.c
> @@ -18,6 +18,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <startup.h>
>  #include <stdint.h>
>  #include <stdbool.h>

Ok.

> diff --git a/elf/enbl-secure.c b/elf/enbl-secure.c
> index 5dcf649626..9e47526bd3 100644
> --- a/elf/enbl-secure.c
> +++ b/elf/enbl-secure.c
> @@ -19,6 +19,10 @@
>  /* This file is used in the static libc.  For the shared library,
>     dl-sysdep.c defines and initializes __libc_enable_secure.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <startup.h>
>  #include <libc-internal.h>
>  

Ok.

> diff --git a/misc/sbrk.c b/misc/sbrk.c
> index 99b3fb517e..95800b32aa 100644
> --- a/misc/sbrk.c
> +++ b/misc/sbrk.c
> @@ -15,6 +15,10 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +#if BUILD_PIE_DEFAULT
> +# pragma GCC visibility push(hidden)
> +#endif
>  #include <errno.h>
>  #include <libc-internal.h>
>  #include <stdbool.h>

Ok.

> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.c b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> index f816f04ee1..e1604a6ed0 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.c
> @@ -17,6 +17,11 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef SHARED
> +
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# if BUILD_PIE_DEFAULT
> +#  pragma GCC visibility push(hidden)
> +# endif
>  # include <ldsodefs.h>
>  # include <cpu-features.c>
>  

Ok.

> diff --git a/sysdeps/x86/libc-start.c b/sysdeps/x86/libc-start.c
> index 4bbd7d555b..d30aec2aa1 100644
> --- a/sysdeps/x86/libc-start.c
> +++ b/sysdeps/x86/libc-start.c
> @@ -16,6 +16,11 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #ifndef SHARED
> +
> +/* Mark symbols hidden in static PIE for early self relocation to work.  */
> +# if BUILD_PIE_DEFAULT
> +#  pragma GCC visibility push(hidden)
> +# endif
>  /* Define I386_USE_SYSENTER to support syscall during startup in static
>     PIE.  */
>  # include <startup.h>
> 

Ok.

[PATCH v5 6/7] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

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 up to relocation processing includes

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

These are simple enough that RELATIVE relocs can be avoided.

The following steps include

  ARCH_SETUP_IREL ();
  ARCH_SETUP_TLS ();
  ARCH_APPLY_IREL ();

On some targets IRELATIVE processing relies on TLS setup on
others TLS setup relies on IRELATIVE relocs, so the right
position for _dl_relocate_static_pie is target dependent.
For now move self relocation as early as possible on targets
that support static PIE.

Fixes bug 27072.
---
 csu/libc-start.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index a2f6e12728..feb0d7ce11 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -146,8 +146,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;
@@ -199,6 +197,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 ();
 
-- 
2.17.1

Re: [PATCH v5 6/7] csu: Move static pie self relocation later [BZ #27072]

[Adhemerval Zanella via Libc-alpha]

On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha 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.

As a side note, how hard would that be? I really think we should aim
to make the bootstrap less fragile and it would also make porting
static-pie to other architecture easier.

> 
> The early startup code up to relocation processing includes
> 
>   _dl_aux_init (auxvec);
>   __libc_init_secure ();
>   __tunables_init (__environ);
>   ARCH_INIT_CPU_FEATURES ();
>   _dl_relocate_static_pie ();
> 
> These are simple enough that RELATIVE relocs can be avoided.
> 
> The following steps include
> 
>   ARCH_SETUP_IREL ();
>   ARCH_SETUP_TLS ();
>   ARCH_APPLY_IREL ();
> 
> On some targets IRELATIVE processing relies on TLS setup on
> others TLS setup relies on IRELATIVE relocs, so the right
> position for _dl_relocate_static_pie is target dependent.
> For now move self relocation as early as possible on targets
> that support static PIE.
> 
> Fixes bug 27072.

LGTM, thanks.

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

> ---
>  csu/libc-start.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index a2f6e12728..feb0d7ce11 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -146,8 +146,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;
> @@ -199,6 +197,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 ();
>  
> 

Ok.

Re: [PATCH v5 6/7] csu: Move static pie self relocation later [BZ #27072]

[Szabolcs Nagy via Libc-alpha]

The 01/21/2021 11:07, Adhemerval Zanella wrote:
> 
> 
> On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha 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.
> 
> As a side note, how hard would that be? I really think we should aim
> to make the bootstrap less fragile and it would also make porting
> static-pie to other architecture easier.

the IRELATIVE relocs are in the DT_JMPREL area, while RELATIVE
relocs are outside of it. (this split is to allow simpler lazy
binding, i don't know how strictly this is enforced in elf, but
since ifunc is a gnu extension we can just say that this is abi.)

so i think you only have to change/copy elf/dynamic-link.h and
have a ELF_DYNAMIC_RELOCATE variant that processes the two sets
of relocations separately.

if you don't want to rely on DT_JMPREL then i think you have to
change elf/do-rel.h as well to have a elf_dynamic_do_Rel variant
that does IRELATIVE separately.

either case changing these looked more scary to me than the
current patches: likely we have to copy complex logic with slight
modification and there are a lot of things going on there. (but
that was before i learnt about the i386 hidden issue and hidden
weak refs and tunables_strdup.)

> > 
> > The early startup code up to relocation processing includes
> > 
> >   _dl_aux_init (auxvec);
> >   __libc_init_secure ();
> >   __tunables_init (__environ);
> >   ARCH_INIT_CPU_FEATURES ();
> >   _dl_relocate_static_pie ();
> > 
> > These are simple enough that RELATIVE relocs can be avoided.
> > 
> > The following steps include
> > 
> >   ARCH_SETUP_IREL ();
> >   ARCH_SETUP_TLS ();
> >   ARCH_APPLY_IREL ();
> > 
> > On some targets IRELATIVE processing relies on TLS setup on
> > others TLS setup relies on IRELATIVE relocs, so the right
> > position for _dl_relocate_static_pie is target dependent.
> > For now move self relocation as early as possible on targets
> > that support static PIE.
> > 
> > Fixes bug 27072.
> 
> LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

thanks.

[PATCH v5 7/7] Make libc symbols hidden in static PIE

[Szabolcs Nagy via Libc-alpha]

Hidden visibility can avoid indirections and RELATIVE relocs in
static PIE libc.

The check should use IS_IN_LIB instead of IS_IN(libc) since all
symbols are defined locally in static PIE and the optimization is
useful in all libraries not just libc. However the test system
links objects from libcrypt.a into dynamic linked test binaries
where hidden visibility does not work.  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.

On some targets (i386) this optimization cannot be applied because
hidden visibility PIE ifunc functions don't work, so it is gated by
NO_HIDDEN_EXTERN_FUNC_IN_PIE.

From -static-pie linked 'int main(){}' this shaves off 71 relative
relocs on aarch64 and reduces code size by about 2k.
---
 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 v5 7/7] Make libc symbols hidden in static PIE

[Adhemerval Zanella via Libc-alpha]

On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha wrote:
> Hidden visibility can avoid indirections and RELATIVE relocs in
> static PIE libc.
> 
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> symbols are defined locally in static PIE and the optimization is
> useful in all libraries not just libc. However the test system
> links objects from libcrypt.a into dynamic linked test binaries
> where hidden visibility does not work.  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.

Why do we need this linkage for testing? Could we fix it so this
change could be used in all static libraries?

> 
> On some targets (i386) this optimization cannot be applied because
> hidden visibility PIE ifunc functions don't work, so it is gated by
> NO_HIDDEN_EXTERN_FUNC_IN_PIE.
> 
> From -static-pie linked 'int main(){}' this shaves off 71 relative
> relocs on aarch64 and reduces code size by about 2k.

Nice.

LGTM, thanks.

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

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

Ok.

Re: [PATCH v5 7/7] Make libc symbols hidden in static PIE

[Szabolcs Nagy via Libc-alpha]

The 01/21/2021 11:10, Adhemerval Zanella wrote:
> On 20/01/2021 12:31, Szabolcs Nagy via Libc-alpha wrote:
> > Hidden visibility can avoid indirections and RELATIVE relocs in
> > static PIE libc.
> > 
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > symbols are defined locally in static PIE and the optimization is
> > useful in all libraries not just libc. However the test system
> > links objects from libcrypt.a into dynamic linked test binaries
> > where hidden visibility does not work.  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.
> 
> Why do we need this linkage for testing? Could we fix it so this
> change could be used in all static libraries?

i think the md5 and sha* tests want to directly
call some internal symbol (for the hash
implementation instead of the iterated password
hash) which you can do if you static link
libcrypt.a

i don't know if this may appear in user code
(static linking some libfoo.a from libc into
a dynamic linked executable)

may be the crypt tests can be changed to be
fully static linked.

Re: [PATCH v5 7/7] Make libc symbols hidden in static PIE

[Tulio Magno Quites Machado Filho via Libc-alpha]

Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org> writes:

> Hidden visibility can avoid indirections and RELATIVE relocs in
> static PIE libc.
>
> The check should use IS_IN_LIB instead of IS_IN(libc) since all
> symbols are defined locally in static PIE and the optimization is
> useful in all libraries not just libc. However the test system
> links objects from libcrypt.a into dynamic linked test binaries
> where hidden visibility does not work.  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.
>
> On some targets (i386) this optimization cannot be applied because
> hidden visibility PIE ifunc functions don't work, so it is gated by
> NO_HIDDEN_EXTERN_FUNC_IN_PIE.
>
> From -static-pie linked 'int main(){}' this shaves off 71 relative
> relocs on aarch64 and reduces code size by about 2k.

After this patch got merged, I noticed that
malloc/tst-malloc-stats-cancellation never returns on ppc (32-bit only).

It seems stuck in a futex_wait:

#0  0x6fddab58 in futex_wait (private=0, expected=2, futex_word=0x6ff405ec <main_arena>) at ../sysdeps/nptl/futex-internal.h:146
#1  __lll_lock_wait_private (futex=0x6ff405ec <main_arena>) at ./lowlevellock.c:35
#2  0x6fde3614 in __GI___libc_malloc (bytes=bytes@entry=4096) at malloc.c:3235
#3  0x6fdbf7f0 in __GI__IO_file_doallocate (fp=0xf7dc01a0) at filedoalloc.c:101
#4  0x6fdd6c20 in __GI__IO_doallocbuf (fp=0xf7dc01a0) at libioP.h:948
#5  __GI__IO_doallocbuf (fp=fp@entry=0xf7dc01a0) at genops.c:342
#6  0x6fdd5768 in _IO_new_file_overflow (f=0xf7dc01a0, ch=-1) at fileops.c:745
#7  0x6fdd41c0 in _IO_new_file_xsputn (n=37, data=<optimized out>, f=<optimized out>) at libioP.h:948
#8  _IO_new_file_xsputn (f=0xf7dc01a0, data=<optimized out>, n=37) at fileops.c:1197
#9  0x6fdc19e8 in __GI__IO_fwrite (buf=0x6ffc1880, size=1, count=37, fp=0xf7dc01a0) at libioP.h:948
#10 0x6ffc0e80 in ?? ()
#11 0x6fd6641c in generic_start_main (main=0x6ffc0b10, argc=1, argv=0xf6c00760, auxvec=0x0, init=<optimized out>, rtld_fini=<optimized out>, stack_end=<optimized out>, fini=<optimized out>) at ../csu/libc-start.c:332
#12 0x6fd665c0 in __libc_start_main (argc=<optimized out>, argv=<optimized out>, ev=<optimized out>, auxvec=<optimized out>, rtld_fini=<optimized out>, stinfo=<optimized out>, stack_on_entry=<optimized out>) at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
#13 0x00000000 in ?? ()

-- 
Tulio Magno

Re: [PATCH v5 7/7] Make libc symbols hidden in static PIE

[Szabolcs Nagy via Libc-alpha]

The 01/21/2021 22:25, Tulio Magno Quites Machado Filho wrote:
> Szabolcs Nagy via Libc-alpha <libc-alpha@sourceware.org> writes:
> 
> > Hidden visibility can avoid indirections and RELATIVE relocs in
> > static PIE libc.
> >
> > The check should use IS_IN_LIB instead of IS_IN(libc) since all
> > symbols are defined locally in static PIE and the optimization is
> > useful in all libraries not just libc. However the test system
> > links objects from libcrypt.a into dynamic linked test binaries
> > where hidden visibility does not work.  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.
> >
> > On some targets (i386) this optimization cannot be applied because
> > hidden visibility PIE ifunc functions don't work, so it is gated by
> > NO_HIDDEN_EXTERN_FUNC_IN_PIE.
> >
> > From -static-pie linked 'int main(){}' this shaves off 71 relative
> > relocs on aarch64 and reduces code size by about 2k.
> 
> After this patch got merged, I noticed that
> malloc/tst-malloc-stats-cancellation never returns on ppc (32-bit only).

that patch is only expected to change anything if

defined PIC && !defined SHARED && defined IS_IN(libc) && !defined LIBC_NONSHARED

i.e. when libc.a is compiled as pie.

can you check the build log e.g.

grep ' -DMODULE_NAME=libc .*-DPIC' build-and-check.log |grep -v ' -DSHARED'

should not find anything.

the backtrace e.g. can happen if malloc_stat was cancelled somehow
while the malloc lock is held and then the main thread tries to
print some error message but cannot get the malloc lock. but it is
hard to tell just from the backtrace where things went wrong.

> 
> It seems stuck in a futex_wait:
> 
> #0  0x6fddab58 in futex_wait (private=0, expected=2, futex_word=0x6ff405ec <main_arena>) at ../sysdeps/nptl/futex-internal.h:146
> #1  __lll_lock_wait_private (futex=0x6ff405ec <main_arena>) at ./lowlevellock.c:35
> #2  0x6fde3614 in __GI___libc_malloc (bytes=bytes@entry=4096) at malloc.c:3235
> #3  0x6fdbf7f0 in __GI__IO_file_doallocate (fp=0xf7dc01a0) at filedoalloc.c:101
> #4  0x6fdd6c20 in __GI__IO_doallocbuf (fp=0xf7dc01a0) at libioP.h:948
> #5  __GI__IO_doallocbuf (fp=fp@entry=0xf7dc01a0) at genops.c:342
> #6  0x6fdd5768 in _IO_new_file_overflow (f=0xf7dc01a0, ch=-1) at fileops.c:745
> #7  0x6fdd41c0 in _IO_new_file_xsputn (n=37, data=<optimized out>, f=<optimized out>) at libioP.h:948
> #8  _IO_new_file_xsputn (f=0xf7dc01a0, data=<optimized out>, n=37) at fileops.c:1197
> #9  0x6fdc19e8 in __GI__IO_fwrite (buf=0x6ffc1880, size=1, count=37, fp=0xf7dc01a0) at libioP.h:948
> #10 0x6ffc0e80 in ?? ()
> #11 0x6fd6641c in generic_start_main (main=0x6ffc0b10, argc=1, argv=0xf6c00760, auxvec=0x0, init=<optimized out>, rtld_fini=<optimized out>, stack_end=<optimized out>, fini=<optimized out>) at ../csu/libc-start.c:332
> #12 0x6fd665c0 in __libc_start_main (argc=<optimized out>, argv=<optimized out>, ev=<optimized out>, auxvec=<optimized out>, rtld_fini=<optimized out>, stinfo=<optimized out>, stack_on_entry=<optimized out>) at ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
> #13 0x00000000 in ?? ()
> 
> -- 
> Tulio Magno

Re: [PATCH v5 7/7] Make libc symbols hidden in static PIE

[Andreas Schwab]

This breaks every newly linked program.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."