[PATCH] m68k: Handle fewer relocations for RTLD_BOOTSTRAP (#BZ29071)

[PATCH] m68k: Handle fewer relocations for RTLD_BOOTSTRAP (#BZ29071)

[Fangrui Song]

m68k is a non-PI_STATIC_AND_HIDDEN arch which uses a GOT relocation when
loading a jump table. The GOT load may be reordered before processing
R_68K_RELATIVE relocations, leading to an unrelocated/incorrect jump
table. The foolproof approach is to add an optimization barrier (e.g.
calling a function after relative relocations are resolved). That isn't
suitable given the current code structure, so just use the simple
approach to avoid the jump table: handle only the essential reloctions
for RTLD_BOOTSTRAP code.

This is based on Andreas Schwab's patch and fixed ld.so crash on m68k.
---
 sysdeps/m68k/dl-machine.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sysdeps/m68k/dl-machine.h b/sysdeps/m68k/dl-machine.h
index c44ab055aa..bb51b4198c 100644
--- a/sysdeps/m68k/dl-machine.h
+++ b/sysdeps/m68k/dl-machine.h
@@ -234,6 +234,11 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
 
       switch (r_type)
 	{
+	case R_68K_GLOB_DAT:
+	case R_68K_JMP_SLOT:
+	  *reloc_addr = value;
+	  break;
+#ifndef RTLD_BOOTSTRAP
 	case R_68K_COPY:
 	  if (sym == NULL)
 	    /* This can happen in trace mode if an object could not be
@@ -252,10 +257,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
 	  memcpy (reloc_addr_arg, (void *) value,
 		  MIN (sym->st_size, refsym->st_size));
 	  break;
-	case R_68K_GLOB_DAT:
-	case R_68K_JMP_SLOT:
-	  *reloc_addr = value;
-	  break;
 	case R_68K_8:
 	  *(char *) reloc_addr = value + reloc->r_addend;
 	  break;
@@ -276,7 +277,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
 	case R_68K_PC32:
 	  *reloc_addr = value + reloc->r_addend - (Elf32_Addr) reloc_addr;
 	  break;
-#ifndef RTLD_BOOTSTRAP
 	case R_68K_TLS_DTPMOD32:
 	  /* Get the information from the link map returned by the
 	     resolv function.  */
@@ -294,9 +294,9 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
 	      *reloc_addr = TLS_TPREL_VALUE (sym_map, sym, reloc);
 	    }
 	  break;
-#endif /* !RTLD_BOOTSTRAP */
 	case R_68K_NONE:		/* Alright, Wilbur.  */
 	  break;
+#endif /* !RTLD_BOOTSTRAP */
 	default:
 	  _dl_reloc_bad_type (map, r_type, 0);
 	  break;
-- 
2.36.0.rc0.470.gd361397f0d-goog

Re: [PATCH] m68k: Handle fewer relocations for RTLD_BOOTSTRAP (#BZ29071)

[Adhemerval Zanella]

On 19/04/2022 23:35, Fangrui Song wrote:
> m68k is a non-PI_STATIC_AND_HIDDEN arch which uses a GOT relocation when
> loading a jump table. The GOT load may be reordered before processing
> R_68K_RELATIVE relocations, leading to an unrelocated/incorrect jump
> table. The foolproof approach is to add an optimization barrier (e.g.
> calling a function after relative relocations are resolved). That isn't
> suitable given the current code structure, so just use the simple
> approach to avoid the jump table: handle only the essential reloctions
> for RTLD_BOOTSTRAP code.
> 
> This is based on Andreas Schwab's patch and fixed ld.so crash on m68k.

LGTM, thanks.  The loader does not triggers a segfault anymore.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  sysdeps/m68k/dl-machine.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/sysdeps/m68k/dl-machine.h b/sysdeps/m68k/dl-machine.h
> index c44ab055aa..bb51b4198c 100644
> --- a/sysdeps/m68k/dl-machine.h
> +++ b/sysdeps/m68k/dl-machine.h
> @@ -234,6 +234,11 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  
>        switch (r_type)
>  	{
> +	case R_68K_GLOB_DAT:
> +	case R_68K_JMP_SLOT:
> +	  *reloc_addr = value;
> +	  break;
> +#ifndef RTLD_BOOTSTRAP
>  	case R_68K_COPY:
>  	  if (sym == NULL)
>  	    /* This can happen in trace mode if an object could not be
> @@ -252,10 +257,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  	  memcpy (reloc_addr_arg, (void *) value,
>  		  MIN (sym->st_size, refsym->st_size));
>  	  break;
> -	case R_68K_GLOB_DAT:
> -	case R_68K_JMP_SLOT:
> -	  *reloc_addr = value;
> -	  break;
>  	case R_68K_8:
>  	  *(char *) reloc_addr = value + reloc->r_addend;
>  	  break;
> @@ -276,7 +277,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  	case R_68K_PC32:
>  	  *reloc_addr = value + reloc->r_addend - (Elf32_Addr) reloc_addr;
>  	  break;
> -#ifndef RTLD_BOOTSTRAP
>  	case R_68K_TLS_DTPMOD32:
>  	  /* Get the information from the link map returned by the
>  	     resolv function.  */
> @@ -294,9 +294,9 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>  	      *reloc_addr = TLS_TPREL_VALUE (sym_map, sym, reloc);
>  	    }
>  	  break;
> -#endif /* !RTLD_BOOTSTRAP */
>  	case R_68K_NONE:		/* Alright, Wilbur.  */
>  	  break;
> +#endif /* !RTLD_BOOTSTRAP */
>  	default:
>  	  _dl_reloc_bad_type (map, r_type, 0);
>  	  break;