diff mbox series

Fix ifunc support with DT_TEXTREL segments (BZ#20480)

Message ID 20180830200724.30045-1-adhemerval.zanella@linaro.org
State Accepted
Commit b5c45e83753b27dc538dff2d55d4410c385cf3a4
Headers show
Series Fix ifunc support with DT_TEXTREL segments (BZ#20480) | expand

Commit Message

Adhemerval Zanella Netto Aug. 30, 2018, 8:07 p.m. UTC 
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 (which requires PROT_EXEC
as well for the IFUNC resolver).

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.

Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,
sparc64-linux-gnu, sparcv9-linux-gnu, and armv8-linux-gnueabihf.

	Adam J. Richte  <adam_richter2004@yahoo.com>
	Adhemerval Zanella  <adhemerval.zanella@linaro.org>
	Fangrui Song  <maskray@google.com>

	[BZ #20480]
	* config.h.in (CAN_TEXTREL_IFUNC): New define.
	* configure.ac: Add check if linker supports textrel relocation with
	ifunc.
	* elf/dl-reloc.c (_dl_relocate_object): Use all required flags on
	DT_TEXTREL segments, not only PROT_READ and PROT_WRITE.
	* elf/Makefile (ifunc-pie-tests): Add tst-ifunc-textrel.
	(CFLAGS-tst-ifunc-textrel.c): New rule.
	* elf/tst-ifunc-textrel.c: New file.
---
 ChangeLog               | 14 +++++++++++
 config.h.in             |  3 +++
 configure               | 51 +++++++++++++++++++++++++++++++++++++++
 configure.ac            | 38 +++++++++++++++++++++++++++++
 elf/Makefile            |  4 +++-
 elf/dl-reloc.c          | 20 +++++++---------
 elf/tst-ifunc-textrel.c | 53 +++++++++++++++++++++++++++++++++++++++++
 7 files changed, 170 insertions(+), 13 deletions(-)
 create mode 100644 elf/tst-ifunc-textrel.c

-- 
2.17.1

Comments

Adhemerval Zanella Netto Sept. 20, 2018, 6:34 p.m. UTC | #1 
Ping.

On 30/08/2018 13:07, Adhemerval Zanella 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 (which requires PROT_EXEC

> as well for the IFUNC resolver).

> 

> 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.

> 

> Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,

> sparc64-linux-gnu, sparcv9-linux-gnu, and armv8-linux-gnueabihf.

> 

> 	Adam J. Richte  <adam_richter2004@yahoo.com>

> 	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 	Fangrui Song  <maskray@google.com>

> 

> 	[BZ #20480]

> 	* config.h.in (CAN_TEXTREL_IFUNC): New define.

> 	* configure.ac: Add check if linker supports textrel relocation with

> 	ifunc.

> 	* elf/dl-reloc.c (_dl_relocate_object): Use all required flags on

> 	DT_TEXTREL segments, not only PROT_READ and PROT_WRITE.

> 	* elf/Makefile (ifunc-pie-tests): Add tst-ifunc-textrel.

> 	(CFLAGS-tst-ifunc-textrel.c): New rule.

> 	* elf/tst-ifunc-textrel.c: New file.

> ---

>  ChangeLog               | 14 +++++++++++

>  config.h.in             |  3 +++

>  configure               | 51 +++++++++++++++++++++++++++++++++++++++

>  configure.ac            | 38 +++++++++++++++++++++++++++++

>  elf/Makefile            |  4 +++-

>  elf/dl-reloc.c          | 20 +++++++---------

>  elf/tst-ifunc-textrel.c | 53 +++++++++++++++++++++++++++++++++++++++++

>  7 files changed, 170 insertions(+), 13 deletions(-)

>  create mode 100644 elf/tst-ifunc-textrel.c

> 

> diff --git a/config.h.in b/config.h.in

> index 141db213a9..d6fcbbac78 100644

> --- a/config.h.in

> +++ b/config.h.in

> @@ -247,4 +247,7 @@

>     in i386 6 argument syscall issue).  */

>  #define CAN_USE_REGISTER_ASM_EBP 0

>  

> +/* Linker supports ifunc with text relocation (used in tests).  */

> +#define CAN_TEXTREL_IFUNC 0

> +

>  #endif

> diff --git a/configure b/configure

> index 285a6537f0..ab4c12c5ba 100755

> --- a/configure

> +++ b/configure

> @@ -664,6 +664,7 @@ INSTALL_PROGRAM

>  sysnames

>  submachine

>  multi_arch

> +libc_cv_textrel_ifunc

>  no_stack_protector

>  stack_protector

>  libc_cv_ssp

> @@ -4052,6 +4053,56 @@ fi

>  { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_indirect_function" >&5

>  $as_echo "$libc_cv_gcc_indirect_function" >&6; }

>  

> +# Check if linker supports textrel relocation with ifunc (used on elf/tests).

> +# Note that it relies on libc_cv_ld_gnu_indirect_function test above.

> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the linker supports textrels along with ifunc" >&5

> +$as_echo_n "checking whether the linker supports textrels along with ifunc... " >&6; }

> +if ${libc_cv_textrel_ifunc+:} false; then :

> +  $as_echo_n "(cached) " >&6

> +else

> +  cat > conftest.S <<EOF

> +.type foo,%gnu_indirect_function

> +foo:

> +.globl _start

> +_start:

> +.globl __start

> +__start:

> +.data

> +#ifdef _LP64

> +.quad foo

> +#else

> +.long foo

> +#endif

> +.text

> +.globl address

> +address:

> +#ifdef _LP64

> +.quad address

> +#else

> +.long address

> +#endif

> +EOF

> +libc_cv_textrel_ifunc=no

> +if test $libc_cv_ld_gnu_indirect_function = yes; then

> +   if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostartfiles -nostdlib $no_ssp -pie -o conftest conftest.S'

> +  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5

> +  (eval $ac_try) 2>&5

> +  ac_status=$?

> +  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5

> +  test $ac_status = 0; }; }; then

> +     libc_cv_textrel_ifunc=yes

> +   fi

> +fi

> +rm -f conftest*

> +fi

> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_textrel_ifunc" >&5

> +$as_echo "$libc_cv_textrel_ifunc" >&6; }

> +if test $libc_cv_textrel_ifunc = yes; then

> +  $as_echo "#define CAN_TEXTREL_IFUNC 1" >>confdefs.h

> +

> +fi

> +

> +

>  # Check if gcc warns about alias for function with incompatible types.

>  { $as_echo "$as_me:${as_lineno-$LINENO}: checking if compiler warns about alias for function with incompatible types" >&5

>  $as_echo_n "checking if compiler warns about alias for function with incompatible types... " >&6; }

> diff --git a/configure.ac b/configure.ac

> index 8045d44dd0..e3d5c0598b 100644

> --- a/configure.ac

> +++ b/configure.ac

> @@ -671,6 +671,44 @@ if ${CC-cc} -c conftest.c -o conftest.o 1>&AS_MESSAGE_LOG_FD \

>  fi

>  rm -f conftest*])

>  

> +# Check if linker supports textrel relocation with ifunc (used on elf/tests).

> +# Note that it relies on libc_cv_ld_gnu_indirect_function test above.

> +AC_CACHE_CHECK([whether the linker supports textrels along with ifunc],

> +               libc_cv_textrel_ifunc, [dnl

> +cat > conftest.S <<EOF

> +.type foo,%gnu_indirect_function

> +foo:

> +.globl _start

> +_start:

> +.globl __start

> +__start:

> +.data

> +#ifdef _LP64

> +.quad foo

> +#else

> +.long foo

> +#endif

> +.text

> +.globl address

> +address:

> +#ifdef _LP64

> +.quad address

> +#else

> +.long address

> +#endif

> +EOF

> +libc_cv_textrel_ifunc=no

> +if test $libc_cv_ld_gnu_indirect_function = yes; then

> +   if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostartfiles -nostdlib $no_ssp -pie -o conftest conftest.S); then

> +     libc_cv_textrel_ifunc=yes

> +   fi

> +fi

> +rm -f conftest*])

> +if test $libc_cv_textrel_ifunc = yes; then

> +  AC_DEFINE(CAN_TEXTREL_IFUNC)

> +fi

> +AC_SUBST(libc_cv_textrel_ifunc)

> +

>  # Check if gcc warns about alias for function with incompatible types.

>  AC_CACHE_CHECK([if compiler warns about alias for function with incompatible types],

>  	       libc_cv_gcc_incompatible_alias, [dnl

> diff --git a/elf/Makefile b/elf/Makefile

> index cd0771307f..77eb6ef104 100644

> --- a/elf/Makefile

> +++ b/elf/Makefile

> @@ -335,7 +335,8 @@ extra-test-objs += $(ifunc-test-modules:=.o)

>  test-internal-extras += $(ifunc-test-modules)

>  ifeq (yes,$(have-fpie))

>  ifunc-pie-tests = ifuncmain1pie ifuncmain1vispie ifuncmain1staticpie \

> -		  ifuncmain5pie ifuncmain6pie ifuncmain7pie

> +		  ifuncmain5pie ifuncmain6pie ifuncmain7pie \

> +		  tst-ifunc-textrel

>  tests-internal += $(ifunc-pie-tests)

>  tests-pie += $(ifunc-pie-tests)

>  endif

> @@ -1265,6 +1266,7 @@ CFLAGS-ifuncmain1staticpie.c += $(pie-ccflag)

>  CFLAGS-ifuncmain5pie.c += $(pie-ccflag)

>  CFLAGS-ifuncmain6pie.c += $(pie-ccflag)

>  CFLAGS-ifuncmain7pie.c += $(pie-ccflag)

> +CFLAGS-tst-ifunc-textrel.c += $(pic-ccflag)

>  

>  $(objpfx)ifuncmain1pie: $(objpfx)ifuncmod1.so

>  $(objpfx)ifuncmain1staticpie: $(objpfx)ifuncdep1pic.o

> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c

> index 053916eeae..164f4efa10 100644

> --- a/elf/dl-reloc.c

> +++ b/elf/dl-reloc.c

> @@ -200,17 +200,6 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],

>  	    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;

> -#else

>  	    newp->prot = 0;

>  	    if (ph->p_flags & PF_R)

>  	      newp->prot |= PROT_READ;

> @@ -218,7 +207,14 @@ _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],

>  	      newp->prot |= PROT_WRITE;

>  	    if (ph->p_flags & PF_X)

>  	      newp->prot |= PROT_EXEC;

> -#endif

> +

> +	    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);

> +	      }

> +

>  	    newp->next = textrels;

>  	    textrels = newp;

>  	  }

> diff --git a/elf/tst-ifunc-textrel.c b/elf/tst-ifunc-textrel.c

> new file mode 100644

> index 0000000000..60e5ac3bf8

> --- /dev/null

> +++ b/elf/tst-ifunc-textrel.c

> @@ -0,0 +1,53 @@

> +/* Check DT_TEXTREL/DF_TEXTREL support with ifunc.

> +   Copyright (C) 2018 Free Software Foundation, Inc.

> +   This file is part of the GNU C Library.

> +

> +   The GNU C Library is free software; you can redistribute it and/or

> +   modify it under the terms of the GNU Lesser General Public

> +   License as published by the Free Software Foundation; either

> +   version 2.1 of the License, or (at your option) any later version.

> +

> +   The GNU C Library is distributed in the hope that it will be useful,

> +   but WITHOUT ANY WARRANTY; without even the implied warranty of

> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU

> +   Lesser General Public License for more details.

> +

> +   You should have received a copy of the GNU Lesser General Public

> +   License along with the GNU C Library; if not, see

> +   <http://www.gnu.org/licenses/>.  */

> +

> +#include <stdint.h>

> +

> +/* Some linkers may fail to produce a shared object with ifunc and text

> +   relocations.  */

> +#if CAN_TEXTREL_IFUNC

> +/* Force a text relocation in the object.  */

> +static const uintptr_t

> +address __attribute__((section(".text"))) = (uintptr_t) &address;

> +

> +static uintptr_t

> +foo_impl (void)

> +{

> +  return address;

> +}

> +

> +void *

> +__attribute__((noinline))

> +foo (void)

> +{

> +  return (void*) foo_impl;

> +}

> +__asm__ (".type foo, %gnu_indirect_function");

> +#endif

> +

> +static int

> +do_test (void)

> +{

> +#if CAN_TEXTREL_IFUNC

> +  return (uintptr_t) foo () != 0 ? 0 : 1;

> +#else

> +  return 0;

> +#endif

> +}

> +

> +#include <support/test-driver.c>

>
Florian Weimer Sept. 22, 2018, 12:14 p.m. UTC | #2 
* Adhemerval Zanella:

> 	Adam J. Richte  <adam_richter2004@yahoo.com>

> 	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 	Fangrui Song  <maskray@google.com>

>

> 	[BZ #20480]

> 	* config.h.in (CAN_TEXTREL_IFUNC): New define.

> 	* configure.ac: Add check if linker supports textrel relocation with

> 	ifunc.

> 	* elf/dl-reloc.c (_dl_relocate_object): Use all required flags on

> 	DT_TEXTREL segments, not only PROT_READ and PROT_WRITE.

> 	* elf/Makefile (ifunc-pie-tests): Add tst-ifunc-textrel.

> 	(CFLAGS-tst-ifunc-textrel.c): New rule.

> 	* elf/tst-ifunc-textrel.c: New file.


The code change looks okay to me.  Not sure about copyright assignment
needs for this one, but it's probably below the threshold because it's
just moving code.

I would prefer if the new test was disabled at the makefile level, not
with a preprocessor conditional.  But it's not a strong preference.

Thanks,
Florian
Adhemerval Zanella Netto Sept. 25, 2018, 8:13 p.m. UTC | #3 
On 22/09/2018 09:14, Florian Weimer wrote:
> * Adhemerval Zanella:

> 

>> 	Adam J. Richte  <adam_richter2004@yahoo.com>

>> 	Adhemerval Zanella  <adhemerval.zanella@linaro.org>

>> 	Fangrui Song  <maskray@google.com>

>>

>> 	[BZ #20480]

>> 	* config.h.in (CAN_TEXTREL_IFUNC): New define.

>> 	* configure.ac: Add check if linker supports textrel relocation with

>> 	ifunc.

>> 	* elf/dl-reloc.c (_dl_relocate_object): Use all required flags on

>> 	DT_TEXTREL segments, not only PROT_READ and PROT_WRITE.

>> 	* elf/Makefile (ifunc-pie-tests): Add tst-ifunc-textrel.

>> 	(CFLAGS-tst-ifunc-textrel.c): New rule.

>> 	* elf/tst-ifunc-textrel.c: New file.

> 

> The code change looks okay to me.  Not sure about copyright assignment

> needs for this one, but it's probably below the threshold because it's

> just moving code.

> 

> I would prefer if the new test was disabled at the makefile level, not

> with a preprocessor conditional.  But it's not a strong preference.


I have changed the patch to your suggestion, thanks for reviewing it.
diff mbox series

Patch

diff --git a/config.h.in b/config.h.in
index 141db213a9..d6fcbbac78 100644
--- a/config.h.in
+++ b/config.h.in
@@ -247,4 +247,7 @@ 
    in i386 6 argument syscall issue).  */
 #define CAN_USE_REGISTER_ASM_EBP 0
 
+/* Linker supports ifunc with text relocation (used in tests).  */
+#define CAN_TEXTREL_IFUNC 0
+
 #endif
diff --git a/configure b/configure
index 285a6537f0..ab4c12c5ba 100755
--- a/configure
+++ b/configure
@@ -664,6 +664,7 @@  INSTALL_PROGRAM
 sysnames
 submachine
 multi_arch
+libc_cv_textrel_ifunc
 no_stack_protector
 stack_protector
 libc_cv_ssp
@@ -4052,6 +4053,56 @@  fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_gcc_indirect_function" >&5
 $as_echo "$libc_cv_gcc_indirect_function" >&6; }
 
+# Check if linker supports textrel relocation with ifunc (used on elf/tests).
+# Note that it relies on libc_cv_ld_gnu_indirect_function test above.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the linker supports textrels along with ifunc" >&5
+$as_echo_n "checking whether the linker supports textrels along with ifunc... " >&6; }
+if ${libc_cv_textrel_ifunc+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  cat > conftest.S <<EOF
+.type foo,%gnu_indirect_function
+foo:
+.globl _start
+_start:
+.globl __start
+__start:
+.data
+#ifdef _LP64
+.quad foo
+#else
+.long foo
+#endif
+.text
+.globl address
+address:
+#ifdef _LP64
+.quad address
+#else
+.long address
+#endif
+EOF
+libc_cv_textrel_ifunc=no
+if test $libc_cv_ld_gnu_indirect_function = yes; then
+   if { ac_try='${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostartfiles -nostdlib $no_ssp -pie -o conftest conftest.S'
+  { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
+  (eval $ac_try) 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; }; then
+     libc_cv_textrel_ifunc=yes
+   fi
+fi
+rm -f conftest*
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_textrel_ifunc" >&5
+$as_echo "$libc_cv_textrel_ifunc" >&6; }
+if test $libc_cv_textrel_ifunc = yes; then
+  $as_echo "#define CAN_TEXTREL_IFUNC 1" >>confdefs.h
+
+fi
+
+
 # Check if gcc warns about alias for function with incompatible types.
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking if compiler warns about alias for function with incompatible types" >&5
 $as_echo_n "checking if compiler warns about alias for function with incompatible types... " >&6; }
diff --git a/configure.ac b/configure.ac
index 8045d44dd0..e3d5c0598b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -671,6 +671,44 @@  if ${CC-cc} -c conftest.c -o conftest.o 1>&AS_MESSAGE_LOG_FD \
 fi
 rm -f conftest*])
 
+# Check if linker supports textrel relocation with ifunc (used on elf/tests).
+# Note that it relies on libc_cv_ld_gnu_indirect_function test above.
+AC_CACHE_CHECK([whether the linker supports textrels along with ifunc],
+               libc_cv_textrel_ifunc, [dnl
+cat > conftest.S <<EOF
+.type foo,%gnu_indirect_function
+foo:
+.globl _start
+_start:
+.globl __start
+__start:
+.data
+#ifdef _LP64
+.quad foo
+#else
+.long foo
+#endif
+.text
+.globl address
+address:
+#ifdef _LP64
+.quad address
+#else
+.long address
+#endif
+EOF
+libc_cv_textrel_ifunc=no
+if test $libc_cv_ld_gnu_indirect_function = yes; then
+   if AC_TRY_COMMAND(${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS -nostartfiles -nostdlib $no_ssp -pie -o conftest conftest.S); then
+     libc_cv_textrel_ifunc=yes
+   fi
+fi
+rm -f conftest*])
+if test $libc_cv_textrel_ifunc = yes; then
+  AC_DEFINE(CAN_TEXTREL_IFUNC)
+fi
+AC_SUBST(libc_cv_textrel_ifunc)
+
 # Check if gcc warns about alias for function with incompatible types.
 AC_CACHE_CHECK([if compiler warns about alias for function with incompatible types],
 	       libc_cv_gcc_incompatible_alias, [dnl
diff --git a/elf/Makefile b/elf/Makefile
index cd0771307f..77eb6ef104 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -335,7 +335,8 @@  extra-test-objs += $(ifunc-test-modules:=.o)
 test-internal-extras += $(ifunc-test-modules)
 ifeq (yes,$(have-fpie))
 ifunc-pie-tests = ifuncmain1pie ifuncmain1vispie ifuncmain1staticpie \
-		  ifuncmain5pie ifuncmain6pie ifuncmain7pie
+		  ifuncmain5pie ifuncmain6pie ifuncmain7pie \
+		  tst-ifunc-textrel
 tests-internal += $(ifunc-pie-tests)
 tests-pie += $(ifunc-pie-tests)
 endif
@@ -1265,6 +1266,7 @@  CFLAGS-ifuncmain1staticpie.c += $(pie-ccflag)
 CFLAGS-ifuncmain5pie.c += $(pie-ccflag)
 CFLAGS-ifuncmain6pie.c += $(pie-ccflag)
 CFLAGS-ifuncmain7pie.c += $(pie-ccflag)
+CFLAGS-tst-ifunc-textrel.c += $(pic-ccflag)
 
 $(objpfx)ifuncmain1pie: $(objpfx)ifuncmod1.so
 $(objpfx)ifuncmain1staticpie: $(objpfx)ifuncdep1pic.o
diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
index 053916eeae..164f4efa10 100644
--- a/elf/dl-reloc.c
+++ b/elf/dl-reloc.c
@@ -200,17 +200,6 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
 	    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;
-#else
 	    newp->prot = 0;
 	    if (ph->p_flags & PF_R)
 	      newp->prot |= PROT_READ;
@@ -218,7 +207,14 @@  _dl_relocate_object (struct link_map *l, struct r_scope_elem *scope[],
 	      newp->prot |= PROT_WRITE;
 	    if (ph->p_flags & PF_X)
 	      newp->prot |= PROT_EXEC;
-#endif
+
+	    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);
+	      }
+
 	    newp->next = textrels;
 	    textrels = newp;
 	  }
diff --git a/elf/tst-ifunc-textrel.c b/elf/tst-ifunc-textrel.c
new file mode 100644
index 0000000000..60e5ac3bf8
--- /dev/null
+++ b/elf/tst-ifunc-textrel.c
@@ -0,0 +1,53 @@ 
+/* Check DT_TEXTREL/DF_TEXTREL support with ifunc.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdint.h>
+
+/* Some linkers may fail to produce a shared object with ifunc and text
+   relocations.  */
+#if CAN_TEXTREL_IFUNC
+/* Force a text relocation in the object.  */
+static const uintptr_t
+address __attribute__((section(".text"))) = (uintptr_t) &address;
+
+static uintptr_t
+foo_impl (void)
+{
+  return address;
+}
+
+void *
+__attribute__((noinline))
+foo (void)
+{
+  return (void*) foo_impl;
+}
+__asm__ (".type foo, %gnu_indirect_function");
+#endif
+
+static int
+do_test (void)
+{
+#if CAN_TEXTREL_IFUNC
+  return (uintptr_t) foo () != 0 ? 0 : 1;
+#else
+  return 0;
+#endif
+}
+
+#include <support/test-driver.c>