* [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272]
@ 2019-10-24 10:34 David Kilroy
2019-10-24 10:34 ` [PATCH v2 2/3] elf: avoid redundant sort in dlopen David Kilroy
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: David Kilroy @ 2019-10-24 10:34 UTC (permalink / raw)
To: libc-alpha; +Cc: nd
This series fixes up the patchset for the comments raised so far.
Patches 2 and 3 are unchanged.
v2:
- code formatting fixups
- add dependency of test output on filtee library
- tests changed to use the test framework
The main outstanding question is whether it is valid to use l_initfini
to do the relocations, or whether we need to stick to following
new->l_next (and new->l_prev). I'm reasonably confident that
l_initfini contains all the objects in new->l_next. See
https://sourceware.org/ml/libc-alpha/2019-10/msg00659.html
The majority of the discussion on this series has been around how
filter objects are specified, and the alternatives to using filter
objects. The most persuasive is to use a version_script to specify the
ABI in the link library (.so), and setup the soname symlink (.so.1) to
point to the filtee. This relies on:
* library versioning being used.
* dlopen() calls must use the soname.
* if there are multiple implementors, they need to be consistent in
versioning the library.
I think these are reasonable requirements, though may be difficult to
ensure in the field.
Independent of the alternatives, I'd still like to advocate fixing
dlopen for filter objects. glibc currently works (with various
provisos) with filter objects when the application is linked against
one. Not being able to work with the same library via dlopen is
unexpected.
David Kilroy (3):
elf: Allow dlopen of filter object to work [BZ #16272]
elf: avoid redundant sort in dlopen
elf: avoid stack allocation in dl_open_worker
elf/Makefile | 13 +++++++++++--
elf/dl-deps.c | 35 ++++++++++++++++++++++++++---------
elf/dl-open.c | 32 +++++++++++++++-----------------
elf/tst-filterobj-dlopen.c | 39 +++++++++++++++++++++++++++++++++++++++
elf/tst-filterobj-flt.c | 24 ++++++++++++++++++++++++
elf/tst-filterobj-lib.c | 24 ++++++++++++++++++++++++
elf/tst-filterobj-lib.h | 18 ++++++++++++++++++
elf/tst-filterobj.c | 36 ++++++++++++++++++++++++++++++++++++
8 files changed, 193 insertions(+), 28 deletions(-)
create mode 100644 elf/tst-filterobj-dlopen.c
create mode 100644 elf/tst-filterobj-flt.c
create mode 100644 elf/tst-filterobj-lib.c
create mode 100644 elf/tst-filterobj-lib.h
create mode 100644 elf/tst-filterobj.c
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/3] elf: avoid redundant sort in dlopen 2019-10-24 10:34 [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy @ 2019-10-24 10:34 ` David Kilroy 2019-10-24 10:34 ` [PATCH v2 3/3] elf: avoid stack allocation in dl_open_worker David Kilroy ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: David Kilroy @ 2019-10-24 10:34 UTC (permalink / raw) To: libc-alpha; +Cc: nd l_initfini is already sorted by dependency in _dl_map_object_deps(), so avoid sorting again in dl_open_worker(). Tested by running the testsuite on x86_64. --- elf/dl-open.c | 1 - 1 file changed, 1 deletion(-) diff --git a/elf/dl-open.c b/elf/dl-open.c index 03f2b86..4df2e3f 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -326,7 +326,6 @@ dl_open_worker (void *a) l = new->l_initfini[++j]; } while (l != NULL); - _dl_sort_maps (maps, nmaps, NULL, false); int relocation_in_progress = 0; -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] elf: avoid stack allocation in dl_open_worker 2019-10-24 10:34 [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy 2019-10-24 10:34 ` [PATCH v2 2/3] elf: avoid redundant sort in dlopen David Kilroy @ 2019-10-24 10:34 ` David Kilroy 2019-10-24 10:34 ` [PATCH v2 1/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy 2019-11-06 16:03 ` [PATCH v2 0/3] " David Kilroy 3 siblings, 0 replies; 6+ messages in thread From: David Kilroy @ 2019-10-24 10:34 UTC (permalink / raw) To: libc-alpha; +Cc: nd As the sort was removed, there's no need to keep a separate map of links. Instead, when relocating objects iterate over l_initfini directly. This allows us to remove the loop copying l_initfini elements into map. We still need a loop to identify the first and last elements that need relocation. Tested by running the testsuite on x86_64. --- elf/dl-open.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/elf/dl-open.c b/elf/dl-open.c index 4df2e3f..4826e25 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -304,34 +304,30 @@ dl_open_worker (void *a) /* Sort the objects by dependency for the relocation process. This allows IFUNC relocations to work and it also means copy relocation of dependencies are if necessary overwritten. */ - unsigned int nmaps = 0; + unsigned int first = UINT_MAX; + unsigned int last = 0; unsigned int j = 0; struct link_map *l = new->l_initfini[0]; do { if (! l->l_real->l_relocated) - ++nmaps; - l = new->l_initfini[++j]; - } - while (l != NULL); - /* Stack allocation is limited by the number of loaded objects. */ - struct link_map *maps[nmaps]; - nmaps = 0; - j = 0; - l = new->l_initfini[0]; - do - { - if (! l->l_real->l_relocated) - maps[nmaps++] = l; + { + if (first == UINT_MAX) + first = j; + last = j + 1; + } l = new->l_initfini[++j]; } while (l != NULL); int relocation_in_progress = 0; - for (unsigned int i = nmaps; i-- > 0; ) + for (unsigned int i = last; i-- > first; ) { - l = maps[i]; + l = new->l_initfini[i]; + + if (l->l_real->l_relocated) + continue; if (! relocation_in_progress) { -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] elf: Allow dlopen of filter object to work [BZ #16272] 2019-10-24 10:34 [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy 2019-10-24 10:34 ` [PATCH v2 2/3] elf: avoid redundant sort in dlopen David Kilroy 2019-10-24 10:34 ` [PATCH v2 3/3] elf: avoid stack allocation in dl_open_worker David Kilroy @ 2019-10-24 10:34 ` David Kilroy 2019-11-06 16:03 ` [PATCH v2 0/3] " David Kilroy 3 siblings, 0 replies; 6+ messages in thread From: David Kilroy @ 2019-10-24 10:34 UTC (permalink / raw) To: libc-alpha; +Cc: nd _dl_map_object_deps assumes that map will always be at the beginning of the search list in l_searchlist.r_list[]. This is not true if map is a filter object, in which case the filtees are inserted before map. Fix this by: * avoiding relocation dependencies of map by setting l_reserved to 0 and otherwise processing the rest of the search list. * ensuring that map remains at the beginning of l_initfini - the list of things that need initialisation (and destruction). Do this by splitting the copy up. This may not be required, but matches the initialization order without dlopen. With that in place, the filtee objects are not getting relocations applied when dlopen is called. Modify dl_open_worker to attempt relocations on new->l_initfini instead of objects referenced by new->l_next (the filtees are in l_prev). Add tests to verify that symbols resolve to the filtee implementation when filter objects are used, both as a normal link and when dlopen'd. Tested by running the testsuite on x86_64. --- elf/Makefile | 13 +++++++++++-- elf/dl-deps.c | 35 ++++++++++++++++++++++++++--------- elf/dl-open.c | 11 +++++++---- elf/tst-filterobj-dlopen.c | 39 +++++++++++++++++++++++++++++++++++++++ elf/tst-filterobj-flt.c | 24 ++++++++++++++++++++++++ elf/tst-filterobj-lib.c | 24 ++++++++++++++++++++++++ elf/tst-filterobj-lib.h | 18 ++++++++++++++++++ elf/tst-filterobj.c | 36 ++++++++++++++++++++++++++++++++++++ 8 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 elf/tst-filterobj-dlopen.c create mode 100644 elf/tst-filterobj-flt.c create mode 100644 elf/tst-filterobj-lib.c create mode 100644 elf/tst-filterobj-lib.h create mode 100644 elf/tst-filterobj.c diff --git a/elf/Makefile b/elf/Makefile index 5e4cdb4..3292335 100644 --- a/elf/Makefile +++ b/elf/Makefile @@ -193,7 +193,8 @@ tests += restest1 preloadtest loadfail multiload origtest resolvfail \ tst-debug1 tst-main1 tst-absolute-sym tst-absolute-zero tst-big-note \ tst-unwind-ctor tst-unwind-main tst-audit13 \ tst-sonamemove-link tst-sonamemove-dlopen tst-dlopen-tlsmodid \ - tst-dlopen-self + tst-dlopen-self \ + tst-filterobj tst-filterobj-dlopen # reldep9 tests-internal += loadtest unload unload2 circleload1 \ neededtest neededtest2 neededtest3 neededtest4 \ @@ -281,7 +282,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \ tst-main1mod tst-libc_dlvsym-dso tst-absolute-sym-lib \ tst-absolute-zero-lib tst-big-note-lib tst-unwind-ctor-lib \ tst-audit13mod1 tst-sonamemove-linkmod1 \ - tst-sonamemove-runmod1 tst-sonamemove-runmod2 + tst-sonamemove-runmod1 tst-sonamemove-runmod2 \ + tst-filterobj-flt tst-filterobj-lib + # Most modules build with _ISOMAC defined, but those filtered out # depend on internal headers. modules-names-tests = $(filter-out ifuncmod% tst-libc_dlvsym-dso tst-tlsmod%,\ @@ -1569,3 +1572,9 @@ $(objpfx)tst-big-note-lib.so: $(objpfx)tst-big-note-lib.o $(objpfx)tst-unwind-ctor: $(objpfx)tst-unwind-ctor-lib.so CFLAGS-tst-unwind-main.c += -funwind-tables -DUSE_PTHREADS=0 + +LDFLAGS-tst-filterobj-flt.so = -Wl,--filter=$(objpfx)tst-filterobj-lib.so +$(objpfx)tst-filterobj: $(objpfx)tst-filterobj-flt.so | $(objpfx)tst-filterobj-lib.so +$(objpfx)tst-filterobj-dlopen: $(libdl) | $(objpfx)tst-filterobj-lib.so +$(objpfx)tst-filterobj.out: $(objpfx)tst-filterobj-lib.so +$(objpfx)tst-filterobj-dlopen.out: $(objpfx)tst-filterobj-lib.so diff --git a/elf/dl-deps.c b/elf/dl-deps.c index c29b988..bb85c83 100644 --- a/elf/dl-deps.c +++ b/elf/dl-deps.c @@ -550,13 +550,14 @@ Filters not supported with LD_TRACE_PRELINKING")); } /* Maybe we can remove some relocation dependencies now. */ - assert (map->l_searchlist.r_list[0] == map); struct link_map_reldeps *l_reldeps = NULL; if (map->l_reldeps != NULL) { - for (i = 1; i < nlist; ++i) + for (i = 0; i < nlist; ++i) map->l_searchlist.r_list[i]->l_reserved = 1; + /* Avoid removing relocation dependencies of the main binary. */ + map->l_reserved = 0; struct link_map **list = &map->l_reldeps->list[0]; for (i = 0; i < map->l_reldeps->act; ++i) if (list[i]->l_reserved) @@ -581,16 +582,32 @@ Filters not supported with LD_TRACE_PRELINKING")); } } - for (i = 1; i < nlist; ++i) + for (i = 0; i < nlist; ++i) map->l_searchlist.r_list[i]->l_reserved = 0; } - /* 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 *)); - /* We can skip looking for the binary itself which is at the front of - the search list. */ + /* Sort the initializer list to take dependencies into account. Always + initialize the binary itself last. First, find it in the search list. */ + for (i = 0; i < nlist; ++i) + if (map->l_searchlist.r_list[i] == map) + break; + assert (i < nlist); + if (i > 0) + { + /* Copy the binary into position 0. */ + memcpy (l_initfini, &map->l_searchlist.r_list[i], + sizeof (struct link_map *)); + /* Copy the filtees. */ + memcpy (&l_initfini[1], map->l_searchlist.r_list, + i * sizeof (struct link_map *)); + /* Copy the remainder. */ + memcpy (&l_initfini[i + 1], &map->l_searchlist.r_list[i + 1], + (nlist - i - 1) * sizeof (struct link_map *)); + } + else + memcpy (l_initfini, map->l_searchlist.r_list, + nlist * sizeof (struct link_map *)); + _dl_sort_maps (&l_initfini[1], nlist - 1, NULL, false); /* Terminate the list of dependencies. */ diff --git a/elf/dl-open.c b/elf/dl-open.c index a9fd4cb..03f2b86 100644 --- a/elf/dl-open.c +++ b/elf/dl-open.c @@ -305,22 +305,25 @@ dl_open_worker (void *a) allows IFUNC relocations to work and it also means copy relocation of dependencies are if necessary overwritten. */ unsigned int nmaps = 0; - struct link_map *l = new; + unsigned int j = 0; + struct link_map *l = new->l_initfini[0]; do { if (! l->l_real->l_relocated) ++nmaps; - l = l->l_next; + l = new->l_initfini[++j]; } while (l != NULL); + /* Stack allocation is limited by the number of loaded objects. */ struct link_map *maps[nmaps]; nmaps = 0; - l = new; + j = 0; + l = new->l_initfini[0]; do { if (! l->l_real->l_relocated) maps[nmaps++] = l; - l = l->l_next; + l = new->l_initfini[++j]; } while (l != NULL); _dl_sort_maps (maps, nmaps, NULL, false); diff --git a/elf/tst-filterobj-dlopen.c b/elf/tst-filterobj-dlopen.c new file mode 100644 index 0000000..81eed0f --- /dev/null +++ b/elf/tst-filterobj-dlopen.c @@ -0,0 +1,39 @@ +/* Test for BZ16272, dlopen'ing a filter object. + Ensure that symbols from the filter object resolve to the filtee. + + 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/>. */ + +#include <stdio.h> +#include "support/check.h" +#include "support/xdlfcn.h" + +static int do_test (void) +{ + void *lib = xdlopen ("tst-filterobj-flt.so", RTLD_LAZY); + char *(*fn)(void) = xdlsym (lib, "get_text"); + const char* text = fn (); + + printf ("%s\n", text); + + /* Verify the text matches what we expect from the filtee */ + TEST_COMPARE_STRING (text, "Hello from filtee (PASS)"); + + return 0; +} + +#include "support/test-driver.c" diff --git a/elf/tst-filterobj-flt.c b/elf/tst-filterobj-flt.c new file mode 100644 index 0000000..b4e10b2 --- /dev/null +++ b/elf/tst-filterobj-flt.c @@ -0,0 +1,24 @@ +/* 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/>. */ + +#include "tst-filterobj-lib.h" + +/* We never want to see the output of the filter object */ +const char *get_text (void) +{ + return "Hello from filter object (FAIL)"; +} diff --git a/elf/tst-filterobj-lib.c b/elf/tst-filterobj-lib.c new file mode 100644 index 0000000..07e2348 --- /dev/null +++ b/elf/tst-filterobj-lib.c @@ -0,0 +1,24 @@ +/* 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/>. */ + +#include "tst-filterobj-lib.h" + +/* This is the real implementation that wants to be called */ +const char *get_text (void) +{ + return "Hello from filtee (PASS)"; +} diff --git a/elf/tst-filterobj-lib.h b/elf/tst-filterobj-lib.h new file mode 100644 index 0000000..bed9bf8 --- /dev/null +++ b/elf/tst-filterobj-lib.h @@ -0,0 +1,18 @@ +/* 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/>. */ + +const char *get_text (void); diff --git a/elf/tst-filterobj.c b/elf/tst-filterobj.c new file mode 100644 index 0000000..d38eb9b --- /dev/null +++ b/elf/tst-filterobj.c @@ -0,0 +1,36 @@ +/* Test that symbols from filter objects are resolved to the filtee. + + 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/>. */ + +#include <stdio.h> +#include "support/check.h" +#include "tst-filterobj-lib.h" + +static int do_test (void) +{ + const char* text = get_text (); + + printf ("%s\n", text); + + /* Verify the text matches what we expect from the filtee */ + TEST_COMPARE_STRING (text, "Hello from filtee (PASS)"); + + return 0; +} + +#include "support/test-driver.c" -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] 2019-10-24 10:34 [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy ` (2 preceding siblings ...) 2019-10-24 10:34 ` [PATCH v2 1/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy @ 2019-11-06 16:03 ` David Kilroy 2019-11-15 17:31 ` David Kilroy 3 siblings, 1 reply; 6+ messages in thread From: David Kilroy @ 2019-11-06 16:03 UTC (permalink / raw) To: libc-alpha, David Kilroy; +Cc: nd Hi, Ping. Just checking what the general feeling is about this patchset. If it helps, I've locally tested a version which scans backwards through new->l_prev and constructs map from the earliest non-relocated object (instead of the current object). This passes the test. Would this version be preferred? Thanks, Dave. Note: for the test case the walk of ->l_prev hits: elf/tst-filterobj-lib.so elf/ld-linux-x86-64.so.2 libc.so.6 dlfcn/libdl.so.2 linux-vdso.1 "" <- an object that doesn't have a ->l_name > -----Original Message----- > From: libc-alpha-owner at sourceware dot org <libc-alpha-owner at > sourceware dot org> On Behalf Of David Kilroy > Sent: 24 October 2019 11:34 > To: libc-alpha at sourceware dot org > Cc: nd <nd at arm dot com> > Subject: [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ > #16272] > > This series fixes up the patchset for the comments raised so far. > Patches 2 and 3 are unchanged. > > v2: > - code formatting fixups > - add dependency of test output on filtee library > - tests changed to use the test framework > > The main outstanding question is whether it is valid to use l_initfini > to do the relocations, or whether we need to stick to following > new->l_next (and new->l_prev). I'm reasonably confident that > l_initfini contains all the objects in new->l_next. See > https://sourceware.org/ml/libc-alpha/2019-10/msg00659.html > > The majority of the discussion on this series has been around how > filter objects are specified, and the alternatives to using filter > objects. The most persuasive is to use a version_script to specify the > ABI in the link library (.so), and setup the soname symlink (.so.1) to > point to the filtee. This relies on: > > * library versioning being used. > > * dlopen() calls must use the soname. > > * if there are multiple implementors, they need to be consistent in > versioning the library. > > I think these are reasonable requirements, though may be difficult to > ensure in the field. > > Independent of the alternatives, I'd still like to advocate fixing > dlopen for filter objects. glibc currently works (with various > provisos) with filter objects when the application is linked against > one. Not being able to work with the same library via dlopen is > unexpected. > > David Kilroy (3): > elf: Allow dlopen of filter object to work [BZ #16272] > elf: avoid redundant sort in dlopen > elf: avoid stack allocation in dl_open_worker > > elf/Makefile | 13 +++++++++++-- > elf/dl-deps.c | 35 ++++++++++++++++++++++++++--------- > elf/dl-open.c | 32 +++++++++++++++----------------- > elf/tst-filterobj-dlopen.c | 39 > +++++++++++++++++++++++++++++++++++++++ > elf/tst-filterobj-flt.c | 24 ++++++++++++++++++++++++ > elf/tst-filterobj-lib.c | 24 ++++++++++++++++++++++++ > elf/tst-filterobj-lib.h | 18 ++++++++++++++++++ > elf/tst-filterobj.c | 36 ++++++++++++++++++++++++++++++++++++ > 8 files changed, 193 insertions(+), 28 deletions(-) > create mode 100644 elf/tst-filterobj-dlopen.c > create mode 100644 elf/tst-filterobj-flt.c > create mode 100644 elf/tst-filterobj-lib.c > create mode 100644 elf/tst-filterobj-lib.h > create mode 100644 elf/tst-filterobj.c > > -- > 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] 2019-11-06 16:03 ` [PATCH v2 0/3] " David Kilroy @ 2019-11-15 17:31 ` David Kilroy 0 siblings, 0 replies; 6+ messages in thread From: David Kilroy @ 2019-11-15 17:31 UTC (permalink / raw) To: libc-alpha; +Cc: nd Hi, I looked further at the implementation that uses new->l_prev to find the filtee objects that need relocation (intending to submit that version). It turns out that this is problematic. The dlfcn/tst-rec-dlopen (recursive dlopen in interposed malloc) test highlighted that during a recursive dlopen, there are non-filtee objects in new->l_prev that aren't ready to be relocated. With my modifications I see the test do something like: dlopen("moddummy1") malloc() dlopen("moddummy2") dummy2() dlclose() malloc() dlopen("moddummy2") dummy2() dlclose() moddummy1 assigned to new (and global namespace?) malloc() dlopen("moddummy2") ->segfault attempting to relocate moddummy1 and moddummy2 If we wanted to use new->l_prev, we would need to be able to identify which objects in the list were filtee objects loaded by the current object. It may be simpler just to keep a list of the filtee objects. l_initfini effectively has this list already - so I think that's the appropriate solution. Thanks, Dave. > -----Original Message----- > From: David Kilroy <David.Kilroy@arm.com> > Sent: 06 November 2019 16:04 > To: libc-alpha@sourceware.org; David Kilroy <David.Kilroy@arm.com> > Cc: nd <nd@arm.com> > Subject: RE: [PATCH v2 0/3] elf: Allow dlopen of filter object to work > [BZ #16272] > > Hi, > > Ping. Just checking what the general feeling is about this patchset. > > If it helps, I've locally tested a version which scans backwards > through > new->l_prev and constructs map from the earliest non-relocated object > (instead > of the current object). This passes the test. Would this version be > preferred? > > > > Thanks, > > Dave. > > Note: for the test case the walk of ->l_prev hits: > elf/tst-filterobj-lib.so > elf/ld-linux-x86-64.so.2 > libc.so.6 > dlfcn/libdl.so.2 > linux-vdso.1 > "" <- an object that doesn't have a ->l_name > > > -----Original Message----- > > From: libc-alpha-owner at sourceware dot org <libc-alpha-owner at > > sourceware dot org> On Behalf Of David Kilroy > > Sent: 24 October 2019 11:34 > > To: libc-alpha at sourceware dot org > > Cc: nd <nd at arm dot com> > > Subject: [PATCH v2 0/3] elf: Allow dlopen of filter object to work > [BZ > > #16272] > > > > This series fixes up the patchset for the comments raised so far. > > Patches 2 and 3 are unchanged. > > > > v2: > > - code formatting fixups > > - add dependency of test output on filtee library > > - tests changed to use the test framework > > > > The main outstanding question is whether it is valid to use > l_initfini > > to do the relocations, or whether we need to stick to following > > new->l_next (and new->l_prev). I'm reasonably confident that > > l_initfini contains all the objects in new->l_next. See > > https://sourceware.org/ml/libc-alpha/2019-10/msg00659.html > > > > The majority of the discussion on this series has been around how > > filter objects are specified, and the alternatives to using filter > > objects. The most persuasive is to use a version_script to specify > the > > ABI in the link library (.so), and setup the soname symlink (.so.1) > to > > point to the filtee. This relies on: > > > > * library versioning being used. > > > > * dlopen() calls must use the soname. > > > > * if there are multiple implementors, they need to be consistent in > > versioning the library. > > > > I think these are reasonable requirements, though may be difficult to > > ensure in the field. > > > > Independent of the alternatives, I'd still like to advocate fixing > > dlopen for filter objects. glibc currently works (with various > > provisos) with filter objects when the application is linked against > > one. Not being able to work with the same library via dlopen is > > unexpected. > > > > David Kilroy (3): > > elf: Allow dlopen of filter object to work [BZ #16272] > > elf: avoid redundant sort in dlopen > > elf: avoid stack allocation in dl_open_worker > > > > elf/Makefile | 13 +++++++++++-- > > elf/dl-deps.c | 35 ++++++++++++++++++++++++++--------- > > elf/dl-open.c | 32 +++++++++++++++----------------- > > elf/tst-filterobj-dlopen.c | 39 > > +++++++++++++++++++++++++++++++++++++++ > > elf/tst-filterobj-flt.c | 24 ++++++++++++++++++++++++ > > elf/tst-filterobj-lib.c | 24 ++++++++++++++++++++++++ > > elf/tst-filterobj-lib.h | 18 ++++++++++++++++++ > > elf/tst-filterobj.c | 36 ++++++++++++++++++++++++++++++++++++ > > 8 files changed, 193 insertions(+), 28 deletions(-) > > create mode 100644 elf/tst-filterobj-dlopen.c > > create mode 100644 elf/tst-filterobj-flt.c > > create mode 100644 elf/tst-filterobj-lib.c > > create mode 100644 elf/tst-filterobj-lib.h > > create mode 100644 elf/tst-filterobj.c > > > > -- > > 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-15 17:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-24 10:34 [PATCH v2 0/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy 2019-10-24 10:34 ` [PATCH v2 2/3] elf: avoid redundant sort in dlopen David Kilroy 2019-10-24 10:34 ` [PATCH v2 3/3] elf: avoid stack allocation in dl_open_worker David Kilroy 2019-10-24 10:34 ` [PATCH v2 1/3] elf: Allow dlopen of filter object to work [BZ #16272] David Kilroy 2019-11-06 16:03 ` [PATCH v2 0/3] " David Kilroy 2019-11-15 17:31 ` David Kilroy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).