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