Add LD_PRELOAD_INIT_EARLY [BZ #14379]
Commit Message
Currently, DSOs preloaded with LD_PRELOAD are initialized after
linked-in DSOs, unless dependencies require otherwise.
However, in some 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.
Changing the default behaviour could lead to breakage, so add a new
environment variable, LD_PRELOAD_INIT_EARLY, the presence of which will
ask preloaded libraries to be initialized as early as possible and
finalized as late as possible. If multiple DSOs are preloaded, DSOs
earlier on the LD_PRELOAD list will be initalized earlier and finalized
later than DSOs present later on the list.
Note that dependencies are still taken into account: DSOs which the
preloaded DSO depends on are correctly initalized before it and
finalized after it.
Add tests to test both the current, default behaviour and the behaviour
when the new environment variable is set.
Test suite run on x86-64.
2019-02-14 Vincent Whitchurch <vincent.whitchurch@axis.com>
[BZ #14379]
* elf/Makefile (tests): Add tst-preload-initorder and
tst-preload-initorder-early and related rules.
* elf/dl-open.c: Pass new argument to _dl_map_object_deps().
* elf/dl-deps.c (_dl_map_object_deps): Based on new parameter,
initialize preloaded DSOs earlier and mark them for late finalization.
* elf/dl-fini.c (_dl_fini): Finalize DSOs marked for late finalization
later.
* elf/rtld.c: Handle environment variable LD_PRELOAD_INIT_EARLY and
pass new argument to _dl_map_object_deps().
* elf/tst-preload-initorder.c: New file.
* elf/tst-preload-initorder.exp: Likewise.
* elf/tst-preload-initorder-early.c: Likewise.
* elf/tst-preload-initorder-early.exp: Likewise.
* include/link.h (struct link_map): Add new field l_late_fini for late
finalization.
Comments
This patch is missing a NEWS update to discuss the new feature. (It seems
the manual generally lacks documentation of LD_* variables; if they were
documented, an update there would be needed as well.)
The commit message needs to discuss security issues (that applies to any
proposed change to glibc that makes its behavior depend on some
environment variable, probably we should add that point to the
contribution checklist). What effect does this have in setuid programs?
Is it ignored, by existing or new code? If not ignored, how can you
demonstrate that to be safe in all cases where LD_PRELOAD is handled and
safe for such programs?
(To confirm for anyone else looking at the patch: there is an FSF
copyright assignment on file from Axis Communications, for a list of
projects including glibc, dated 1999-05-28.)
* Vincent Whitchurch:
> Currently, DSOs preloaded with LD_PRELOAD are initialized after
> linked-in DSOs, unless dependencies require otherwise.
>
> However, in some 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.
>
> Changing the default behaviour could lead to breakage, so add a new
> environment variable, LD_PRELOAD_INIT_EARLY, the presence of which will
> ask preloaded libraries to be initialized as early as possible and
> finalized as late as possible. If multiple DSOs are preloaded, DSOs
> earlier on the LD_PRELOAD list will be initalized earlier and finalized
> later than DSOs present later on the list.
>
> Note that dependencies are still taken into account: DSOs which the
> preloaded DSO depends on are correctly initalized before it and
> finalized after it.
Maybe we should LD_PRELOAD to use this behavior instead? Isn't that
what users expect?
Thanks,
Florian
On Thu, Feb 14, 2019 at 05:43:26PM +0000, Joseph Myers wrote:
> This patch is missing a NEWS update to discuss the new feature. (It seems
> the manual generally lacks documentation of LD_* variables; if they were
> documented, an update there would be needed as well.)
I will add a NEWS entry.
> The commit message needs to discuss security issues (that applies to any
> proposed change to glibc that makes its behavior depend on some
> environment variable, probably we should add that point to the
> contribution checklist). What effect does this have in setuid programs?
> Is it ignored, by existing or new code? If not ignored, how can you
> demonstrate that to be safe in all cases where LD_PRELOAD is handled and
> safe for such programs?
It is not ignored right now, but there's no reason it shouldn't be.
Perhaps it would be better to have the new variable take a list of
libraries just like LD_PRELOAD does, instead of having it modify
LD_PRELOAD's behaviour? Then the security handling for the new variable
could be exactly the same as LD_PRELOAD's?
On Fri, Feb 15, 2019 at 12:43:54PM +0100, Florian Weimer wrote:
> * Vincent Whitchurch:
> > Currently, DSOs preloaded with LD_PRELOAD are initialized after
> > linked-in DSOs, unless dependencies require otherwise.
> >
> > However, in some 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.
> >
> > Changing the default behaviour could lead to breakage, so add a new
> > environment variable, LD_PRELOAD_INIT_EARLY, the presence of which will
> > ask preloaded libraries to be initialized as early as possible and
> > finalized as late as possible. If multiple DSOs are preloaded, DSOs
> > earlier on the LD_PRELOAD list will be initalized earlier and finalized
> > later than DSOs present later on the list.
> >
> > Note that dependencies are still taken into account: DSOs which the
> > preloaded DSO depends on are correctly initalized before it and
> > finalized after it.
>
> Maybe we should LD_PRELOAD to use this behavior instead? Isn't that
> what users expect?
It's certainly what I personally expect, but I feared that someone may
actually be depending on the current behaviour, hence the new variable.
But I can certainly just change LD_PRELOAD's default behaviour if you
think that there is no risk for that.
On 2/15/19 9:50 AM, Vincent Whitchurch wrote:
> On Fri, Feb 15, 2019 at 12:43:54PM +0100, Florian Weimer wrote:
>> * Vincent Whitchurch:
>>> Currently, DSOs preloaded with LD_PRELOAD are initialized after
>>> linked-in DSOs, unless dependencies require otherwise.
>>>
>>> However, in some 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.
>>>
>>> Changing the default behaviour could lead to breakage, so add a new
>>> environment variable, LD_PRELOAD_INIT_EARLY, the presence of which will
>>> ask preloaded libraries to be initialized as early as possible and
>>> finalized as late as possible. If multiple DSOs are preloaded, DSOs
>>> earlier on the LD_PRELOAD list will be initalized earlier and finalized
>>> later than DSOs present later on the list.
>>>
>>> Note that dependencies are still taken into account: DSOs which the
>>> preloaded DSO depends on are correctly initalized before it and
>>> finalized after it.
>>
>> Maybe we should LD_PRELOAD to use this behavior instead? Isn't that
>> what users expect?
>
> It's certainly what I personally expect, but I feared that someone may
> actually be depending on the current behaviour, hence the new variable.
> But I can certainly just change LD_PRELOAD's default behaviour if you
> think that there is no risk for that.
The order of initializers is not guaranteed unless there are explicit
dependencies. I would rather see what you describe becomes the *default*
behviour for LD_PRELOAD, and we should try to add a section in the manual
to describe this so we have documented the new behaviour.
Let me post the new manual text which is Ben Woodard's work and get that
in place first and then we can add to it.
On Fri, 15 Feb 2019, Carlos O'Donell wrote:
> Let me post the new manual text which is Ben Woodard's work and get that
> in place first and then we can add to it.
If that's the text previously posted in Oct 2017, note there were lots of
review comments that should be addressed for a reposting.
On 2/15/19 12:18 PM, Joseph Myers wrote:
> On Fri, 15 Feb 2019, Carlos O'Donell wrote:
>
>> Let me post the new manual text which is Ben Woodard's work and get that
>> in place first and then we can add to it.
>
> If that's the text previously posted in Oct 2017, note there were lots of
> review comments that should be addressed for a reposting.
It is similar text, but it has been updated. I will go through the review.
The text is something I want in RHEL7 anyway to help answer user questions,
and so we have some incentive to create the text.
@@ -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-preload-initorder-early \
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,8 @@ 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-preload-initorder-early-cmp.out \
$(objpfx)tst-initorder2-cmp.out $(objpfx)tst-unused-dep.out \
$(objpfx)tst-unused-dep-cmp.out
endif
@@ -773,6 +776,17 @@ $(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-preload-initorder-early: \
+ $(objpfx)tst-initorder2c.so
+$(objpfx)tst-preload-initorder.out $(objpfx)tst-preload-initorder-early.out: \
+ $(tst-preload-initorder-preloads:%=$(objpfx)%.so)
+tst-preload-initorder-ENV = \
+ LD_PRELOAD=$(subst $(empty) ,:,$(strip $(tst-preload-initorder-preloads:=.so)))
+tst-preload-initorder-early-ENV = \
+ LD_PRELOAD_INIT_EARLY=1 \
+ LD_PRELOAD=$(subst $(empty) ,:,$(strip $(tst-preload-initorder-preloads:=.so)))
+
$(objpfx)loadfail: $(libdl)
LDFLAGS-loadfail = -rdynamic
@@ -1343,19 +1357,33 @@ $(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-preload-initorder-early-cmp.out: \
+ tst-preload-initorder-early.exp $(objpfx)tst-preload-initorder-early.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
@@ -139,7 +139,8 @@ cannot load auxiliary `%s' because of empty dynamic string token " \
__result; })
static void
-preload (struct list *known, unsigned int *nlist, struct link_map *map)
+preload (struct list *known, unsigned int *nlist, struct link_map *map,
+ int late_fini)
{
known[*nlist].done = 0;
known[*nlist].map = map;
@@ -150,12 +151,13 @@ preload (struct list *known, unsigned int *nlist, struct link_map *map)
already put in the search list and avoid adding duplicate
elements later in the list. */
map->l_reserved = 1;
+ map->l_late_fini = late_fini;
}
void
_dl_map_object_deps (struct link_map *map,
struct link_map **preloads, unsigned int npreloads,
- int trace_mode, int open_mode)
+ int trace_mode, int open_mode, int preload_init_early)
{
struct list *known = __alloca (sizeof *known * (1 + npreloads + 1));
struct list *runp, *tail;
@@ -170,11 +172,11 @@ _dl_map_object_deps (struct link_map *map,
nlist = 0;
/* First load MAP itself. */
- preload (known, &nlist, map);
+ preload (known, &nlist, map, 0);
/* Add the preloaded items after MAP but before any of its dependencies. */
for (i = 0; i < npreloads; ++i)
- preload (known, &nlist, preloads[i]);
+ preload (known, &nlist, preloads[i], preload_init_early);
/* Terminate the lists. */
known[nlist - 1].next = NULL;
@@ -587,8 +589,32 @@ 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,
+ if (__glibc_unlikely (preload_init_early && npreloads
+ && nlist > npreloads + 1)) {
+ if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_IMPCALLS))
+ _dl_debug_printf ("\nearly preload init requested\n\n");
+
+ /* The binary itself will always be initialized last. */
+ memcpy (&l_initfini[0], &map->l_searchlist.r_list[0],
+ 1 * sizeof (struct link_map *));
+
+ /* Non-preloaded DSOs will be initialized after preloaded DSOs
+ * (unless dependencies mandate a different order). */
+ unsigned int nnormal = nlist - npreloads - 1;
+ memcpy (&l_initfini[1], &map->l_searchlist.r_list[1 + npreloads],
+ nnormal * sizeof (struct link_map *));
+
+ /* Insert the preloads in reverse order so that preloads earlier on the
+ * list are initialized earlier (again, unless dependencies require
+ * otherwise). */
+ struct link_map **base = &map->l_searchlist.r_list[1];
+ for (i = 0; i < npreloads; i++)
+ l_initfini[1 + nnormal + i] = base[npreloads - 1 - i];
+ } 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);
@@ -68,22 +68,45 @@ _dl_fini (void)
struct link_map *maps[nloaded];
unsigned int i;
- struct link_map *l;
+ bool want_late_fini = 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_late_fini))
+ want_late_fini = true;
+ /* Do not handle ld.so in secondary namespaces. */
+ if (l == l->l_real && !l->l_late_fini)
+ {
+ 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;
+ }
+ }
+
+ /* Late finalized DSOs should be processed in reverse order. */
+ if (__glibc_unlikely (want_late_fini))
+ for (l = last; l != NULL; l = l->l_prev)
+ if (l == l->l_real && l->l_late_fini)
+ {
+ 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;
@@ -258,7 +258,7 @@ dl_open_worker (void *a)
/* Load that object's dependencies. */
_dl_map_object_deps (new, NULL, 0, 0,
- mode & (__RTLD_DLOPEN | RTLD_DEEPBIND | __RTLD_AUDIT));
+ mode & (__RTLD_DLOPEN | RTLD_DEEPBIND | __RTLD_AUDIT), 0);
/* So far, so good. Now check the versions. */
for (unsigned int i = 0; i < new->l_searchlist.r_nlist; ++i)
@@ -828,6 +828,8 @@ static const char *preloadlist attribute_relro;
static int version_info attribute_relro;
/* The preload list passed as a command argument. */
static const char *preloadarg attribute_relro;
+/* Nonzero if preloaded objects should be initialized early. */
+static int preload_init_early attribute_relro;
/* The LD_PRELOAD environment variable gives list of libraries
separated by white space or colons that are loaded before the
@@ -1786,7 +1788,8 @@ ERROR: '%s': cannot process note segment.\n", _dl_argv[0]);
specified some libraries to load, these are inserted before the actual
dependencies in the executable's searchlist for symbol resolution. */
HP_TIMING_NOW (start);
- _dl_map_object_deps (main_map, preloads, npreloads, mode == trace, 0);
+ _dl_map_object_deps (main_map, preloads, npreloads, mode == trace, 0,
+ preload_init_early);
HP_TIMING_NOW (stop);
HP_TIMING_DIFF (diff, start, stop);
HP_TIMING_ACCUM_NT (load_time, diff);
@@ -2676,6 +2679,11 @@ process_envvars (enum mode *modep)
}
break;
+ case 18:
+ if (memcmp (envline, "PRELOAD_INIT_EARLY", 18) == 0)
+ preload_init_early = 1;
+ break;
+
case 20:
/* The mode of the dynamic linker can be set. */
if (memcmp (envline, "TRACE_LOADED_OBJECTS", 20) == 0)
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-initorder.c"
new file mode 100644
@@ -0,0 +1,9 @@
+init: d
+init: x
+init: y
+init: c
+main
+fini: c
+fini: y
+fini: x
+fini: d
new file mode 100644
@@ -0,0 +1 @@
+#include "tst-initorder.c"
new file mode 100644
@@ -0,0 +1,9 @@
+init: d
+init: c
+init: y
+init: x
+main
+fini: x
+fini: y
+fini: c
+fini: d
@@ -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_late_fini:1; /* Nonzero if DSO should be finalized late. */
#include <link_map.h>
@@ -872,7 +872,7 @@ extern struct link_map *_dl_map_object (struct link_map *loader,
extern void _dl_map_object_deps (struct link_map *map,
struct link_map **preloads,
unsigned int npreloads, int trace_mode,
- int open_mode)
+ int open_mode, int preloads_init_early)
attribute_hidden;
/* Cache the locations of MAP's hash table. */