rtld: Add new hook to initialize dp register on hppa

Message ID YgadD3LNNw8Xhmuh@mx3210.localdomain
State Committed
Headers
Series rtld: Add new hook to initialize dp register on hppa |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

John David Anglin Feb. 11, 2022, 5:29 p.m. UTC
  The test elf/tst-audit2 fails on hppa with a segmentation fault in the
long branch stub used to call malloc from calloc.  This occur because
the test is not a PIC executable and calloc is called from the dynamic
linker before the dp register is initialized in _dl_start_user.

The fix is to add a new hook, ELF_MACHINE_AFTER_RTLD_RELOC, in dl_main
to allow initialization of the dp register just after relocation is
completed.

Okay?

Dave
---
  

Comments

Florian Weimer Feb. 11, 2022, 5:51 p.m. UTC | #1
* John David Anglin:

> @@ -338,16 +350,6 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
>     its return value is the user program's entry point.  */
>  
>  #define RTLD_START \
> -/* Set up dp for any non-PIC lib constructors that may be called.  */	\
> -static struct link_map * __attribute__((used))				\
> -set_dp (struct link_map *map)						\
> -{									\
> -  register Elf32_Addr dp asm ("%r27");					\
> -  dp = D_PTR (map, l_info[DT_PLTGOT]);					\
> -  asm volatile ("" : : "r" (dp));					\
> -  return map;								\
> -}									\
> -									\
>  asm (									\
>  "	.text\n"							\
>  "	.globl _start\n"						\
> @@ -447,14 +449,11 @@ asm (									\
>  "	stw	%r24,-44(%sp)\n"					\
>  									\
>  ".Lnofix:\n"								\
> +	/* Call _dl_init(main_map, argc, argv, envp). */		\
>  "	addil	LT'_rtld_local,%r19\n"					\
>  "	ldw	RT'_rtld_local(%r1),%r26\n"				\
> -"	bl	set_dp, %r2\n"						\
>  "	ldw	0(%r26),%r26\n"						\
>  									\
> -	/* Call _dl_init(_dl_loaded, argc, argv, envp). */		\
> -"	copy	%r28,%r26\n"						\
> -									\
>  	/* envp = argv + argc + 1 */					\
>  "	sh2add	%r25,%r24,%r23\n"					\
>  "	bl	_dl_init,%r2\n"						\

Can't you compute the PLTGOT address using a relocation that is filled
in by the link editor, before calling _dl_start?  The link editor should
know the offset between the current code access and the start of the
PLTGOT.

Thanks,
Florian
  
H.J. Lu Feb. 11, 2022, 6:17 p.m. UTC | #2
On Fri, Feb 11, 2022 at 9:30 AM John David Anglin <dave.anglin@bell.net> wrote:
>
> The test elf/tst-audit2 fails on hppa with a segmentation fault in the
> long branch stub used to call malloc from calloc.  This occur because
> the test is not a PIC executable and calloc is called from the dynamic
> linker before the dp register is initialized in _dl_start_user.
>
> The fix is to add a new hook, ELF_MACHINE_AFTER_RTLD_RELOC, in dl_main
> to allow initialization of the dp register just after relocation is
> completed.
>
> Okay?
>
> Dave
> ---
>
> diff --git a/elf/rtld.c b/elf/rtld.c
> index 19e328f89e..fcea41cb86 100644
> --- a/elf/rtld.c
> +++ b/elf/rtld.c
> @@ -2333,6 +2333,10 @@ dl_main (const ElfW(Phdr) *phdr,
>    /* Make sure no new search directories have been added.  */
>    assert (GLRO(dl_init_all_dirs) == GL(dl_all_dirs));
>
> +#ifdef ELF_MACHINE_AFTER_RTLD_RELOC
> +  ELF_MACHINE_AFTER_RTLD_RELOC (main_map);
> +#endif

We should avoid macro  Please use an inline function instead.

>    if (rtld_multiple_ref)
>      {
>        /* There was an explicit ref to the dynamic linker as a shared lib.
> diff --git a/sysdeps/hppa/dl-machine.h b/sysdeps/hppa/dl-machine.h
> index da4d57d2e4..74a81dc1f7 100644
> --- a/sysdeps/hppa/dl-machine.h
> +++ b/sysdeps/hppa/dl-machine.h
> @@ -70,6 +70,18 @@ __hppa_init_bootstrap_fdesc_table (struct link_map *map)
>    map->l_mach.fptr_table = boot_table;
>  }
>
> +/* Set up dp for any non-PIC lib functions that may be called.  */
> +static inline void
> +set_dp (struct link_map *map)
> +{
> +  register Elf32_Addr dp asm ("%r27");
> +  dp = D_PTR (map, l_info[DT_PLTGOT]);
> +  asm volatile ("" : : "r" (dp));
> +}
> +
> +#define ELF_MACHINE_AFTER_RTLD_RELOC(map)                      \
> +       set_dp (map);
> +
>  #define ELF_MACHINE_BEFORE_RTLD_RELOC(map, dynamic_info)       \
>         __hppa_init_bootstrap_fdesc_table (map);                \
>         _dl_fptr_init();
> @@ -338,16 +350,6 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
>     its return value is the user program's entry point.  */
>
>  #define RTLD_START \
> -/* Set up dp for any non-PIC lib constructors that may be called.  */  \
> -static struct link_map * __attribute__((used))                         \
> -set_dp (struct link_map *map)                                          \
> -{                                                                      \
> -  register Elf32_Addr dp asm ("%r27");                                 \
> -  dp = D_PTR (map, l_info[DT_PLTGOT]);                                 \
> -  asm volatile ("" : : "r" (dp));                                      \
> -  return map;                                                          \
> -}                                                                      \
> -                                                                       \
>  asm (                                                                  \
>  "      .text\n"                                                        \
>  "      .globl _start\n"                                                \
> @@ -447,14 +449,11 @@ asm (                                                                     \
>  "      stw     %r24,-44(%sp)\n"                                        \
>                                                                         \
>  ".Lnofix:\n"                                                           \
> +       /* Call _dl_init(main_map, argc, argv, envp). */                \
>  "      addil   LT'_rtld_local,%r19\n"                                  \
>  "      ldw     RT'_rtld_local(%r1),%r26\n"                             \
> -"      bl      set_dp, %r2\n"                                          \
>  "      ldw     0(%r26),%r26\n"                                         \
>                                                                         \
> -       /* Call _dl_init(_dl_loaded, argc, argv, envp). */              \
> -"      copy    %r28,%r26\n"                                            \
> -                                                                       \
>         /* envp = argv + argc + 1 */                                    \
>  "      sh2add  %r25,%r24,%r23\n"                                       \
>  "      bl      _dl_init,%r2\n"                                         \
>
  
John David Anglin Feb. 11, 2022, 6:59 p.m. UTC | #3
On 2022-02-11 12:51 p.m., Florian Weimer wrote:
> * John David Anglin:
>
>> @@ -338,16 +350,6 @@ elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
>>      its return value is the user program's entry point.  */
>>   
>>   #define RTLD_START \
>> -/* Set up dp for any non-PIC lib constructors that may be called.  */	\
>> -static struct link_map * __attribute__((used))				\
>> -set_dp (struct link_map *map)						\
>> -{									\
>> -  register Elf32_Addr dp asm ("%r27");					\
>> -  dp = D_PTR (map, l_info[DT_PLTGOT]);					\
>> -  asm volatile ("" : : "r" (dp));					\
>> -  return map;								\
>> -}									\
>> -									\
>>   asm (									\
>>   "	.text\n"							\
>>   "	.globl _start\n"						\
>> @@ -447,14 +449,11 @@ asm (									\
>>   "	stw	%r24,-44(%sp)\n"					\
>>   									\
>>   ".Lnofix:\n"								\
>> +	/* Call _dl_init(main_map, argc, argv, envp). */		\
>>   "	addil	LT'_rtld_local,%r19\n"					\
>>   "	ldw	RT'_rtld_local(%r1),%r26\n"				\
>> -"	bl	set_dp, %r2\n"						\
>>   "	ldw	0(%r26),%r26\n"						\
>>   									\
>> -	/* Call _dl_init(_dl_loaded, argc, argv, envp). */		\
>> -"	copy	%r28,%r26\n"						\
>> -									\
>>   	/* envp = argv + argc + 1 */					\
>>   "	sh2add	%r25,%r24,%r23\n"					\
>>   "	bl	_dl_init,%r2\n"						\
> Can't you compute the PLTGOT address using a relocation that is filled
> in by the link editor, before calling _dl_start?  The link editor should
> know the offset between the current code access and the start of the
> PLTGOT.
I'm not sure I like this at all.  What relocation?  Even if there was a relocation, it appears this
would make the behavior dependent on the link editor and thousands of applications would
need relinking.

Loading dp using from _rtld_local doesn't work till relocations are done.  Is there a way to compute main_map
earlier?

The suggested approach just moves the dp register initialization earlier and doesn't affect any applications.

Dave
  
Florian Weimer Feb. 11, 2022, 7:07 p.m. UTC | #4
* John David Anglin:


> I'm not sure I like this at all.  What relocation?  Even if there was
> a relocation, it appears this would make the behavior dependent on the
> link editor and thousands of applications would need relinking.

I meant to use this link-time relocation in _start of ld.so.

> Loading dp using from _rtld_local doesn't work till relocations are
> done.  Is there a way to compute main_map earlier?

You shouldn't need main_map for this.  The link editor computes the
DT_PLTGOT and therefore knows the correct (relative) address.

Thanks,
Florian
  
John David Anglin Feb. 14, 2022, 3:25 p.m. UTC | #5
On 2022-02-11 2:07 p.m., Florian Weimer wrote:
>> Loading dp using from _rtld_local doesn't work till relocations are
>> done.  Is there a way to compute main_map earlier?
> You shouldn't need main_map for this.  The link editor computes the
> DT_PLTGOT and therefore knows the correct (relative) address.
Your are correct.  The link editor does define the symbol $global$ at the start of the PLTGOT.
However, I couldn't find a way to load its address without rummaging through the symbol table
of main.  This is less efficient than using the PLTGOT value from the ELF header and main_map.

I committed the attached change.  It doesn't need a new hook.

Thanks,
Dave
  

Patch

diff --git a/elf/rtld.c b/elf/rtld.c
index 19e328f89e..fcea41cb86 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -2333,6 +2333,10 @@  dl_main (const ElfW(Phdr) *phdr,
   /* Make sure no new search directories have been added.  */
   assert (GLRO(dl_init_all_dirs) == GL(dl_all_dirs));
 
+#ifdef ELF_MACHINE_AFTER_RTLD_RELOC
+  ELF_MACHINE_AFTER_RTLD_RELOC (main_map);
+#endif
+
   if (rtld_multiple_ref)
     {
       /* There was an explicit ref to the dynamic linker as a shared lib.
diff --git a/sysdeps/hppa/dl-machine.h b/sysdeps/hppa/dl-machine.h
index da4d57d2e4..74a81dc1f7 100644
--- a/sysdeps/hppa/dl-machine.h
+++ b/sysdeps/hppa/dl-machine.h
@@ -70,6 +70,18 @@  __hppa_init_bootstrap_fdesc_table (struct link_map *map)
   map->l_mach.fptr_table = boot_table;
 }
 
+/* Set up dp for any non-PIC lib functions that may be called.  */
+static inline void
+set_dp (struct link_map *map)
+{
+  register Elf32_Addr dp asm ("%r27");
+  dp = D_PTR (map, l_info[DT_PLTGOT]);
+  asm volatile ("" : : "r" (dp));
+}
+
+#define ELF_MACHINE_AFTER_RTLD_RELOC(map)			\
+	set_dp (map);
+
 #define ELF_MACHINE_BEFORE_RTLD_RELOC(map, dynamic_info)	\
 	__hppa_init_bootstrap_fdesc_table (map);		\
 	_dl_fptr_init();
@@ -338,16 +350,6 @@  elf_machine_runtime_setup (struct link_map *l, struct r_scope_elem *scope[],
    its return value is the user program's entry point.  */
 
 #define RTLD_START \
-/* Set up dp for any non-PIC lib constructors that may be called.  */	\
-static struct link_map * __attribute__((used))				\
-set_dp (struct link_map *map)						\
-{									\
-  register Elf32_Addr dp asm ("%r27");					\
-  dp = D_PTR (map, l_info[DT_PLTGOT]);					\
-  asm volatile ("" : : "r" (dp));					\
-  return map;								\
-}									\
-									\
 asm (									\
 "	.text\n"							\
 "	.globl _start\n"						\
@@ -447,14 +449,11 @@  asm (									\
 "	stw	%r24,-44(%sp)\n"					\
 									\
 ".Lnofix:\n"								\
+	/* Call _dl_init(main_map, argc, argv, envp). */		\
 "	addil	LT'_rtld_local,%r19\n"					\
 "	ldw	RT'_rtld_local(%r1),%r26\n"				\
-"	bl	set_dp, %r2\n"						\
 "	ldw	0(%r26),%r26\n"						\
 									\
-	/* Call _dl_init(_dl_loaded, argc, argv, envp). */		\
-"	copy	%r28,%r26\n"						\
-									\
 	/* envp = argv + argc + 1 */					\
 "	sh2add	%r25,%r24,%r23\n"					\
 "	bl	_dl_init,%r2\n"						\