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

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

[Szabolcs Nagy]

on aarch64 this depends on a patch i posted earlier:
https://sourceware.org/pipermail/libc-alpha/2021-January/121366.html

with that aarch64 static pie tests pass.

i'm still working on the tunables change and thinging about
libc build time checks for reloc-free early startup code.

i havent tested x86, it might need changes.

Szabolcs Nagy (3):
  Make libc symbols hidden in static PIE
  [RFC] elf: hack up tunables to avoid RELATIVE relocs
  csu: Move static pie self relocation later [BZ #27072]

 csu/libc-start.c         | 10 ++++++++++
 elf/dl-tunables.h        |  4 ++--
 include/libc-symbols.h   |  8 ++++++--
 scripts/gen-tunables.awk |  2 +-
 4 files changed, 19 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/3] 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 2/3] [RFC] elf: hack up tunables to avoid RELATIVE relocs

[Szabolcs Nagy]

With static pie linking pointers in the tunables list need
RELATIVE relocs since the absolute address is not known at
link time.

This is a hack to avoid relocs in tunables so the static pie
self relocation can be done later.

The longest tunable name is currently
  glibc.elision.skip_trylock_internal_abort
and the longest env var alias is
  MALLOC_MMAP_THRESHOLD_
There are likely ways to have a compact pointer-free tunable
list data structure with more gen-tunables.awk changes, but
before that i would like to get feedback if this approach
for bug 27072 is acceptable.
---
 elf/dl-tunables.h        | 4 ++--
 scripts/gen-tunables.awk | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
index 518342a300..0196867676 100644
--- a/elf/dl-tunables.h
+++ b/elf/dl-tunables.h
@@ -38,7 +38,7 @@ __tunables_init (char **unused __attribute__ ((unused)))
 /* A tunable.  */
 struct _tunable
 {
-  const char *name;			/* Internal name of the tunable.  */
+  const char name[48];			/* 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
@@ -54,7 +54,7 @@ struct _tunable
 					   target module if the value is
 					   considered unsafe.  */
   /* Compatibility elements.  */
-  const char *env_alias;		/* The compatibility environment
+  const char env_alias[32];		/* The compatibility environment
 					   variable name.  */
 };
 
diff --git a/scripts/gen-tunables.awk b/scripts/gen-tunables.awk
index 622199061a..9e7bd24e13 100644
--- a/scripts/gen-tunables.awk
+++ b/scripts/gen-tunables.awk
@@ -57,7 +57,7 @@ $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"
-- 
2.17.1

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

[Szabolcs Nagy]

On targets where hidden symbol access does not need RELATIVE
relocs, move the static pie self relocation after tunables and
cpu features are set up.  This allows processing IRELATIVE
relocs with correct ifunc dispatch logic.

Unfortunately it is hard to guarantee that there will be no
dynamic relocations in the early start up code, so this is a
bit fragile. Ideally the RELATIVE relocs would be processed as
early as possible and IRELATIVE relocs after cpu features are
setup, but in glibc it is hard to separate them into two steps.
---
 csu/libc-start.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/csu/libc-start.c b/csu/libc-start.c
index db859c3bed..b8d22bd59e 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -142,7 +142,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   int result;
 
 #ifndef SHARED
+# ifndef PI_STATIC_AND_HIDDEN
+  /* Do static pie self relocation as early as possible.  */
   _dl_relocate_static_pie ();
+# endif
 
   char **ev = &argv[argc + 1];
 
@@ -191,6 +194,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
 
   ARCH_INIT_CPU_FEATURES ();
 
+# ifdef PI_STATIC_AND_HIDDEN
+  /* Do static pie self relocation after cpu features are setup.
+     Code before this point must avoid relocations, which in practice
+     means no initialized global pointer or ifunc symbol access.  */
+  _dl_relocate_static_pie ();
+# endif
+
   /* Perform IREL{,A} relocations.  */
   ARCH_SETUP_IREL ();
 
-- 
2.17.1

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

[H.J. Lu]

On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On targets where hidden symbol access does not need RELATIVE
> relocs, move the static pie self relocation after tunables and
> cpu features are set up.  This allows processing IRELATIVE
> relocs with correct ifunc dispatch logic.
>
> Unfortunately it is hard to guarantee that there will be no
> dynamic relocations in the early start up code, so this is a
> bit fragile. Ideally the RELATIVE relocs would be processed as
> early as possible and IRELATIVE relocs after cpu features are
> setup, but in glibc it is hard to separate them into two steps.

x86 linker places IRELATIVE relocations the last:

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

Does your linker have this bug fixed?

-- 
H.J.

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

[Carlos O'Donell]

On 1/7/21 7:36 AM, H.J. Lu via Libc-alpha wrote:
> On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> On targets where hidden symbol access does not need RELATIVE
>> relocs, move the static pie self relocation after tunables and
>> cpu features are set up.  This allows processing IRELATIVE
>> relocs with correct ifunc dispatch logic.
>>
>> Unfortunately it is hard to guarantee that there will be no
>> dynamic relocations in the early start up code, so this is a
>> bit fragile. Ideally the RELATIVE relocs would be processed as
>> early as possible and IRELATIVE relocs after cpu features are
>> setup, but in glibc it is hard to separate them into two steps.
> 
> x86 linker places IRELATIVE relocations the last:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> 
> Does your linker have this bug fixed?

Agreed, this is something I asked about during the design of
IFUNCs and I was told by Ulrich and others that IRELATIVE has
to be placed in a group and after RELATIVE relocs.

We need to document more of the guarantees and semantics here.

-- 
Cheers,
Carlos.

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[H.J. Lu]

On Thu, Jan 7, 2021 at 3:01 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> on aarch64 this depends on a patch i posted earlier:
> https://sourceware.org/pipermail/libc-alpha/2021-January/121366.html
>
> with that aarch64 static pie tests pass.
>
> i'm still working on the tunables change and thinging about
> libc build time checks for reloc-free early startup code.
>
> i havent tested x86, it might need changes.

Please fix the linker bug:

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

first before changing glibc.

-- 
H.J.

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[Carlos O'Donell]

On 1/7/21 7:48 AM, H.J. Lu via Libc-alpha wrote:
> On Thu, Jan 7, 2021 at 3:01 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
>>
>> on aarch64 this depends on a patch i posted earlier:
>> https://sourceware.org/pipermail/libc-alpha/2021-January/121366.html
>>
>> with that aarch64 static pie tests pass.
>>
>> i'm still working on the tunables change and thinging about
>> libc build time checks for reloc-free early startup code.
>>
>> i havent tested x86, it might need changes.
> 
> Please fix the linker bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> 
> first before changing glibc.

You will still need potential changes in the loader to handle
existing binaries, so the binutils fix need not come first,
but it needs fixing.

-- 
Cheers,
Carlos.

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

[H.J. Lu]

On Thu, Jan 7, 2021 at 4:46 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 1/7/21 7:36 AM, H.J. Lu via Libc-alpha wrote:
> > On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> On targets where hidden symbol access does not need RELATIVE
> >> relocs, move the static pie self relocation after tunables and
> >> cpu features are set up.  This allows processing IRELATIVE
> >> relocs with correct ifunc dispatch logic.
> >>
> >> Unfortunately it is hard to guarantee that there will be no
> >> dynamic relocations in the early start up code, so this is a
> >> bit fragile. Ideally the RELATIVE relocs would be processed as
> >> early as possible and IRELATIVE relocs after cpu features are
> >> setup, but in glibc it is hard to separate them into two steps.
> >
> > x86 linker places IRELATIVE relocations the last:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> >
> > Does your linker have this bug fixed?
>
> Agreed, this is something I asked about during the design of
> IFUNCs and I was told by Ulrich and others that IRELATIVE has
> to be placed in a group and after RELATIVE relocs.

Not just before RELATIVE.   IRELATIVE should be processed the
last before all other relocations.   See PR 13302 for other cases.

-- 
H.J.

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[H.J. Lu]

On Thu, Jan 7, 2021 at 4:50 AM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 1/7/21 7:48 AM, H.J. Lu via Libc-alpha wrote:
> > On Thu, Jan 7, 2021 at 3:01 AM Szabolcs Nagy via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> on aarch64 this depends on a patch i posted earlier:
> >> https://sourceware.org/pipermail/libc-alpha/2021-January/121366.html
> >>
> >> with that aarch64 static pie tests pass.
> >>
> >> i'm still working on the tunables change and thinging about
> >> libc build time checks for reloc-free early startup code.
> >>
> >> i havent tested x86, it might need changes.
> >
> > Please fix the linker bug:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> >
> > first before changing glibc.
>
> You will still need potential changes in the loader to handle
> existing binaries, so the binutils fix need not come first,

Linker fix may impact how glibc should be changed.

> but it needs fixing.
>

Whatever we do in glibc,  please make it target dependent
and it should be NOP for x86.


-- 
H.J.

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

[Szabolcs Nagy]

The 01/07/2021 04:51, H.J. Lu wrote:
> On Thu, Jan 7, 2021 at 4:46 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >
> > On 1/7/21 7:36 AM, H.J. Lu via Libc-alpha wrote:
> > > On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
> > > <libc-alpha@sourceware.org> wrote:
> > >>
> > >> On targets where hidden symbol access does not need RELATIVE
> > >> relocs, move the static pie self relocation after tunables and
> > >> cpu features are set up.  This allows processing IRELATIVE
> > >> relocs with correct ifunc dispatch logic.
> > >>
> > >> Unfortunately it is hard to guarantee that there will be no
> > >> dynamic relocations in the early start up code, so this is a
> > >> bit fragile. Ideally the RELATIVE relocs would be processed as
> > >> early as possible and IRELATIVE relocs after cpu features are
> > >> setup, but in glibc it is hard to separate them into two steps.
> > >
> > > x86 linker places IRELATIVE relocations the last:
> > >
> > > https://sourceware.org/bugzilla/show_bug.cgi?id=13302
> > >
> > > Does your linker have this bug fixed?
> >
> > Agreed, this is something I asked about during the design of
> > IFUNCs and I was told by Ulrich and others that IRELATIVE has
> > to be placed in a group and after RELATIVE relocs.
> 
> Not just before RELATIVE.   IRELATIVE should be processed the
> last before all other relocations.   See PR 13302 for other cases.

this is about static pie: sorting relocs does not help, the
problem is not that ifunc resolvers have some relocs in them,
but that the target specific relocation processing is one
monolithic switch in a loop that handles all relocs in one go,
there is no api to request only a subset of relocs to be
processed (either by type or ordering in the list of relocs).

in __libc_start_main we need to do

  do_relative_relocs();
  setup_auxv_tunables_cpu_etc();
  do_irelative_relocs();

which cannot be done without major changes in all targets and
generic elf reloc handling code. so the second best is

  setup_auxv_tunables_cpu_etc(); // avoid relative relocs
  do_all_relocs();

on targets where this can be done (other targets do
not support static pie).

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy]

The 01/07/2021 04:55, H.J. Lu wrote:
> Whatever we do in glibc,  please make it target dependent
> and it should be NOP for x86.

the current x86 logic is completely broken

it is not valid to do irelative before tunables.

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[H.J. Lu]

On Thu, Jan 7, 2021 at 5:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/07/2021 04:55, H.J. Lu wrote:
> > Whatever we do in glibc,  please make it target dependent
> > and it should be NOP for x86.
>
> the current x86 logic is completely broken
>
> it is not valid to do irelative before tunables.

Do you have a testcase to show that?

-- 
H.J.

Re: [PATCH 0/3] fix ifunc with static pie [BZ #27072]

[Szabolcs Nagy]

The 01/07/2021 05:15, H.J. Lu wrote:
> On Thu, Jan 7, 2021 at 5:03 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
> >
> > The 01/07/2021 04:55, H.J. Lu wrote:
> > > Whatever we do in glibc,  please make it target dependent
> > > and it should be NOP for x86.
> >
> > the current x86 logic is completely broken
> >
> > it is not valid to do irelative before tunables.
> 
> Do you have a testcase to show that?

what's the point of

GLIBC_TUNABLES=glibc.cpu.hwcaps=...

if it does not affect ifunc selection?

Re: [PATCH 2/3] [RFC] elf: hack up tunables to avoid RELATIVE relocs

[Siddhesh Poyarekar]

On 1/7/21 4:31 PM, 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.
> 
> This is a hack to avoid relocs in tunables so the static pie
> self relocation can be done later.
> 
> The longest tunable name is currently
>    glibc.elision.skip_trylock_internal_abort
> and the longest env var alias is
>    MALLOC_MMAP_THRESHOLD_
> There are likely ways to have a compact pointer-free tunable
> list data structure with more gen-tunables.awk changes, but
> before that i would like to get feedback if this approach
> for bug 27072 is acceptable.

I think this is a good idea independently of the rest of the patchset. 
The only change I'd like is to make name 64 bytes and env_alias 24 
bytes.  We don't have any additional environment variables to convert 
into tunables, so the max length of MALLOC_MMAP_THRESHOLD_ is about as 
big as it can get.

Siddhesh

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

[Szabolcs Nagy]

The 01/07/2021 13:02, Szabolcs Nagy via Libc-alpha wrote:
> this is about static pie: sorting relocs does not help, the
> problem is not that ifunc resolvers have some relocs in them,
> but that the target specific relocation processing is one
> monolithic switch in a loop that handles all relocs in one go,
> there is no api to request only a subset of relocs to be
> processed (either by type or ordering in the list of relocs).
> 
> in __libc_start_main we need to do
> 
>   do_relative_relocs();
>   setup_auxv_tunables_cpu_etc();
>   do_irelative_relocs();

i just realized we can do this because RELATIVE and IRELATIVE
happen to be in different places (one where DT_REL{A} points
to and another where DT_JUMPREL points to).

i dont know how reliable this is across targets, but we only
need this to work in static pie (which is only supported on
a small number of targets as far as i know).

so i can try to tease _dl_relocate_static_pie apart into
'normal' reloc and 'plt' reloc processing (normally this
is done so that plt relocs can be lazy evaluated, but here
we would do it so IRELATIVE is processed in a separate step).

it still sounds like a big hack that relies on where
IRELATIVE is placed with current linkers and that there
are no other relocs we need to care about in static pie.
i can create a patch to see if it works.

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

[Siddhesh Poyarekar]

On 1/7/21 7:55 PM, Szabolcs Nagy via Libc-alpha wrote:
> it still sounds like a big hack that relies on where
> IRELATIVE is placed with current linkers and that there
> are no other relocs we need to care about in static pie.
> i can create a patch to see if it works.

This is probably a safe assumption for pointer based architectures but 
not for those with capabilities.  I have a feeling you might care about 
the latter ;)

Siddhesh

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

[Szabolcs Nagy]

The 01/07/2021 20:18, Siddhesh Poyarekar wrote:
> On 1/7/21 7:55 PM, Szabolcs Nagy via Libc-alpha wrote:
> > it still sounds like a big hack that relies on where
> > IRELATIVE is placed with current linkers and that there
> > are no other relocs we need to care about in static pie.
> > i can create a patch to see if it works.
> 
> This is probably a safe assumption for pointer based architectures but not
> for those with capabilities.  I have a feeling you might care about the
> latter ;)

well static pie is broken now, i care about that more.
we don't know yet how capability relocs will work with
static linking and ifuncs.

but the main problem with this idea is that i have
to copy a large part of the elf reloc code and hack
it apart to do static pie relocation (and that code
is not a nice piece of code in the first place).

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

[H.J. Lu]

On Thu, Jan 7, 2021 at 7:25 AM Szabolcs Nagy <szabolcs.nagy@arm.com> wrote:
>
> The 01/07/2021 20:18, Siddhesh Poyarekar wrote:
> > On 1/7/21 7:55 PM, Szabolcs Nagy via Libc-alpha wrote:
> > > it still sounds like a big hack that relies on where
> > > IRELATIVE is placed with current linkers and that there
> > > are no other relocs we need to care about in static pie.
> > > i can create a patch to see if it works.
> >
> > This is probably a safe assumption for pointer based architectures but not
> > for those with capabilities.  I have a feeling you might care about the
> > latter ;)
>
> well static pie is broken now, i care about that more.
> we don't know yet how capability relocs will work with
> static linking and ifuncs.
>
> but the main problem with this idea is that i have
> to copy a large part of the elf reloc code and hack
> it apart to do static pie relocation (and that code
> is not a nice piece of code in the first place).

We should call _dl_relocate_static_pie as early as possible
and delay others as much as we can.

-- 
H.J.

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

[H.J. Lu]

On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On targets where hidden symbol access does not need RELATIVE
> relocs, move the static pie self relocation after tunables and
> cpu features are set up.  This allows processing IRELATIVE
> relocs with correct ifunc dispatch logic.
>
> Unfortunately it is hard to guarantee that there will be no
> dynamic relocations in the early start up code, so this is a
> bit fragile. Ideally the RELATIVE relocs would be processed as
> early as possible and IRELATIVE relocs after cpu features are
> setup, but in glibc it is hard to separate them into two steps.
> ---
>  csu/libc-start.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index db859c3bed..b8d22bd59e 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -142,7 +142,10 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    int result;
>
>  #ifndef SHARED
> +# ifndef PI_STATIC_AND_HIDDEN
> +  /* Do static pie self relocation as early as possible.  */
>    _dl_relocate_static_pie ();
> +# endif

It should be simply removed. PI_STATIC_AND_HIDDEN should be
required for static PIE.

>    char **ev = &argv[argc + 1];
>
> @@ -191,6 +194,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),

Does
   {
      /* 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;
        }
    }

here require RELATIVE relocation?

>    ARCH_INIT_CPU_FEATURES ();
>
> +# ifdef PI_STATIC_AND_HIDDEN
> +  /* Do static pie self relocation after cpu features are setup.
> +     Code before this point must avoid relocations, which in practice
> +     means no initialized global pointer or ifunc symbol access.  */
> +  _dl_relocate_static_pie ();
> +# endif
> +
>    /* Perform IREL{,A} relocations.  */
>    ARCH_SETUP_IREL ();
>
> --
> 2.17.1
>


-- 
H.J.

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

[Szabolcs Nagy]

The 01/07/2021 13:03, H.J. Lu wrote:
> On Thu, Jan 7, 2021 at 3:04 AM Szabolcs Nagy via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >  #ifndef SHARED
> > +# ifndef PI_STATIC_AND_HIDDEN
> > +  /* Do static pie self relocation as early as possible.  */
> >    _dl_relocate_static_pie ();
> > +# endif
> 
> It should be simply removed. PI_STATIC_AND_HIDDEN should be
> required for static PIE.

i guess that makes sense, i can add an assertion for that.

> >    char **ev = &argv[argc + 1];
> >
> > @@ -191,6 +194,13 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
> 
> Does
>    {
>       /* 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;
>         }
>     }
> 
> here require RELATIVE relocation?

hm indeed __ehdr_start is accessed via GOT.
(i thought pc relative access would work, but
i guess that cannot work for an undefined
weak symbol.)

without relocation processing &__ehdr_start
will always evaluate to 0, i.e. as if it was
undefined.

but i think GL(dl_phdr) is only used much
later so it can be moved after self relocs.
that would also make it clear that we don't
use anything here from phdrs that might
require relocations.

i'll fix this.