[PATCH v2] Initialize preloaded DSOs earlier [BZ #14379]

[PATCH v2] Initialize preloaded DSOs earlier [BZ #14379]

[Vincent Whitchurch]

Currently, preloaded DSOs are initialized after linked-in DSOs.

However, in many cases it is desirable that preloaded DSO are
initialized before linked-in DSOs (unless dependencies require
otherwise).  For example, when malloc is overloaded using a preloaded
DSO for the purpose of heap profiling, we ideally want the preloaded DSO
to be initialized before other DSOs so that it has a chance to set up
its accounting code before their initializers are called.

So ensure that preloaded libraries are initialized as early as possible
and finalized as late as possible.  Dependencies are still taken into
account: DSOs which the preloaded DSO depends on are correctly
initialized before it and finalized after it.

If multiple DSOs are preloaded, DSOs earlier on the preload list will be
initialized earlier and finalized later than DSOs present later on the
list, analogous to how DSOs earlier on the preload list are searched for
symbols before DSOs later on the preload list.

Add a test case for this.

Test suite run on x86-64.

2019-02-21  Vincent Whitchurch  <vincent.whitchurch@axis.com>

	[BZ #14379]
	* elf/Makefile (tests): Add tst-preload-initorder and related rules.
	* elf/dl-deps.c (_dl_map_object_deps): Initialize preloaded DSOs
	earlier.
	* elf/dl-fini.c (_dl_fini): Finalize preloaded DSOs later.
	* elf/rtld.c (do_preload): Set l_preload flag on preloaded DSOs.
	* elf/tst-preload-initorder.c: New file.
	* elf/tst-preload-initorder.exp: Likewise.
	* include/link.h (struct link_map): Add new field l_preload.

diff --git a/NEWS b/NEWS
index 0a3b6c7a5a1..da3a0d7d200 100644
--- a/NEWS
+++ b/NEWS
@@ -22,6 +22,9 @@ Deprecated and removed features, and other changes affecting compatibility:
   definitions in libc will be used automatically, which have been available
   since glibc 2.17.
 
+* Preloaded shared objects are now initialized as early as dependencies allow,
+  and finalized as late as dependencies allow.
+
 Changes to build and runtime requirements:
 
 * GCC 6.2 or later is required to build the GNU C Library.
diff --git a/elf/Makefile b/elf/Makefile
index 5c625b89fa3..c80645224ff 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -183,6 +183,7 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \
 	 tst-unique1 tst-unique2 $(if $(CXX),tst-unique3 tst-unique4 \
 	 tst-nodelete) \
 	 tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
+	 tst-preload-initorder \
 	 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
 	 tst-nodelete2 tst-audit11 tst-audit12 tst-dlsym-error tst-noload \
 	 tst-latepthread tst-tls-manydynamic tst-nodelete-dlclose \
@@ -266,7 +267,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
 		tst-initordera2 tst-initorderb2 \
 		tst-initordera3 tst-initordera4 \
 		tst-initorder2a tst-initorder2b tst-initorder2c \
-		tst-initorder2d \
+		tst-initorder2d tst-initorder2x tst-initorder2y \
 		tst-relsort1mod1 tst-relsort1mod2 tst-array2dep \
 		tst-array5dep tst-null-argv-lib \
 		tst-tlsalign-lib tst-nodelete-opened-lib tst-nodelete2mod \
@@ -369,6 +370,7 @@ tests-special += $(objpfx)order-cmp.out $(objpfx)tst-array1-cmp.out \
 		 $(objpfx)tst-array4-cmp.out $(objpfx)tst-array5-cmp.out \
 		 $(objpfx)tst-array5-static-cmp.out $(objpfx)order2-cmp.out \
 		 $(objpfx)tst-initorder-cmp.out \
+		 $(objpfx)tst-preload-initorder-cmp.out \
 		 $(objpfx)tst-initorder2-cmp.out $(objpfx)tst-unused-dep.out \
 		 $(objpfx)tst-unused-dep-cmp.out
 endif
@@ -773,6 +775,13 @@ $(objpfx)preloadtest.out: $(preloadtest-preloads:%=$(objpfx)%.so)
 preloadtest-ENV = \
   LD_PRELOAD=$(subst $(empty) ,:,$(strip $(preloadtest-preloads:=.so)))
 
+tst-preload-initorder-preloads = tst-initorder2x tst-initorder2y
+$(objpfx)tst-preload-initorder: $(objpfx)tst-initorder2c.so
+$(objpfx)tst-preload-initorder.out: \
+  $(tst-preload-initorder-preloads:%=$(objpfx)%.so)
+tst-preload-initorder-ENV = \
+  LD_PRELOAD=$(subst $(empty) ,:,$(strip $(tst-preload-initorder-preloads:=.so)))
+
 $(objpfx)loadfail: $(libdl)
 LDFLAGS-loadfail = -rdynamic
 
@@ -1343,19 +1352,28 @@ $(objpfx)tst-initorder-cmp.out: tst-initorder.exp $(objpfx)tst-initorder.out
 	cmp $^ > $@; \
 	$(evaluate-test)
 
+$(objpfx)tst-preload-initorder-cmp.out: \
+  tst-preload-initorder.exp $(objpfx)tst-preload-initorder.out
+	cmp $^ > $@; \
+	$(evaluate-test)
+
 $(objpfx)tst-initorder2: $(objpfx)tst-initorder2a.so $(objpfx)tst-initorder2d.so $(objpfx)tst-initorder2c.so
 $(objpfx)tst-initorder2a.so: $(objpfx)tst-initorder2b.so
 $(objpfx)tst-initorder2b.so: $(objpfx)tst-initorder2c.so
 $(objpfx)tst-initorder2c.so: $(objpfx)tst-initorder2d.so
+$(objpfx)tst-initorder2x.so: $(objpfx)tst-initorder2d.so
+$(objpfx)tst-initorder2y.so: $(objpfx)tst-initorder2d.so
 LDFLAGS-tst-initorder2 = $(no-as-needed)
 LDFLAGS-tst-initorder2a.so = $(no-as-needed)
 LDFLAGS-tst-initorder2b.so = $(no-as-needed)
 LDFLAGS-tst-initorder2c.so = $(no-as-needed)
+LDFLAGS-tst-initorder2x.so = $(no-as-needed)
+LDFLAGS-tst-initorder2y.so = $(no-as-needed)
 define o-iterator-doit
 $(objpfx)tst-initorder2$o.os: tst-initorder2.c; \
 $$(compile-command.c) -DNAME=\"$o\"
 endef
-object-suffixes-left := a b c d
+object-suffixes-left := a b c d x y
 include $(o-iterator)
 
 $(objpfx)tst-initorder2-cmp.out: tst-initorder2.exp $(objpfx)tst-initorder2.out
diff --git a/elf/dl-deps.c b/elf/dl-deps.c
index e12c353158a..5cc9b2d3646 100644
--- a/elf/dl-deps.c
+++ b/elf/dl-deps.c
@@ -587,8 +587,36 @@ Filters not supported with LD_TRACE_PRELINKING"));
 
   /* Sort the initializer list to take dependencies into account.  The binary
      itself will always be initialize last.  */
-  memcpy (l_initfini, map->l_searchlist.r_list,
-	  nlist * sizeof (struct link_map *));
+  if (__glibc_unlikely (preloads > 0 && nlist > npreloads + 1))
+    {
+      unsigned int src, dest = 0;
+
+      /* The binary itself and non-preloaded DSOs will be initialized after
+	 preloaded DSOs.  */
+      for (src = 0; src < nlist; src++)
+	{
+	  struct link_map *dso = map->l_searchlist.r_list[src];
+
+	  if (!dso->l_preload)
+	    l_initfini[dest++] = dso;
+	}
+
+	/* Insert preloads in reverse order so that preloads earlier on the
+	   list are initialized earlier.  */
+      for (src = nlist - 1; src > 0; src--)
+	{
+	  struct link_map *dso = map->l_searchlist.r_list[src];
+
+	  if (dso->l_preload)
+	    l_initfini[dest++] = dso;
+	}
+    }
+  else
+    {
+      memcpy (l_initfini, map->l_searchlist.r_list,
+	      nlist * sizeof (struct link_map *));
+    }
+
   /* We can skip looking for the binary itself which is at the front of
      the search list.  */
   _dl_sort_maps (&l_initfini[1], nlist - 1, NULL, false);
diff --git a/elf/dl-fini.c b/elf/dl-fini.c
index 1e55d398149..b4cb3887110 100644
--- a/elf/dl-fini.c
+++ b/elf/dl-fini.c
@@ -68,22 +68,45 @@ _dl_fini (void)
 	  struct link_map *maps[nloaded];
 
 	  unsigned int i;
-	  struct link_map *l;
+	  bool have_preloads = false;
+	  struct link_map *l, *last = NULL;
 	  assert (nloaded != 0 || GL(dl_ns)[ns]._ns_loaded == NULL);
 	  for (l = GL(dl_ns)[ns]._ns_loaded, i = 0; l != NULL; l = l->l_next)
-	    /* Do not handle ld.so in secondary namespaces.  */
-	    if (l == l->l_real)
-	      {
-		assert (i < nloaded);
-
-		maps[i] = l;
-		l->l_idx = i;
-		++i;
-
-		/* Bump l_direct_opencount of all objects so that they
-		   are not dlclose()ed from underneath us.  */
-		++l->l_direct_opencount;
-	      }
+	    {
+	      last = l;
+	      if (__glibc_unlikely (l->l_preload))
+		have_preloads = true;
+	      /* Do not handle ld.so in secondary namespaces.  */
+	      if (l == l->l_real && !l->l_preload)
+	        {
+		  assert (i < nloaded);
+
+		  maps[i] = l;
+		  l->l_idx = i;
+		  ++i;
+
+		  /* Bump l_direct_opencount of all objects so that they
+		     are not dlclose()ed from underneath us.  */
+		  ++l->l_direct_opencount;
+		}
+	    }
+
+	  /* Preloaded DSOs should be finalized late and in reverse order.  */
+	  if (__glibc_unlikely (have_preloads))
+	    for (l = last; l != NULL; l = l->l_prev)
+	      if (l == l->l_real && l->l_preload)
+	        {
+		  assert (i < nloaded);
+
+		  maps[i] = l;
+		  l->l_idx = i;
+		  ++i;
+
+	          /* Bump l_direct_opencount of all objects so that they
+	             are not dlclose()ed from underneath us.  */
+	          ++l->l_direct_opencount;
+	        }
+
 	  assert (ns != LM_ID_BASE || i == nloaded);
 	  assert (ns == LM_ID_BASE || i == nloaded || i == nloaded - 1);
 	  unsigned int nmaps = i;
diff --git a/elf/rtld.c b/elf/rtld.c
index c1cc1b01f2d..ce7a1ed234b 100644
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -770,8 +770,12 @@ ERROR: ld.so: object '%s' from %s cannot be preloaded (%s): ignored.\n",
 	 the libc's malloc is used.  */
     }
   else if (GL(dl_ns)[LM_ID_BASE]._ns_nloaded != old_nloaded)
-    /* It is no duplicate.  */
-    return 1;
+    {
+      args.map->l_preload = 1;
+
+      /* It is no duplicate.  */
+      return 1;
+    }
 
   /* Nothing loaded.  */
   return 0;
diff --git a/elf/tst-preload-initorder.c b/elf/tst-preload-initorder.c
new file mode 100644
index 00000000000..4305ba79763
--- /dev/null
+++ b/elf/tst-preload-initorder.c
@@ -0,0 +1 @@
+#include "tst-initorder.c"
diff --git a/elf/tst-preload-initorder.exp b/elf/tst-preload-initorder.exp
new file mode 100644
index 00000000000..a163dc3ec09
--- /dev/null
+++ b/elf/tst-preload-initorder.exp
@@ -0,0 +1,9 @@
+init: d
+init: x
+init: y
+init: c
+main
+fini: c
+fini: y
+fini: x
+fini: d
diff --git a/include/link.h b/include/link.h
index 736e1d72aec..3ac11277801 100644
--- a/include/link.h
+++ b/include/link.h
@@ -202,6 +202,7 @@ struct link_map
     unsigned int l_free_initfini:1; /* Nonzero if l_initfini can be
 				       freed, ie. not allocated with
 				       the dummy malloc in ld.so.  */
+    unsigned int l_preload:1; /* Nonzero if DSO has been preloaded.  */
 
 #include <link_map.h>
 

Re: [PATCH v2] Initialize preloaded DSOs earlier [BZ #14379]

[Florian Weimer]

* Vincent Whitchurch:

> Currently, preloaded DSOs are initialized after linked-in DSOs.
>
> However, in many cases it is desirable that preloaded DSO are
> initialized before linked-in DSOs (unless dependencies require
> otherwise).  For example, when malloc is overloaded using a preloaded
> DSO for the purpose of heap profiling, we ideally want the preloaded DSO
> to be initialized before other DSOs so that it has a chance to set up
> its accounting code before their initializers are called.
>
> So ensure that preloaded libraries are initialized as early as possible
> and finalized as late as possible.  Dependencies are still taken into
> account: DSOs which the preloaded DSO depends on are correctly
> initialized before it and finalized after it.
>
> If multiple DSOs are preloaded, DSOs earlier on the preload list will be
> initialized earlier and finalized later than DSOs present later on the
> list, analogous to how DSOs earlier on the preload list are searched for
> symbols before DSOs later on the preload list.
>
> Add a test case for this.
>
> Test suite run on x86-64.

Rich Felker shared this insightful comment:

  <https://sourceware.org/bugzilla/show_bug.cgi?id=14379#c9>

I'm no longer sure if the feature is a good idea after all (whether
applied to all LD_PRELOAD objects, or a subset of them using a new
environment variable).  Sorry.

Thanks,
Florian