diff mbox

S390: Use DT_JUMPREL in prelink undo code.

Message ID nl3haq$m5v$1@ger.gmane.org
State Committed
Headers

Commit Message

Stefan Liebler June 30, 2016, 4:28 p.m. UTC 
  Hi,

on s390, the current prelink undo code in elf_machine_lazy_rel()
has the requirement, that the plt stubs use the first got slots
after the 3 reserved ones.

In case of undoing prelink, the plt got slots are reset to the correct 
addresses whithin the corresponding plt-stub. Therefore the address
is calculated by the address of the first plt-stub-address which
was written by prelink (see l->l_mach.plt) to got[1] and index of 
current relocation multiplied with 32 (=size of one plt slot).
The index was calculated with &current-got-slot - &got[3].

This patch removes the requirement, that the plt-got-slots are
starting at got[3]. The index is now calculated with
&current-reloc - &reloc[0]. The first struct Elf64_Rela is stored
at DT_JMPREL.

This patch is needed to prepare for partial relro support.

Ulrich Weigand suggested this approach to use DT_JMPREL - Thanks.

Okay to commit?

Bye Stefan

ChangeLog:

	* sysdeps/s390/linkmap.h (struct link_map_machine):
	Remove member gotplt and add member jmprel.
	* sysdeps/s390/s390-32/dl-machine.h
	(elf_machine_runtime_setup): Setup member jmprel with DT_JMPREL
	instead of gotplt with &got[3].
	(elf_machine_lazy_rel): Calculate address with reloc and jmprel.
	* sysdeps/s390/s390-64/dl-machine.h: Likewise.

Comments

Stefan Liebler July 5, 2016, 7:24 a.m. UTC | #1 
PING

On 06/30/2016 06:28 PM, Stefan Liebler wrote:
> Hi,
>
> on s390, the current prelink undo code in elf_machine_lazy_rel()
> has the requirement, that the plt stubs use the first got slots
> after the 3 reserved ones.
>
> In case of undoing prelink, the plt got slots are reset to the correct
> addresses whithin the corresponding plt-stub. Therefore the address
> is calculated by the address of the first plt-stub-address which
> was written by prelink (see l->l_mach.plt) to got[1] and index of
> current relocation multiplied with 32 (=size of one plt slot).
> The index was calculated with &current-got-slot - &got[3].
>
> This patch removes the requirement, that the plt-got-slots are
> starting at got[3]. The index is now calculated with
> &current-reloc - &reloc[0]. The first struct Elf64_Rela is stored
> at DT_JMPREL.
>
> This patch is needed to prepare for partial relro support.
>
> Ulrich Weigand suggested this approach to use DT_JMPREL - Thanks.
>
> Okay to commit?
>
> Bye Stefan
>
> ChangeLog:
>
>      * sysdeps/s390/linkmap.h (struct link_map_machine):
>      Remove member gotplt and add member jmprel.
>      * sysdeps/s390/s390-32/dl-machine.h
>      (elf_machine_runtime_setup): Setup member jmprel with DT_JMPREL
>      instead of gotplt with &got[3].
>      (elf_machine_lazy_rel): Calculate address with reloc and jmprel.
>      * sysdeps/s390/s390-64/dl-machine.h: Likewise.
Florian Weimer July 5, 2016, 8:17 a.m. UTC | #2 
On 06/30/2016 06:28 PM, Stefan Liebler wrote:
>     * sysdeps/s390/linkmap.h (struct link_map_machine):
>     Remove member gotplt and add member jmprel.
>     * sysdeps/s390/s390-32/dl-machine.h
>     (elf_machine_runtime_setup): Setup member jmprel with DT_JMPREL
>     instead of gotplt with &got[3].
>     (elf_machine_lazy_rel): Calculate address with reloc and jmprel.
>     * sysdeps/s390/s390-64/dl-machine.h: Likewise.

I don't see anything obviously wrong with this patch (which doesn't mean 
much, it's hardly my area of expertise).

If you, as one of the architecture maintainers, think this is the right 
way forward, please commit it.

Thanks,
Florian
Stefan Liebler July 6, 2016, 1:28 p.m. UTC | #3 
On 07/05/2016 10:17 AM, Florian Weimer wrote:
> On 06/30/2016 06:28 PM, Stefan Liebler wrote:
>>     * sysdeps/s390/linkmap.h (struct link_map_machine):
>>     Remove member gotplt and add member jmprel.
>>     * sysdeps/s390/s390-32/dl-machine.h
>>     (elf_machine_runtime_setup): Setup member jmprel with DT_JMPREL
>>     instead of gotplt with &got[3].
>>     (elf_machine_lazy_rel): Calculate address with reloc and jmprel.
>>     * sysdeps/s390/s390-64/dl-machine.h: Likewise.
>
> I don't see anything obviously wrong with this patch (which doesn't mean
> much, it's hardly my area of expertise).
>
> If you, as one of the architecture maintainers, think this is the right
> way forward, please commit it.
>
> Thanks,
> Florian
>
Okay. I've pushed it and updated release-blockers section in wiki.
Carlos O'Donell July 6, 2016, 3:15 p.m. UTC | #4 
On 07/06/2016 09:28 AM, Stefan Liebler wrote:
> On 07/05/2016 10:17 AM, Florian Weimer wrote:
>> On 06/30/2016 06:28 PM, Stefan Liebler wrote:
>>>     * sysdeps/s390/linkmap.h (struct link_map_machine):
>>>     Remove member gotplt and add member jmprel.
>>>     * sysdeps/s390/s390-32/dl-machine.h
>>>     (elf_machine_runtime_setup): Setup member jmprel with DT_JMPREL
>>>     instead of gotplt with &got[3].
>>>     (elf_machine_lazy_rel): Calculate address with reloc and jmprel.
>>>     * sysdeps/s390/s390-64/dl-machine.h: Likewise.
>>
>> I don't see anything obviously wrong with this patch (which doesn't mean
>> much, it's hardly my area of expertise).
>>
>> If you, as one of the architecture maintainers, think this is the right
>> way forward, please commit it.
>>
>> Thanks,
>> Florian
>>
> Okay. I've pushed it and updated release-blockers section in wiki.
> 

Thanks for pushing this. This looks good to me.

I reviewed what you did and it makes sense.
diff mbox

Patch

diff --git a/sysdeps/s390/linkmap.h b/sysdeps/s390/linkmap.h
index fc1fba3..283615b 100644
--- a/sysdeps/s390/linkmap.h
+++ b/sysdeps/s390/linkmap.h
@@ -2,12 +2,12 @@ 
 struct link_map_machine
   {
     Elf64_Addr plt; /* Address of .plt + 0x2e */
-    Elf64_Addr gotplt; /* Address of .got + 0x18 */
+    const Elf64_Rela *jmprel; /* Address of first JMP_SLOT reloc */
   };
 #else
 struct link_map_machine
   {
     Elf32_Addr plt; /* Address of .plt + 0x2c */
-    Elf32_Addr gotplt; /* Address of .got + 0x0c */
+    const Elf32_Rela *jmprel; /* Address of first JMP_SLOT reloc */
   };
 #endif
diff --git a/sysdeps/s390/s390-32/dl-machine.h b/sysdeps/s390/s390-32/dl-machine.h
index ec0ae4a..0a58897 100644
--- a/sysdeps/s390/s390-32/dl-machine.h
+++ b/sysdeps/s390/s390-32/dl-machine.h
@@ -109,7 +109,7 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
       if (got[1])
 	{
 	  l->l_mach.plt = got[1] + l->l_addr;
-	  l->l_mach.gotplt = (Elf32_Addr) &got[3];
+	  l->l_mach.jmprel = (const Elf32_Rela *) D_PTR (l, l_info[DT_JMPREL]);
 	}
       got[1] = (Elf32_Addr) l;	/* Identify this shared object.  */
 
@@ -506,9 +506,7 @@  elf_machine_lazy_rel (struct link_map *map,
       if (__builtin_expect (map->l_mach.plt, 0) == 0)
 	*reloc_addr += l_addr;
       else
-	*reloc_addr =
-	  map->l_mach.plt
-	  + (((Elf32_Addr) reloc_addr) - map->l_mach.gotplt) * 8;
+	*reloc_addr = map->l_mach.plt + (reloc - map->l_mach.jmprel) * 32;
     }
   else if (__glibc_likely (r_type == R_390_IRELATIVE))
     {
diff --git a/sysdeps/s390/s390-64/dl-machine.h b/sysdeps/s390/s390-64/dl-machine.h
index 9ee7c92..6e5ee1e 100644
--- a/sysdeps/s390/s390-64/dl-machine.h
+++ b/sysdeps/s390/s390-64/dl-machine.h
@@ -98,7 +98,7 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
       if (got[1])
 	{
 	  l->l_mach.plt = got[1] + l->l_addr;
-	  l->l_mach.gotplt = (Elf64_Addr) &got[3];
+	  l->l_mach.jmprel = (const Elf64_Rela *) D_PTR (l, l_info[DT_JMPREL]);
 	}
       got[1] = (Elf64_Addr) l;	/* Identify this shared object.	 */
 
@@ -460,9 +460,7 @@  elf_machine_lazy_rel (struct link_map *map,
       if (__builtin_expect (map->l_mach.plt, 0) == 0)
 	*reloc_addr += l_addr;
       else
-	*reloc_addr =
-	  map->l_mach.plt
-	  + (((Elf64_Addr) reloc_addr) - map->l_mach.gotplt) * 4;
+	*reloc_addr = map->l_mach.plt + (reloc - map->l_mach.jmprel) * 32;
     }
   else if (__glibc_likely (r_type == R_390_IRELATIVE))
     {