x86: Check IFUNC definition in unrelocated executable [BZ #20019]

Message ID 20201228141102.2562718-1-hjl.tools@gmail.com
State Superseded
Delegated to: Carlos O'Donell
Headers
Series x86: Check IFUNC definition in unrelocated executable [BZ #20019] |

Commit Message

H.J. Lu Dec. 28, 2020, 2:11 p.m. UTC
  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.
---
 sysdeps/i386/dl-machine.h   | 15 ++++++++++-----
 sysdeps/x86_64/dl-machine.h | 15 ++++++++++-----
 2 files changed, 20 insertions(+), 10 deletions(-)
  

Comments

Carlos O'Donell Jan. 4, 2021, 6:47 p.m. UTC | #1
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."

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) ();
>
  

Patch

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
 	  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
 	  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 = ((ElfW(Addr) (*) (void)) value) ();