diff mbox series

x86-64 prelink support for TLS dialect gnu2

Message ID CAMZr8UrsztX+n5id7cTCR=QD93bHDK7=yj2_ujn0tAbEdkGngQ@mail.gmail.com
State New
Headers show
Series x86-64 prelink support for TLS dialect gnu2 | expand

Commit Message

Steven Newbury Nov. 5, 2018, 4:21 p.m. UTC 
I'm working on gettting TLS gnu2 dialect support working for x86-64
with prelink.  Yes, it's still maintained, see:
https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/

I've written the attached patch which enables prelink to recognise
R_X86_64_TLSDESC relocations and do the same as is done currently for
ARM.  Unfortunately, while this seems to work, when a prelinked binary
is loaded with a shared library with such a relocation it fails with:

  "error while loading shared libraries: unexpected reloc type 0x24".

reloc type 0x24 is indeed R_X86_64_TLSDESC.

The strange thing is the code in glibc which generates this error
(sysdeps/x86_64/dl-machine.h) specifically handles R_X86_64_TLSDESC.
I can't understand how it is falling through to _dl_reloc_bad_type ()
with reloc 0x24 since in both instances the error case shouldn't be
reached!

Can anybody help?

Comments

Andreas Schwab Nov. 5, 2018, 5:02 p.m. UTC | #1 
On Nov 05 2018, Steven Newbury <steven.newbury@googlemail.com> wrote:

> The strange thing is the code in glibc which generates this error
> (sysdeps/x86_64/dl-machine.h) specifically handles R_X86_64_TLSDESC.
> I can't understand how it is falling through to _dl_reloc_bad_type ()
> with reloc 0x24 since in both instances the error case shouldn't be
> reached!

Are you sure you are running the patched ld.so?

Andreas.
Steven Newbury Nov. 5, 2018, 5:13 p.m. UTC | #2 
ld.so isn't (yet) patched.  The patch is for prelink, and mirrors what
is already in
place for ARM.  The ld.so appears, to me at least, to already have all the
support in place, although possibly never tested.  Up to now prelink just failed
on tls-dialect=gnu2 with an "Unsupported relocation type" error.  This only
applies of course objects ending up with type 0x24 relocations, like libstdc++!!
;-)



On Mon, 5 Nov 2018 at 17:02, Andreas Schwab <schwab@suse.de> wrote:
>
> On Nov 05 2018, Steven Newbury <steven.newbury@googlemail.com> wrote:
>
> > The strange thing is the code in glibc which generates this error
> > (sysdeps/x86_64/dl-machine.h) specifically handles R_X86_64_TLSDESC.
> > I can't understand how it is falling through to _dl_reloc_bad_type ()
> > with reloc 0x24 since in both instances the error case shouldn't be
> > reached!
>
> Are you sure you are running the patched ld.so?
>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Szabolcs Nagy Nov. 5, 2018, 6:02 p.m. UTC | #3 
On 05/11/18 16:21, Steven Newbury wrote:
> I'm working on gettting TLS gnu2 dialect support working for x86-64
> with prelink.  Yes, it's still maintained, see:
> https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/
> 

prelinking for tlsdesc was never supported in ld.so
on any target.

at some point arm had support in case of lazy binding,
but now lazy binding of tlsdesc is disabled on arm
so that code got removed:

https://sourceware.org/ml/libc-alpha/2017-10/msg01121.html

> I've written the attached patch which enables prelink to recognise
> R_X86_64_TLSDESC relocations and do the same as is done currently for
> ARM.  Unfortunately, while this seems to work, when a prelinked binary
> is loaded with a shared library with such a relocation it fails with:
> 
>   "error while loading shared libraries: unexpected reloc type 0x24".
> 
> reloc type 0x24 is indeed R_X86_64_TLSDESC.
> 
> The strange thing is the code in glibc which generates this error
> (sysdeps/x86_64/dl-machine.h) specifically handles R_X86_64_TLSDESC.
> I can't understand how it is falling through to _dl_reloc_bad_type ()
> with reloc 0x24 since in both instances the error case shouldn't be
> reached!
> 
> Can anybody help?
>
Steven Newbury Nov. 5, 2018, 7:32 p.m. UTC | #4 
On 05/11/2018, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
> On 05/11/18 16:21, Steven Newbury wrote:
>> I'm working on gettting TLS gnu2 dialect support working for x86-64
>> with prelink.  Yes, it's still maintained, see:
>> https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/
>>
>
> prelinking for tlsdesc was never supported in ld.so
> on any target.
>
> at some point arm had support in case of lazy binding,
> but now lazy binding of tlsdesc is disabled on arm
> so that code got removed:
>
I've been comparing the removed code to what is currently in the
x86_64 dl-machine.h and it's almost identical, which makes sense since
x86_64 still supports lazy binding of tlsdesc AFAIK.  The only notable
difference that I see is:

Removed ARM support code

-  if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy)
-    *(Elf32_Addr*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->l_addr)
-      = (Elf32_Addr) &_dl_tlsdesc_lazy_resolver;

Currently in x86_64

  if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy)
    *(ElfW(Addr)*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->l_addr)
      = (ElfW(Addr)) &_dl_tlsdesc_resolve_rela;

Am I looking in the right place?
Steven Newbury Nov. 6, 2018, 6:24 a.m. UTC | #5 
On Mon, 5 Nov 2018, 19:32 Steven Newbury <steven.newbury@googlemail.com
wrote:

> On 05/11/2018, Szabolcs Nagy <Szabolcs.Nagy@arm.com> wrote:
> > On 05/11/18 16:21, Steven Newbury wrote:
> >> I'm working on gettting TLS gnu2 dialect support working for x86-64
> >> with prelink.  Yes, it's still maintained, see:
> >> https://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/
> >>
> >
> > prelinking for tlsdesc was never supported in ld.so
> > on any target.
> >
> > at some point arm had support in case of lazy binding,
> > but now lazy binding of tlsdesc is disabled on arm
> > so that code got removed:
> >
> I've been comparing the removed code to what is currently in the
> x86_64 dl-machine.h and it's almost identical, which makes sense since
> x86_64 still supports lazy binding of tlsdesc AFAIK.  The only notable
> difference that I see is:
>
> Removed ARM support code
>
> -  if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy)
> -    *(Elf32_Addr*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) +
> l->l_addr)
> -      = (Elf32_Addr) &_dl_tlsdesc_lazy_resolver;
>
> Currently in x86_64
>
>   if (l->l_info[ADDRIDX (DT_TLSDESC_GOT)] && lazy)
>     *(ElfW(Addr)*)(D_PTR (l, l_info[ADDRIDX (DT_TLSDESC_GOT)]) + l->l_addr)
>       = (ElfW(Addr)) &_dl_tlsdesc_resolve_rela;


>
> Am I looking in the right place?
>
 I think not, since that snippet predates the prelink support.

From the reverted ARM prelink patch it's clear the magic is in the
section guarded
by RESOLVE_CONFLICT_FIND_MAP.  The x86_64 code already handles several
different cases of R_X86_64_TLSDESC in that switch so I still don't
understand how it falls through to _dl_reloc_bad_type rather than just
ignoring the prelink suppliied address.  Most of the ARM patch
consists of asserts presumably because the code wasn't fully trusted.
Steven Newbury Nov. 6, 2018, noon UTC | #6 
On 06/11/2018, Steven Newbury <steven.newbury@googlemail.com> wrote:

> From the reverted ARM prelink patch it's clear the magic is in the
> section guarded
> by RESOLVE_CONFLICT_FIND_MAP.  The x86_64 code already handles several
> different cases of R_X86_64_TLSDESC in that switch so I still don't
> understand how it falls through to _dl_reloc_bad_type rather than just
> ignoring the prelink suppliied address.  Most of the ARM patch
> consists of asserts presumably because the code wasn't fully trusted.
>
Okay, I've applied the attached patch inserting the code removed from
the ARM dl-machine.h into the x86_64 code handling R_X86_64_TLSDESC.
I still get the same error:

error while loading shared libraries: unexpected reloc type 0x24

Which can only be coming from _dl_reloc_bad_type() after it's fallen
through the switch in elf_machine_rela() or the if-else block in
elf_machine_lazy_rel(), both of which have should not fall through
since R_X86_64_TLSDESC is reloc type 0x24 and it is, or should be
handled!
--- ./sysdeps/x86_64/dl-machine.h.orig	2018-11-06 09:51:14.316220266 +0000
+++ ./sysdeps/x86_64/dl-machine.h	2018-11-06 09:53:17.511458780 +0000
@@ -392,6 +392,29 @@
 	    struct tlsdesc volatile *td =
 	      (struct tlsdesc volatile *)reloc_addr;
 
+#  ifdef RESOLVE_CONFLICT_FIND_MAP
+	    if (map->l_info[VALIDX (DT_GNU_PRELINKED)] != NULL)
+	      {
+		RESOLVE_CONFLICT_FIND_MAP (map, reloc_addr);
+
+		/* Make sure we know what's going on.  */
+		assert (td->entry
+		    == (void *) (D_PTR (map, l_info[ADDRIDX (DT_TLSDESC_PLT)])
+				 + map->l_addr));
+		assert (map->l_info[ADDRIDX (DT_TLSDESC_GOT)]);
+
+		/* Set up the lazy resolver and store the pointer to our link
+		map in _GLOBAL_OFFSET_TABLE[1] now as for a prelinked
+		binary elf_machine_runtime_setup() is not called and hence
+		neither has been initialized.  */
+		*(ElfW(Addr) *) (D_PTR (map, l_info[ADDRIDX (DT_TLSDESC_GOT)])
+			     + map->l_addr)
+			= (ElfW(Addr)) &_dl_tlsdesc_lazy_resolver;
+		((ElfW(Addr) *) D_PTR (map, l_info[DT_PLTGOT]))[1]
+			= (ElfW(Addr)) map;
+	      }
+	    else
+#  endif
 #  ifndef RTLD_BOOTSTRAP
 	    if (! sym)
 	      {
Steven Newbury Nov. 6, 2018, 3:16 p.m. UTC | #7 
On 06/11/2018, Steven Newbury <steven.newbury@googlemail.com> wrote:
> On 06/11/2018, Steven Newbury <steven.newbury@googlemail.com> wrote:
>
>> From the reverted ARM prelink patch it's clear the magic is in the
>> section guarded
>> by RESOLVE_CONFLICT_FIND_MAP.  The x86_64 code already handles several
>> different cases of R_X86_64_TLSDESC in that switch so I still don't
>> understand how it falls through to _dl_reloc_bad_type rather than just
>> ignoring the prelink suppliied address.  Most of the ARM patch
>> consists of asserts presumably because the code wasn't fully trusted.
>>
> Okay, I've applied the attached patch inserting the code removed from
> the ARM dl-machine.h into the x86_64 code handling R_X86_64_TLSDESC.
> I still get the same error:
>
> error while loading shared libraries: unexpected reloc type 0x24
>
> Which can only be coming from _dl_reloc_bad_type() after it's fallen
> through the switch in elf_machine_rela() or the if-else block in
> elf_machine_lazy_rel(), both of which have should not fall through
> since R_X86_64_TLSDESC is reloc type 0x24 and it is, or should be
> handled!
>
Despite staring at the code for hours I didn't see the case was
guarded by an #ifdef block.  The patch I last attached doesn't compile
when the code is moved outside without the resolver being changed
since _dl_tlsdesc_lazy_resolver isnt present for x86-64.  Even then
the map->l_info[VALIDX (DT_GNU_PRELINKED)] != NULL doesn't seem to
work.  Mind you it no longer fails to run the binary, whether it's
actually using the prelinked value is another matter..?
diff mbox series

Patch

From 963c95558ac1167121393d584bf00641a26eda4d Mon Sep 17 00:00:00 2001
From: Steven Newbury <steve@snewbury.org.uk>
Date: Mon, 5 Nov 2018 09:29:05 +0000
Subject: [PATCH] Add support for R_X86_64_TLSDESC

This is based on the R_ARM_TLS_DESC support code as
integrated in commit 874b3d3f6413c16597ec92ed92c57d6bfe2bbb68

Signed-off-by: Steven Newbury <steve@snewbury.org.uk>
---
 src/arch-x86_64.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/src/arch-x86_64.c b/src/arch-x86_64.c
index 2f6c551..9b1b7fb 100644
--- a/src/arch-x86_64.c
+++ b/src/arch-x86_64.c
@@ -133,6 +133,7 @@  x86_64_prelink_rela (struct prelink_info *info, GElf_Rela *rela,
 {
   DSO *dso;
   GElf_Addr value;
+  Elf64_Sword val;
 
   dso = info->dso;
   if (GELF_R_TYPE (rela->r_info) == R_X86_64_NONE
@@ -184,6 +185,24 @@  x86_64_prelink_rela (struct prelink_info *info, GElf_Rela *rela,
 	return 0;
       error (0, 0, "%s: R_X86_64_COPY reloc in shared library?", dso->filename);
       return 1;
+    case R_X86_64_TLSDESC:
+      if (!dso->info_DT_TLSDESC_PLT)
+        {
+          error (0, 0,
+		 "%s: Unsupported R_X86_64_TLSDESC relocation in non-lazily bound object.",
+                 dso->filename);
+          return 1;
+        }
+      val = read_ule64 (dso, rela->r_offset + 8);
+      if (val != 0 && !dynamic_info_is_set (dso, DT_GNU_PRELINKED_BIT))
+        {
+          error (0, 0,
+		 "%s: Unexpected non-zero value (0x%x) in R_X86_64_TLSDESC?",
+                 dso->filename, val);
+          return 1;
+        }
+      write_le64 (dso, rela->r_offset + 8, dso->info_DT_TLSDESC_PLT);
+      break;
     default:
       error (0, 0, "%s: Unknown X86-64 relocation type %d", dso->filename,
 	     (int) GELF_R_TYPE (rela->r_info));
@@ -303,6 +322,9 @@  x86_64_prelink_conflict_rela (DSO *dso, struct prelink_info *info,
 	/* Similarly IRELATIVE relocations always need conflicts.  */
 	case R_X86_64_IRELATIVE:
 	  break;
+	/* Likewise TLSDESC.  */
+	case R_X86_64_TLSDESC:
+	  break;
 	default:
 	  return 0;
 	}
@@ -372,7 +394,9 @@  x86_64_prelink_conflict_rela (DSO *dso, struct prelink_info *info,
 	  break;
 	}
       break;
-
+    case R_X86_64_TLSDESC:
+      /* Nothing to do.  */
+      break;
     default:
       error (0, 0, "%s: Unknown X86-64 relocation type %d", dso->filename,
 	     (int) GELF_R_TYPE (rela->r_info));
@@ -498,6 +522,9 @@  x86_64_undo_prelink_rela (DSO *dso, GElf_Rela *rela, GElf_Addr relaaddr)
     case R_X86_64_TPOFF64:
       write_le64 (dso, rela->r_offset, 0);
       break;
+    case R_X86_64_TLSDESC:
+      write_le64 (dso, rela->r_offset + 8, 0);
+      break;
     case R_X86_64_32:
     case R_X86_64_PC32:
       write_le32 (dso, rela->r_offset, 0);
@@ -556,6 +583,7 @@  x86_64_reloc_class (int reloc_type)
     case R_X86_64_DTPMOD64:
     case R_X86_64_DTPOFF64:
     case R_X86_64_TPOFF64:
+    case R_X86_64_TLSDESC:
       return RTYPE_CLASS_TLS;
     default: return RTYPE_CLASS_VALID;
     }
-- 
2.19.1