Fix recursive dlopen.

The ability to recursively call dlopen is useful for malloc
implementations that wish to load other dynamic modules that
implement reentrant/AS-safe functions to use in their own
implementation.

Given that a user malloc implementation may be called by an
ongoing dlopen to allocate memory the user malloc
implementation interrupts dlopen and if it calls dlopen again
that's a reentrant call.

This patch fixes the issues with the ld.so.cache mapping
and the _r_debug assertion which prevent this from working
as expected.

See:
https://sourceware.org/ml/libc-alpha/2014-12/msg00446.html

10 files changed, 222 insertions, 21 deletions

diff --git a/ChangeLog b/ChangeLog
index 030491a..d83be7a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,22 @@
+2015-01-21  Carlos O'Donell  <carlos@redhat.com>
+
+	[BZ #17702]
+	* dlfcn/Makefile (tests): Add tst-rec-dlopen.
+	(modules-names): Add moddummy1 and moddummy2.
+	($(objpfx)tst-rec-dlopen): Define.
+	* dlfcn/moddummy1.c: New file.
+	* dlfcn/moddummy2.c: New file.
+	* dlfcn/tst-rec-dlopen.c: New file.
+	* elf/dl-cache.c (_dl_load_cache_lookup):
+	Return char*. Copy result with alloca/strcpy/strdup.
+	* elf/dl-load.c (_dl_map_object): _dl_load_cached_lookup
+	returns char*. Free cached. If not saving realname
+	free cached.
+	* elf/dl-open.c (dl_open_worker): Do not assert that
+	_r_debug->r_state is RT_CONSISTENT.
+	* sysdeps/generic/ldsodefs.h: _dl_load_cache_lookup
+	returns char*.
+
 2015-01-21  Torvald Riegel  <triegel@redhat.com>
 	    Carlos O'Donell  <carlos@redhat.com>
 
diff --git a/NEWS b/NEWS
index db1c282..136f16b 100644
--- a/NEWS
+++ b/NEWS
@@ -15,9 +15,10 @@ Version 2.21
   17485, 17501, 17506, 17508, 17522, 17555, 17570, 17571, 17572, 17573,
   17574, 17582, 17583, 17584, 17585, 17589, 17594, 17601, 17608, 17616,
   17625, 17630, 17633, 17634, 17635, 17647, 17653, 17657, 17658, 17664,
-  17665, 17668, 17682, 17717, 17719, 17722, 17723, 17724, 17725, 17732,
-  17733, 17744, 17745, 17746, 17747, 17748, 17775, 17777, 17780, 17781,
-  17782, 17791, 17793, 17796, 17797, 17803, 17806, 17834, 17844, 17848
+  17665, 17668, 17682, 17702, 17717, 17719, 17722, 17723, 17724, 17725,
+  17732, 17733, 17744, 17745, 17746, 17747, 17748, 17775, 17777, 17780,
+  17781, 17782, 17791, 17793, 17796, 17797, 17803, 17806, 17834, 17844,
+  17848
 
 * A new semaphore algorithm has been implemented in generic C code for all
   machines. Previous custom assembly implementations of semaphore were
diff --git a/dlfcn/Makefile b/dlfcn/Makefile
index fce4aae..363278a 100644
--- a/dlfcn/Makefile
+++ b/dlfcn/Makefile
@@ -36,13 +36,13 @@ endif
 ifeq (yes,$(build-shared))
 tests = glrefmain failtest tst-dladdr default errmsg1 tstcxaatexit \
 	bug-dlopen1 bug-dlsym1 tst-dlinfo bug-atexit1 bug-atexit2 \
-	bug-atexit3 tstatexit bug-dl-leaf
+	bug-atexit3 tstatexit bug-dl-leaf tst-rec-dlopen
 endif
 modules-names = glreflib1 glreflib2 glreflib3 failtestmod defaultmod1 \
 		defaultmod2 errmsg1mod modatexit modcxaatexit \
 		bug-dlsym1-lib1 bug-dlsym1-lib2 bug-atexit1-lib \
 		bug-atexit2-lib bug-atexit3-lib bug-dl-leaf-lib \
-		bug-dl-leaf-lib-cb
+		bug-dl-leaf-lib-cb moddummy1 moddummy2
 
 failtestmod.so-no-z-defs = yes
 glreflib2.so-no-z-defs = yes
@@ -139,3 +139,6 @@ $(objpfx)bug-dl-leaf: $(objpfx)bug-dl-leaf-lib.so
 $(objpfx)bug-dl-leaf.out: $(objpfx)bug-dl-leaf-lib-cb.so
 $(objpfx)bug-dl-leaf-lib.so: $(libdl)
 $(objpfx)bug-dl-leaf-lib-cb.so: $(objpfx)bug-dl-leaf-lib.so
+
+$(objpfx)tst-rec-dlopen: $(libdl)
+$(objpfx)tst-rec-dlopen.out: $(objpfx)moddummy1.so $(objpfx)moddummy2.so
diff --git a/dlfcn/moddummy1.c b/dlfcn/moddummy1.c
new file mode 100644
index 0000000..6e549fa
--- /dev/null
+++ b/dlfcn/moddummy1.c
@@ -0,0 +1,10 @@
+/* Provide a dummy DSO for tst-rec-dlopen to use.  */
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+dummy1 (void)
+{
+  printf ("Called dummy1()\n");
+  return 1;
+}
diff --git a/dlfcn/moddummy2.c b/dlfcn/moddummy2.c
new file mode 100644
index 0000000..cb4edc8
--- /dev/null
+++ b/dlfcn/moddummy2.c
@@ -0,0 +1,13 @@
+/* Provide a dummy DSO for tst-rec-dlopen to use.  */
+#include <stdio.h>
+#include <stdlib.h>
+
+int
+dummy2 (void)
+{
+  printf ("Called dummy2()\n");
+  /* If the outer dlopen is not dummy1 (becuase of some error)
+     then tst-rec-dlopen will see a value of -1 as the returned
+     result and fail.  */
+  return -1;
+}
diff --git a/dlfcn/tst-rec-dlopen.c b/dlfcn/tst-rec-dlopen.c
new file mode 100644
index 0000000..35b08d4
--- /dev/null
+++ b/dlfcn/tst-rec-dlopen.c
@@ -0,0 +1,144 @@
+/* Test recursive dlopen using malloc hooks.
+   Copyright (C) 1998-2014 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@cygnus.com>, 1998.
+
+   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 <stdio.h>
+#include <stdlib.h>
+#include <malloc.h>
+#include <dlfcn.h>
+
+#define DSO "moddummy1.so"
+#define FUNC "dummy1"
+
+#define DSO1 "moddummy2.so"
+#define FUNC1 "dummy2"
+
+/* Result of the called function.  */
+int func_result;
+
+/* Prototype for my hook.  */
+void *custom_malloc_hook (size_t, const void *);
+
+/* Pointer to old malloc hooks.  */
+void *(*old_malloc_hook) (size_t, const void *);
+
+/* Call function func_name in DSO dso_name via dlopen.  */
+void
+call_func (const char *dso_name, const char *func_name)
+{
+  int ret;
+  void *dso;
+  int (*func) (void);
+  char *err;
+
+  /* Open the DSO.  */
+  dso = dlopen (dso_name, RTLD_NOW|RTLD_GLOBAL);
+  if (dso == NULL)
+    {
+      err = dlerror ();
+      fprintf (stderr, "%s\n", err);
+      exit (1);
+    }
+  /* Clear any errors.  */
+  dlerror ();
+
+  /* Lookup func.  */
+  *(void **) (&func) = dlsym (dso, func_name);
+  if (func == NULL)
+    {
+      err = dlerror ();
+      if (err != NULL)
+        {
+	  fprintf (stderr, "%s\n", err);
+	  exit (1);
+        }
+    }
+  /* Call func.  */
+  func_result = (*func) ();
+
+  /* Close the library and look for errors too.  */
+  ret = dlclose (dso);
+  if (ret != 0)
+    {
+      err = dlerror ();
+      fprintf (stderr, "%s\n", err);
+      exit (1);
+    }
+
+}
+
+/* Empty hook that does nothing.  */
+void *
+custom_malloc_hook (size_t size, const void *caller)
+{
+  void *result;
+  /* Restore old hooks.  */
+  __malloc_hook = old_malloc_hook;
+  /* First call a function in another library via dlopen.  */
+  call_func (DSO1, FUNC1);
+  /* Called recursively.  */
+  result = malloc (size);
+  /* Restore new hooks.  */
+  __malloc_hook = custom_malloc_hook;
+  return result;
+}
+
+static int
+do_test (void)
+{
+  /* Save old hook.  */
+  old_malloc_hook = __malloc_hook;
+  /* Install new hook.  */
+  __malloc_hook = custom_malloc_hook;
+
+  /* Bug 17702 fixes two things:
+       * A recursive dlopen unmapping the ld.so.cache.
+       * An assertion that _r_debug is RT_CONSISTENT at entry to dlopen.
+     We can only test the latter. Testing the former requires modifying
+     ld.so.conf to cache the dummy libraries, then running ldconfig,
+     then run the test. If you do all of that (and glibc's test
+     infrastructure doesn't support that yet) then the test will
+     SEGFAULT without the fix. If you don't do that, then the test
+     will abort because of the assert described in detail below.  */
+  call_func (DSO, FUNC);
+
+  /* Restore old hook.  */
+  __malloc_hook = old_malloc_hook;
+
+  /* The function dummy2() is called by the malloc hook. Check to
+     see that it was called. This ensures the second recursive
+     dlopen happened and we called the function in that library.
+     Before the fix you either get a SIGSEGV when accessing mmap'd
+     ld.so.cache data or an assertion failure about _r_debug not
+     beint RT_CONSISTENT.  We don't test for the SIGSEGV since it
+     would require finding moddummy1 or moddummy2 in the cache and
+     we don't have any infrastructure to test that, but the _r_debug
+     assertion triggers.  */
+  printf ("Returned result is %d\n", func_result);
+  if (func_result <= 0)
+    {
+      printf ("FAIL: Function call_func() not called.\n");
+      exit (1);
+    }
+
+  printf ("PASS: Function call_func() called more than once.\n");
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 80d5e30..dec49bc 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -174,9 +174,12 @@ _dl_cache_libcmp (const char *p1, const char *p2)
 
 /* Look up NAME in ld.so.cache and return the file name stored there, or null
    if none is found.  The cache is loaded if it was not already.  If loading
-   the cache previously failed there will be no more attempts to load it.  */
-
-const char *
+   the cache previously failed there will be no more attempts to load it.
+   The caller is responsible for freeing the returned string.  The ld.so.cache
+   may be unmapped at any time by a completing recursive dlopen and
+   this function must take care that it does not return references to
+   any data in the mapping.  */
+char *
 internal_function
 _dl_load_cache_lookup (const char *name)
 {
@@ -289,7 +292,17 @@ _dl_load_cache_lookup (const char *name)
       && best != NULL)
     _dl_debug_printf ("  trying file=%s\n", best);
 
-  return best;
+  if (best == NULL)
+    return NULL;
+
+  /* The double copy is *required* since malloc may be interposed
+     and call dlopen itself whose completion would unmap the data
+     we are accessing. Therefore we must make the copy of the
+     mapping data without using malloc.  */
+  char *temp;
+  temp = alloca (strlen (best) + 1);
+  strcpy (temp, best);
+  return strdup (temp);
 }
 
 #ifndef MAP_COPY
diff --git a/elf/dl-load.c b/elf/dl-load.c
index d6726b6..73174aa 100644
--- a/elf/dl-load.c
+++ b/elf/dl-load.c
@@ -2051,7 +2051,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 	{
 	  /* Check the list of libraries in the file /etc/ld.so.cache,
 	     for compatibility with Linux's ldconfig program.  */
-	  const char *cached = _dl_load_cache_lookup (name);
+	  char *cached = _dl_load_cache_lookup (name);
 
 	  if (cached != NULL)
 	    {
@@ -2075,6 +2075,7 @@ _dl_map_object (struct link_map *loader, const char *name,
 		      if (memcmp (cached, dirp, system_dirs_len[cnt]) == 0)
 			{
 			  /* The prefix matches.  Don't use the entry.  */
+			  free (cached);
 			  cached = NULL;
 			  break;
 			}
@@ -2092,14 +2093,9 @@ _dl_map_object (struct link_map *loader, const char *name,
 				    LA_SER_CONFIG, mode, &found_other_class,
 				    false);
 		  if (__glibc_likely (fd != -1))
-		    {
-		      realname = __strdup (cached);
-		      if (realname == NULL)
-			{
-			  __close (fd);
-			  fd = -1;
-			}
-		    }
+		    realname = cached;
+		  else
+		    free (cached);
 		}
 	    }
 	}
diff --git a/elf/dl-open.c b/elf/dl-open.c
index c358fff..47b4cb5 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -217,7 +217,9 @@ dl_open_worker (void *a)
 	args->nsid = call_map->l_ns;
     }
 
-  assert (_dl_debug_initialize (0, args->nsid)->r_state == RT_CONSISTENT);
+  /* One might be tempted to assert that we are RT_CONSISTENT at this point, but that
+     may not be true if this is a recursive call to dlopen.  */
+  _dl_debug_initialize (0, args->nsid);
 
   /* Load the named object.  */
   struct link_map *new;
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index 96dec1f..d738988 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -905,8 +905,8 @@ extern const struct r_strlenpair *_dl_important_hwcaps (const char *platform,
      internal_function;
 
 /* Look up NAME in ld.so.cache and return the file name stored there,
-   or null if none is found.  */
-extern const char *_dl_load_cache_lookup (const char *name)
+   or null if none is found.  Caller must free returned string.  */
+extern char *_dl_load_cache_lookup (const char *name)
      internal_function;
 
 /* If the system does not support MAP_COPY we cannot leave the file open