This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCHv5][PING^3][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- From: Pavel Kopyl <p dot kopyl at samsung dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Yury Gribov <y dot gribov at samsung dot com>, Roland McGrath <roland at hack dot frob dot com>, GNU C Library <libc-alpha at sourceware dot org>, Carlos O'Donell <carlos at redhat dot com>, Viacheslav Garbuzov <v dot garbuzov at samsung dot com>
- Date: Tue, 07 Jul 2015 19:27:08 +0300
- Subject: Re: [PATCHv5][PING^3][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- Authentication-results: sourceware.org; auth=none
- References: <54BD4F65 dot 2090108 at samsung dot com> <20150301191710 dot GB19363 at vapier> <54F57B52 dot 6080202 at samsung dot com> <553A1BEE dot 6070705 at samsung dot com> <CAMe9rOqiyuyNAtJDZGbfs+0kk0j16-Vowp5f0z_x2zfsd76fMQ at mail dot gmail dot com> <5553382F dot 3020906 at samsung dot com> <5565B5E5 dot 7060101 at samsung dot com> <CAMe9rOr8yDpnHtRFbL3M56Sx5FWX-FVqEstnwsgtW6H+khvziQ at mail dot gmail dot com> <5565C2A8 dot 60306 at samsung dot com> <CAMe9rOq++pD-ugdYFEte49v8TLZEM505J+=WzPTOT_Lo-MdDHQ at mail dot gmail dot com> <5565C862 dot 1040003 at samsung dot com> <CAMe9rOo7TStj3SX8OK8s3H3G=2Pyr1WKTW=R-=SzVFBWY8PF0A at mail dot gmail dot com> <5566395A dot 3090605 at samsung dot com> <CAMe9rOp4Jrz4AE3-C5VmJ0PLmxoST3phyEQt3t59ag6UGbimBw at mail dot gmail dot com> <5567892C dot 4070004 at samsung dot com> <5568A408 dot 2080903 at samsung dot com> <5592AB91 dot 2050709 at samsung dot com> <CAMe9rOoK64VuNfgZ-8_BTqes0tJcpc55atKw1k6ewBTwFzuGKg at mail dot gmail dot com> <5595C0F8 dot 3060300 at samsung dot com> <CAMe9rOof9j6RwGgNGuxXUgBnYXK0c_UAp3zN2Ne4JhHqiZMFEQ at mail dot gmail dot com> <559B829C dot 8080700 at samsung dot com> <CAMe9rOom4ttO2cGYGovagHpu3zy4L7qn+7E4jm7k=5rD+xgNaQ at mail dot gmail dot com>
On 07/07/2015 06:30 PM, H.J. Lu wrote:
On Tue, Jul 7, 2015 at 12:41 AM, Yury Gribov <y.gribov@samsung.com> wrote:
On 07/03/2015 02:46 AM, H.J. Lu wrote:
On Thu, Jul 2, 2015 at 3:53 PM, Pavel Kopyl <p.kopyl@samsung.com> wrote:
On 06/30/2015 06:12 PM, H.J. Lu wrote:
On Tue, Jun 30, 2015 at 7:45 AM, Pavel Kopyl <p.kopyl@samsung.com>
wrote:
This patch changes DF_1_NODELETE path. Do we have a testcase for
sucessfully loading/unloading DF_1_NODELETE DSO with undefined
symbols?
It may have been asked before. Can we reset unique symbols
in _dl_open before calling _dl_close_worker?
Yes, I added testcases for three possible ways where we can get
DF_1_NODELETE:
1. Unique symbols
2. Load with RTLD_NODELETE flag.
3. Link with '-z nodelete' option
Can we reset unique symbols in _dl_open before calling _dl_close_worker?
But I clear unique symbols exactly in
_dl_close_worker.<https://slovari.yandex.ru/exactly/en-ru>
Looks good to me.
Folks,
Could someone commit the patch for us? We do not have write access to Glibc
repo. We'll be around to cope with bugs (if any).
Please regenerate the patch with "gcc format-patch" and resubmit it
so that I can do "git am your-patch".
Thanks.
Thanks a lot.
-Pavel
>From b6f602f16bf1e32ccc990e178f2559babad45733 Mon Sep 17 00:00:00 2001
From: Pavel Kopyl <p.kopyl@samsung.com>
Date: Tue, 7 Jul 2015 18:45:46 +0300
Subject: [PATCH] 2015-07-07 Pavel Kopyl <p.kopyl@samsung.com> Mikhail
Ilin <m.ilin@samsung.com>
[BZ #17833]
* elf/Makefile: Add build test commands.
* elf/dl-close.c (_dl_close_worker): Implement force object deletion.
* elf/dl-open.c (_dl_open): Forced _dl_close_worker call.
(_dl_close): Default _dl_close_worker call.
* elf/tst-nodelete.cc: Test executable. New file.
* elf/tst-nodeletelib.cc: Test library. New file.
* elf/tst-znodeletelib.cc: Likewise.
* include/dlfcn.h (_dl_close_worker): Add new parameter.
---
elf/Makefile | 16 +++++++++++--
elf/dl-close.c | 33 ++++++++++++++++++++++++--
elf/dl-open.c | 2 +-
elf/tst-nodelete-rtldmod.cc | 6 +++++
elf/tst-nodelete-uniquemod.cc | 14 +++++++++++
elf/tst-nodelete-zmod.cc | 6 +++++
elf/tst-nodelete.cc | 51 +++++++++++++++++++++++++++++++++++++++++
elf/tst-znodelete-zlib.cc | 6 +++++
include/dlfcn.h | 3 ++-
9 files changed, 131 insertions(+), 6 deletions(-)
create mode 100644 elf/tst-nodelete-rtldmod.cc
create mode 100644 elf/tst-nodelete-uniquemod.cc
create mode 100644 elf/tst-nodelete-zmod.cc
create mode 100644 elf/tst-nodelete.cc
create mode 100644 elf/tst-znodelete-zlib.cc
diff --git a/elf/Makefile b/elf/Makefile
index f21276c0..d36a3c5 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -132,7 +132,7 @@ tests += $(tests-static)
ifeq (yes,$(build-shared))
tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
constload1 order noload filter unload \
- reldep reldep2 reldep3 reldep4 nodelete nodelete2 \
+ reldep reldep2 reldep3 reldep4 nodelete nodelete2 tst-nodelete \
nodlopen nodlopen2 neededtest neededtest2 \
neededtest3 neededtest4 unload2 lateglobal initfirst global \
restest2 next dblload dblunload reldep5 reldep6 reldep7 reldep8 \
@@ -207,7 +207,9 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
tst-unique1mod1 tst-unique1mod2 \
tst-unique2mod1 tst-unique2mod2 \
tst-auditmod9a tst-auditmod9b \
- $(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib) \
+ $(if $(CXX),tst-unique3lib tst-unique3lib2 tst-unique4lib \
+ tst-nodelete-uniquemod) \
+ tst-nodelete-rtldmod tst-nodelete-zmod \
tst-initordera1 tst-initorderb1 \
tst-initordera2 tst-initorderb2 \
tst-initordera3 tst-initordera4 \
@@ -591,6 +593,9 @@ ifuncmod5.so-no-z-defs = yes
ifuncmod6.so-no-z-defs = yes
tst-auditmod9a.so-no-z-defs = yes
tst-auditmod9b.so-no-z-defs = yes
+tst-nodelete-uniquemod.so-no-z-defs = yes
+tst-nodelete-rtldmod.so-no-z-defs = yes
+tst-nodelete-zmod.so-no-z-defs = yes
ifeq ($(build-shared),yes)
# Build all the modules even when not actually running test programs.
@@ -1153,6 +1158,13 @@ $(objpfx)tst-unique3.out: $(objpfx)tst-unique3lib2.so
$(objpfx)tst-unique4: $(objpfx)tst-unique4lib.so
+$(objpfx)tst-nodelete: $(libdl)
+$(objpfx)tst-nodelete.out: $(objpfx)tst-nodelete-uniquemod.so \
+ $(objpfx)tst-nodelete-rtldmod.so \
+ $(objpfx)tst-nodelete-zmod.so
+
+LDFLAGS-tst-nodelete = -rdynamic
+LDFLAGS-tst-nodelete-zmod.so = -Wl,--enable-new-dtags,-z,nodelete
$(objpfx)tst-initorder-cmp.out: tst-initorder.exp $(objpfx)tst-initorder.out
cmp $^ > $@; \
$(evaluate-test)
diff --git a/elf/dl-close.c b/elf/dl-close.c
index 412f71d..0595675 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -108,7 +108,7 @@ remove_slotinfo (size_t idx, struct dtv_slotinfo_list *listp, size_t disp,
void
-_dl_close_worker (struct link_map *map)
+_dl_close_worker (struct link_map *map, bool force)
{
/* One less direct use. */
--map->l_direct_opencount;
@@ -152,6 +152,10 @@ _dl_close_worker (struct link_map *map)
l->l_idx = idx;
maps[idx] = l;
++idx;
+
+ /* clear DF_1_NODELETE to force object deletion. */
+ if (force)
+ l->l_flags_1 &= ~DF_1_NODELETE;
}
assert (idx == nloaded);
@@ -635,6 +639,31 @@ _dl_close_worker (struct link_map *map)
}
}
+ /* Reset unique symbols if forced. */
+ if (force)
+ {
+ struct unique_sym_table *tab = &ns->_ns_unique_sym_table;
+ __rtld_lock_lock_recursive (tab->lock);
+ struct unique_sym *entries = tab->entries;
+ if (entries != NULL)
+ {
+ size_t idx, size = tab->size;
+ for (idx = 0; idx < size; ++idx)
+ {
+ /* Clear unique symbol entries
+ that belong to this object. */
+ if (entries[idx].name != NULL
+ && entries[idx].map == imap)
+ {
+ entries[idx].name = NULL;
+ entries[idx].hashval = 0;
+ tab->n_elements--;
+ }
+ }
+ }
+ __rtld_lock_unlock_recursive (tab->lock);
+ }
+
/* We can unmap all the maps at once. We determined the
start address and length when we loaded the object and
the `munmap' call does the rest. */
@@ -782,7 +811,7 @@ _dl_close (void *_map)
/* Acquire the lock. */
__rtld_lock_lock_recursive (GL(dl_load_lock));
- _dl_close_worker (map);
+ _dl_close_worker (map, false);
__rtld_lock_unlock_recursive (GL(dl_load_lock));
}
diff --git a/elf/dl-open.c b/elf/dl-open.c
index 2d0e082..027c1e0 100644
--- a/elf/dl-open.c
+++ b/elf/dl-open.c
@@ -670,7 +670,7 @@ no more namespaces available for dlmopen()"));
if ((mode & __RTLD_AUDIT) == 0)
GL(dl_tls_dtv_gaps) = true;
- _dl_close_worker (args.map);
+ _dl_close_worker (args.map, true);
}
assert (_dl_debug_initialize (0, args.nsid)->r_state == RT_CONSISTENT);
diff --git a/elf/tst-nodelete-rtldmod.cc b/elf/tst-nodelete-rtldmod.cc
new file mode 100644
index 0000000..740e1d8
--- /dev/null
+++ b/elf/tst-nodelete-rtldmod.cc
@@ -0,0 +1,6 @@
+extern int not_exist (void);
+
+int foo (void)
+{
+ return not_exist ();
+}
diff --git a/elf/tst-nodelete-uniquemod.cc b/elf/tst-nodelete-uniquemod.cc
new file mode 100644
index 0000000..632b303
--- /dev/null
+++ b/elf/tst-nodelete-uniquemod.cc
@@ -0,0 +1,14 @@
+extern int not_exist (void);
+
+inline int make_unique (void)
+{
+ /* Static variables in inline functions and classes
+ generate STB_GNU_UNIQUE symbols. */
+ static int unique;
+ return ++unique;
+}
+
+int foo (void)
+{
+ return make_unique () + not_exist ();
+}
diff --git a/elf/tst-nodelete-zmod.cc b/elf/tst-nodelete-zmod.cc
new file mode 100644
index 0000000..740e1d8
--- /dev/null
+++ b/elf/tst-nodelete-zmod.cc
@@ -0,0 +1,6 @@
+extern int not_exist (void);
+
+int foo (void)
+{
+ return not_exist ();
+}
diff --git a/elf/tst-nodelete.cc b/elf/tst-nodelete.cc
new file mode 100644
index 0000000..176cb68
--- /dev/null
+++ b/elf/tst-nodelete.cc
@@ -0,0 +1,51 @@
+#include "../dlfcn/dlfcn.h"
+#include <stdio.h>
+#include <stdlib.h>
+
+static int
+do_test (void)
+{
+ int result = 0;
+
+ /* This is a test for correct handling of dlopen failures for library that
+ is loaded with RTLD_NODELETE flag. The first dlopen should fail because
+ of undefined symbols in shared library. The second dlopen then verifies
+ that library was properly unloaded. */
+ if (dlopen ("tst-nodelete-rtldmod.so", RTLD_NOW | RTLD_NODELETE) != NULL
+ || dlopen ("tst-nodelete-rtldmod.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
+ {
+ printf ("RTLD_NODELETE test failed\n");
+ result = 1;
+ }
+
+ /* This is a test for correct handling of dlopen failures for library that
+ is linked with '-z nodelete' option and hence has DF_1_NODELETE flag.
+ The first dlopen should fail because of undefined symbols in shared
+ library. The second dlopen then verifies that library was properly
+ unloaded. */
+ if (dlopen ("tst-nodelete-zmod.so", RTLD_NOW) != NULL
+ || dlopen ("tst-nodelete-zmod.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
+ {
+ printf ("-z nodelete test failed\n");
+ result = 1;
+ }
+
+ /* This is a test for correct handling of dlopen failures for library
+ with unique symbols. The first dlopen should fail because of undefined
+ symbols in shared library. The second dlopen then verifies that library
+ was properly unloaded. */
+ if (dlopen ("tst-nodelete-uniquemod.so", RTLD_NOW) != NULL
+ || dlopen ("tst-nodelete-uniquemod.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
+ {
+ printf ("Unique symbols test failed\n");
+ result = 1;
+ }
+
+ if (result == 0)
+ printf ("SUCCESS\n");
+
+ return result;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/elf/tst-znodelete-zlib.cc b/elf/tst-znodelete-zlib.cc
new file mode 100644
index 0000000..1e8f368
--- /dev/null
+++ b/elf/tst-znodelete-zlib.cc
@@ -0,0 +1,6 @@
+extern int not_exist (void);
+
+int foo (void)
+{
+ return not_exist ();
+}
diff --git a/include/dlfcn.h b/include/dlfcn.h
index a67b2e3..0ce0af5 100644
--- a/include/dlfcn.h
+++ b/include/dlfcn.h
@@ -54,7 +54,8 @@ struct link_map;
extern void _dl_close (void *map) attribute_hidden;
/* Same as above, but without locking and safety checks for user
provided map arguments. */
-extern void _dl_close_worker (struct link_map *map) attribute_hidden;
+extern void _dl_close_worker (struct link_map *map, bool force)
+ attribute_hidden;
/* Look up NAME in shared object HANDLE (which may be RTLD_DEFAULT or
RTLD_NEXT). WHO is the calling function, for RTLD_NEXT. Returns
--
1.7.9.5