elf: Do not run IFUNC resolvers for LD_DEBUG=unused [BZ #24214]

Message ID 87y2vy1v2i.fsf@oldenburg2.str.redhat.com
State New
Headers show
Series
  • elf: Do not run IFUNC resolvers for LD_DEBUG=unused [BZ #24214]
Related show

Commit Message

Florian Weimer Nov. 29, 2019, 4:55 p.m.
Tested on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,
s390-linux-gnu, s390x-linux-gnu, powerpc64le-linux-gnu.
build-many-glibcs.py is still running, but looks good so far.

Thanks,
Florian

8<------------------------------------------------------------------8<
This commit adds missing skip_ifunc checks to aarch64, arm, i386,
sparc, and x86_64.  A new test case ensures that IRELATIVE IFUNC
resolvers do not run in various diagnostic modes of the dynamic
loader.

-----
 elf/Makefile                       | 16 +++++++++++
 elf/tst-ifunc-fault-bindnow.c      | 21 ++++++++++++++
 elf/tst-ifunc-fault-lazy.c         | 57 ++++++++++++++++++++++++++++++++++++++
 sysdeps/aarch64/dl-machine.h       |  3 +-
 sysdeps/arm/dl-machine.h           |  6 ++--
 sysdeps/i386/dl-machine.h          |  6 ++--
 sysdeps/sparc/sparc32/dl-machine.h |  6 ++--
 sysdeps/sparc/sparc64/dl-machine.h |  6 ++--
 sysdeps/x86_64/dl-machine.h        |  3 +-
 9 files changed, 114 insertions(+), 10 deletions(-)

Comments

Szabolcs Nagy Dec. 2, 2019, 12:59 p.m. | #1
On 29/11/2019 16:55, Florian Weimer wrote:
> Tested on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,

> s390-linux-gnu, s390x-linux-gnu, powerpc64le-linux-gnu.

> build-many-glibcs.py is still running, but looks good so far.

> 

> Thanks,

> Florian

> 

> 8<------------------------------------------------------------------8<

> This commit adds missing skip_ifunc checks to aarch64, arm, i386,

> sparc, and x86_64.  A new test case ensures that IRELATIVE IFUNC

> resolvers do not run in various diagnostic modes of the dynamic

> loader.


thanks, it looks good to me.

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 305bed2d38..f825354d3f 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -348,6 +348,7 @@  tests-ifuncstatic := ifuncmain1static ifuncmain1picstatic \
 tests-static += $(tests-ifuncstatic)
 tests-internal += $(tests-ifuncstatic)
 ifeq (yes,$(build-shared))
+tests += tst-ifunc-fault-lazy tst-ifunc-fault-bindnow
 # Note: sysdeps/x86_64/ifuncmain8.c uses ifuncmain8.
 tests-internal += \
 	 ifuncmain1 ifuncmain1pic ifuncmain1vis ifuncmain1vispic \
@@ -1354,6 +1355,21 @@  $(objpfx)ifuncmain5static: $(addprefix $(objpfx),ifuncdep5.o)
 $(objpfx)ifuncmain5staticpic: $(addprefix $(objpfx),ifuncdep5pic.o)
 $(objpfx)ifuncmain5picstatic: $(addprefix $(objpfx),ifuncdep5pic.o)
 
+LDFLAGS-tst-ifunc-fault-lazy = -Wl,-z,lazy
+LDFLAGS-tst-ifunc-fault-bindnow = -Wl,-z,now
+define tst-ifunc-fault-script
+( $(objpfx)ld.so --verify --library-path  $(objpfx) $^ \
+  && LD_TRACE_LOADED_OBJECTS=1 $(objpfx)ld.so --library-path $(objpfx) $^ \
+  && LD_TRACE_LOADED_OBJECTS=1 LD_DEBUG=unused \
+    $(objpfx)ld.so --library-path  $(objpfx) $^ \
+) > $@; $(evaluate-test)
+endef
+$(objpfx)tst-ifunc-fault-lazy.out: $(objpfx)tst-ifunc-fault-lazy $(objpfx)ld.so
+	$(tst-ifunc-fault-script)
+$(objpfx)tst-ifunc-fault-bindnow.out: $(objpfx)tst-ifunc-fault-bindnow \
+   $(objpfx)ld.so
+	$(tst-ifunc-fault-script)
+
 $(objpfx)tst-unique1: $(libdl)
 $(objpfx)tst-unique1.out: $(objpfx)tst-unique1mod1.so \
 			  $(objpfx)tst-unique1mod2.so
diff --git a/elf/tst-ifunc-fault-bindnow.c b/elf/tst-ifunc-fault-bindnow.c
new file mode 100644
index 0000000000..d767c1f1c1
--- /dev/null
+++ b/elf/tst-ifunc-fault-bindnow.c
@@ -0,0 +1,21 @@ 
+/* Program with local IFUNC resolver which crashes.  BIND_NOW variant.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+/* The code is the same as the lazy variant.  It is just linked
+   differently.  */
+#include "tst-ifunc-fault-lazy.c"
diff --git a/elf/tst-ifunc-fault-lazy.c b/elf/tst-ifunc-fault-lazy.c
new file mode 100644
index 0000000000..3389de273f
--- /dev/null
+++ b/elf/tst-ifunc-fault-lazy.c
@@ -0,0 +1,57 @@ 
+/* Program with local IFUNC resolver which crashes, for testing bug 24214.
+   Copyright (C) 2019 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
+   <https://www.gnu.org/licenses/>.  */
+
+/* The construct below is expected to produce an IRELATIVE relocation
+   with an IFUNC resolver that crashes. ldd should not performs such
+   relocations.  */
+
+#include <config.h>
+
+#ifdef HAVE_GCC_IFUNC
+
+# include <stddef.h>
+
+static void
+implementation (void)
+{
+  /* Produce a crash, without depending on any relocations.  */
+  volatile char *volatile p = NULL;
+  *p = 0;
+}
+
+static __typeof__ (implementation) *
+resolver (void)
+{
+  /* Produce a crash, without depending on any relocations.  */
+  volatile char *volatile p = NULL;
+  *p = 0;
+  return implementation;
+}
+
+static void magic (void) __attribute__ ((ifunc ("resolver")));
+
+void (*magic_ptr) (void) = magic;
+
+#endif /* HAVE_GCC_IFUNC */
+
+/* The program is expected not to run.  */
+int
+main (void)
+{
+  return 1;
+}
diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index 10b91c33ae..bf6b9d0e7d 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -358,7 +358,8 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 
 	case AARCH64_R(IRELATIVE):
 	  value = map->l_addr + reloc->r_addend;
-	  value = elf_ifunc_invoke (value);
+	  if (__glibc_likely (!skip_ifunc))
+	    value = elf_ifunc_invoke (value);
 	  *reloc_addr = value;
 	  break;
 
diff --git a/sysdeps/arm/dl-machine.h b/sysdeps/arm/dl-machine.h
index eeb3adfff2..27dffc71bf 100644
--- a/sysdeps/arm/dl-machine.h
+++ b/sysdeps/arm/dl-machine.h
@@ -522,7 +522,8 @@  elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
 	  break;
 	case R_ARM_IRELATIVE:
 	  value = map->l_addr + *reloc_addr;
-	  value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
+	  if (__glibc_likely (!skip_ifunc))
+	    value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
 	  *reloc_addr = value;
 	  break;
 #endif
@@ -614,7 +615,8 @@  elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
 	  break;
 	case R_ARM_IRELATIVE:
 	  value = map->l_addr + reloc->r_addend;
-	  value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
+	  if (__glibc_likely (!skip_ifunc))
+	    value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
 	  *reloc_addr = value;
 	  break;
 #endif
diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index 1e2a3b333d..e225aa3739 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -480,7 +480,8 @@  elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
 	  break;
 	case R_386_IRELATIVE:
 	  value = map->l_addr + *reloc_addr;
-	  value = ((Elf32_Addr (*) (void)) value) ();
+	  if (__glibc_likely (!skip_ifunc))
+	    value = ((Elf32_Addr (*) (void)) value) ();
 	  *reloc_addr = value;
 	  break;
 	default:
@@ -627,7 +628,8 @@  elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
 #  endif /* !RESOLVE_CONFLICT_FIND_MAP */
 	case R_386_IRELATIVE:
 	  value = map->l_addr + reloc->r_addend;
-	  value = ((Elf32_Addr (*) (void)) value) ();
+	  if (__glibc_likely (!skip_ifunc))
+	    value = ((Elf32_Addr (*) (void)) value) ();
 	  *reloc_addr = value;
 	  break;
 	default:
diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h
index 1d8da32c47..359a004271 100644
--- a/sysdeps/sparc/sparc32/dl-machine.h
+++ b/sysdeps/sparc/sparc32/dl-machine.h
@@ -425,11 +425,13 @@  elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
       *reloc_addr = value;
       break;
     case R_SPARC_IRELATIVE:
-      value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
+      if (__glibc_likely (!skip_ifunc))
+	value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
       *reloc_addr = value;
       break;
     case R_SPARC_JMP_IREL:
-      value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
+      if (__glibc_likely (!skip_ifunc))
+	value = ((Elf32_Addr (*) (int)) value) (GLRO(dl_hwcap));
       /* Fall thru */
     case R_SPARC_JMP_SLOT:
       {
diff --git a/sysdeps/sparc/sparc64/dl-machine.h b/sysdeps/sparc/sparc64/dl-machine.h
index ce1261d2a0..0acaf92a53 100644
--- a/sysdeps/sparc/sparc64/dl-machine.h
+++ b/sysdeps/sparc/sparc64/dl-machine.h
@@ -450,11 +450,13 @@  elf_machine_rela (struct link_map *map, const Elf64_Rela *reloc,
       *reloc_addr = value;
       break;
     case R_SPARC_IRELATIVE:
-      value = ((Elf64_Addr (*) (int)) value) (GLRO(dl_hwcap));
+      if (__glibc_likely (!skip_ifunc))
+	value = ((Elf64_Addr (*) (int)) value) (GLRO(dl_hwcap));
       *reloc_addr = value;
       break;
     case R_SPARC_JMP_IREL:
-      value = ((Elf64_Addr (*) (int)) value) (GLRO(dl_hwcap));
+      if (__glibc_likely (!skip_ifunc))
+	value = ((Elf64_Addr (*) (int)) value) (GLRO(dl_hwcap));
       /* 'high' is always zero, for large PLT entries the linker
 	 emits an R_SPARC_IRELATIVE.  */
 #ifdef RESOLVE_CONFLICT_FIND_MAP
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index f17f5fb7cd..58260c7876 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -512,7 +512,8 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 #  endif
 	case R_X86_64_IRELATIVE:
 	  value = map->l_addr + reloc->r_addend;
-	  value = ((ElfW(Addr) (*) (void)) value) ();
+	  if (__glibc_likely (!skip_ifunc))
+	    value = ((ElfW(Addr) (*) (void)) value) ();
 	  *reloc_addr = value;
 	  break;
 	default: