V2 [PATCH] x86: Check IFUNC definition in unrelocated executable [BZ #20019]
H.J. Lu
hjl.tools@gmail.com
Mon Jan 4 22:57:24 GMT 2021
On Mon, Jan 4, 2021 at 11:59 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Jan 4, 2021 at 11:50 AM Carlos O'Donell <carlos@redhat.com> wrote:
> >
> > On 1/4/21 2:34 PM, H.J. Lu wrote:
> > > On Mon, Jan 4, 2021 at 10:47 AM Carlos O'Donell <carlos@redhat.com> wrote:
> > >>
> > >> On 12/28/20 9:11 AM, H.J. Lu via Libc-alpha wrote:
> > >>> Calling an IFUNC function defined in unrelocated executable may also
> > >>> lead to segfault. Issue an error message when calling IFUNC function
> > >>> defined in the unrelocated executable from a shared library.
> > >>
> > >> The logic here makes sense, but we need a stronger error message.
> > >>
> > >> Please review my understanding and suggested error message.
> > >>
> > >> Looking forward to v2.
> > >>
> > >>> ---
> > >>> sysdeps/i386/dl-machine.h | 15 ++++++++++-----
> > >>> sysdeps/x86_64/dl-machine.h | 15 ++++++++++-----
> > >>> 2 files changed, 20 insertions(+), 10 deletions(-)
> > >>>
> > >>> diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
> > >>> index fea9e579ec..dedda484ba 100644
> > >>> --- a/sysdeps/i386/dl-machine.h
> > >>> +++ b/sysdeps/i386/dl-machine.h
> > >>> @@ -337,16 +337,21 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
> > >>> {
> > >>> # ifndef RTLD_BOOTSTRAP
> > >>
> > >> OK. Logic is in the correct place in dl-machine.h for i386.
> > >>
> > >>> if (sym_map != map
> > >>> - && sym_map->l_type != lt_executable
> > >>> && !sym_map->l_relocated)
> > >>> {
> > >>> const char *strtab
> > >>> = (const char *) D_PTR (map, l_info[DT_STRTAB]);
> > >>> - _dl_error_printf ("\
> > >>> + if (sym_map->l_type == lt_executable)
> > >>> + _dl_error_printf ("\
> > >>> +%s: IFUNC symbol `%s' referenced in `%s' is defined in executable\n",
> > >>> + RTLD_PROGNAME, strtab + refsym->st_name,
> > >>> + map->l_name);
> > >>> + else
> > >>> + _dl_error_printf ("\
> > >>> %s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
> > >>> - RTLD_PROGNAME, map->l_name,
> > >>> - sym_map->l_name,
> > >>> - strtab + refsym->st_name);
> > >>> + RTLD_PROGNAME, map->l_name,
> > >>> + sym_map->l_name,
> > >>> + strtab + refsym->st_name);
> > >>> }
> > >>> # endif
> > >>> value = ((Elf32_Addr (*) (void)) value) ();
> > >>> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> > >>> index bb93c7c6ab..fc847f4bc2 100644
> > >>> --- a/sysdeps/x86_64/dl-machine.h
> > >>> +++ b/sysdeps/x86_64/dl-machine.h
> > >>> @@ -314,16 +314,21 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
> > >>> {
> > >>> # ifndef RTLD_BOOTSTRAP
> > >>
> > >> OK. Logic is in the correct place in dl-machine.h for x86_64.
> > >>
> > >>> if (sym_map != map
> > >>> - && sym_map->l_type != lt_executable
> > >>> && !sym_map->l_relocated)
> > >>> {
> > >>> const char *strtab
> > >>> = (const char *) D_PTR (map, l_info[DT_STRTAB]);
> > >>> - _dl_error_printf ("\
> > >>> + if (sym_map->l_type == lt_executable)
> > >>> + _dl_error_printf ("\
> > >>> +%s: IFUNC symbol `%s' referenced in `%s' is defined in executable\n",
> > >>
> > >> The message should explain the error
> > >> e.g. "Such and such *must not* reference such and such."
> > >>
> > >> Or the message should explain how to fix the error (as the other does)
> > >> e.g. "Such and such must be relinked with such and such."
> > >>
> > >> We have made this a hard error. An executable with immediate binding
> > >> may not define an IFUNC resolver and implementation that is used from
> > >> a shared library since it creates an ordering issue with the dependent
> > >> libraries that use the resolution of the symbol i.e. you must initialize
> > >> the executable but to do that you must initialize the libraries, but to
> > >> do that you must initialize the executable etc. etc.
> > >>
> > >> In which case the error message could be:
> > >>
> > >> "%s: IFUNC symbol '%s' referenced in '%s' is defined in the executable
> > >> and creates an unsatisfiable circular dependency."
> > >
> > > Fixed.
> > >
> > >> Note: Use '' quotes not `' since the GNU Coding standards have changed.
> > >> https://www.gnu.org/prep/standards/standards.html#Quote-Characters
> > >>
> > >>> + RTLD_PROGNAME, strtab + refsym->st_name,
> > >>> + map->l_name);
> > >>> + else
> > >>> + _dl_error_printf ("\
> > >>> %s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
> > >>> - RTLD_PROGNAME, map->l_name,
> > >>> - sym_map->l_name,
> > >>> - strtab + refsym->st_name);
> > >>> + RTLD_PROGNAME, map->l_name,
> > >>> + sym_map->l_name,
> > >>> + strtab + refsym->st_name);
> > >>> }
> > >>> # endif
> > >>> value = ((ElfW(Addr) (*) (void)) value) ();
> > >>>
> > >>
> > >>
> > >
> > > Here is the updated patch. Changes from V1:
> > >
> > > 1. Update the error message based on feedback from Carlos.
> > > 2. Make the error fatal instead of segfault later.
> > >
> > > OK for master?
> >
> > Could binutils have given the user a better warnings?
>
> I will take a look.
>
The problem is
0000000000003fe8 0000000300000006 R_X86_64_GLOB_DAT
0000000000000000 foo + 0
0000000000004018 0000000300000001 R_X86_64_64
0000000000000000 foo + 0
in ifuncmod6.so. When linker creates ifuncmod6.so, linker doesn't
know that foo will
be an IFUNC symbol defined in executable at run-time. When linker
creates ifuncmain6pie,
linker doesn't check dynamic relocations in ifuncmod6.so and
ifuncmod6.so used in link-time
can be different from run-time.
So there is not much linker can do.
H.J.
More information about the Libc-alpha
mailing list