| Message ID | b5dbaa167f4317c312e22fb761036d3570e6614a.1611155254.git.szabolcs.nagy@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | fix ifunc with static pie [BZ #27072] | expand |
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.
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.
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 ();