[PATCH] mprotect segments with extra PROT_WRITE bit when DT_TEXTREL bit is set

[Fangrui Song]

On 2018-08-22, Fangrui Song wrote:
>On 2018-08-22, Fangrui Song wrote:
>>Currently, DT_TEXTREL is incompatible with IFUNC. When DT_TEXTREL or
>>DF_TEXTREL is seen, the dynamic linker calls __mprotect on the segments
>>with PROT_READ|PROT_WRITE before applying dynamic relocations. It leads
>>to segfault when performing IFUNC resolution.
>>
>>This patch makes it call __mprotect with extra PROT_WRITE bit, which
>>will keep the PROT_EXEC bit if exists, and thus fixes the segfault.
>>FreeBSD rtld libexec/rtld-elf/rtld.c (reloc_textrel_prot) does the same.
>>
>>2018-08-22  Fangrui Song  <maskray@google.com>
>>
>>	* elf/dl-reloc.c (_dl_relocate_object): __mprotect with extra
>>	PROT_WRITE bit.
>>
>>diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
>>index 053916eeae..bd7d1ae84f 100644
>>--- a/elf/dl-reloc.c
>>+++ b/elf/dl-reloc.c
>>@@ -199,14 +199,6 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>			- ALIGN_DOWN (ph->p_vaddr, GLRO(dl_pagesize));
>>	    newp->start = PTR_ALIGN_DOWN (ph->p_vaddr, GLRO(dl_pagesize))
>>			  + (caddr_t) l->l_addr;
>>-
>>-	    if (__mprotect (newp->start, newp->len, PROT_READ|PROT_WRITE) < 0)
>>-	      {
>>-		errstring = N_("cannot make segment writable for relocation");
>>-	      call_error:
>>-		_dl_signal_error (errno, l->l_name, NULL, errstring);
>>-	      }
>>-
>>#if (PF_R | PF_W | PF_X) == 7 && (PROT_READ | PROT_WRITE | PROT_EXEC) == 7
>>	    newp->prot = (PF_TO_PROT
>>			  >> ((ph->p_flags & (PF_R | PF_W | PF_X)) * 4)) & 0xf;
>>@@ -220,6 +212,14 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
>>	      newp->prot |= PROT_EXEC;
>>#endif
>>	    newp->next = textrels;
>>+
>>+	    if (__mprotect (newp->start, newp->len, newp->prot|PROT_WRITE) < 0)
>>+	      {
>>+		errstring = N_("cannot make segment writable for relocation");
>>+	      call_error:
>>+		_dl_signal_error (errno, l->l_name, NULL, errstring);
>>+	      }
>>+
>>	    textrels = newp;
>>	  }
>>   }
>
>A simple reproduce.
>
>// a.c
>void f_imp(void) {}
>void (*resolve_f(void))(void) { return f_imp; }
>void f(void) __attribute__((ifunc("resolve_f")));
>int main(void) { f(); }
>
>% clang -fuse-ld=lld -Wl,-z,notext a.c -o a; ./a
>[1]    88059 segmentation fault  ./a
>% clang -fuse-ld=lld -Wl,-z,notext a.c -Wl,--dynamic-linker=~/Dev/Util/glibc/build/elf/ld.so -o a; ./a
># good
>
>gcc is similar. But upstream gcc does not support -fuse-ld=lld. It is available
>on Debian and its derivatives via a patch.
>
>This works fine on FreeBSD because its rtld does the same as the patch does.
>OpenBSD rtld is probably also faulty.

To give a bit more context, ld.bfd and gold emit DT_TEXTREL on demand
(and default to -z notext): they create DT_TEXTREL only if text relocation is
required. While lld chooses a more explicit approach:

* By default text relocation is disallowed (-z text)
* If -z notext is specified, text relocation will be allowed. DT_TEXTREL
  will be created no matter if text relocation is really used.

ld.bfd probably defends this by rejecting linking when both IFUNC and DT_TEXTREL are required.


-- 
宋方睿