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