This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- From: Pavel Kopyl <p dot kopyl at samsung dot com>
- To: GNU C Library <libc-alpha at sourceware dot org>
- Cc: "H.J. Lu" <hjl dot tools at gmail dot com>, Carlos O'Donell <carlos at redhat dot com>, Yury Gribov <y dot gribov at samsung dot com>, Roland McGrath <roland at hack dot frob dot com>, Viacheslav Garbuzov <v dot garbuzov at samsung dot com>
- Date: Wed, 27 May 2015 15:17:41 +0300
- Subject: [PATCHv4][PING][BZ #17833] _dl_close_worker() does not release inconsistent objects.
- Authentication-results: sourceware.org; auth=none
- References: <54BD4F65 dot 2090108 at samsung dot com> <54BEF851 dot 70902 at redhat dot com> <54DBC3CB dot 5080703 at samsung dot com> <54F071DB dot 9040106 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>
On 05/13/2015 02:40 PM, Pavel Kopyl wrote:
On 04/24/2015 02:47 PM, H.J. Lu wrote:
On Fri, Apr 24, 2015 at 3:33 AM, Pavel Kopyl <p.kopyl@samsung.com>
wrote:
On 03/03/2015 12:13 PM, Pavel Kopyl wrote:
On 03/01/2015 10:17 PM, Mike Frysinger wrote:
On 27 Feb 2015 16:32, Pavel Kopyl wrote:
--- /dev/null
+++ b/elf/tst-unique5lib.cc
@@ -0,0 +1,13 @@
+
i know existing tests are bad examples, but lets try and start fixing
that.
namely, there should be a header here giving a quick overview of
what it
is
exactly you're testing for, and a BZ reference.
+extern int not_exist ();
+
+inline int make_unique ()
+{
+ static int unique;
+ return ++unique;
+}
+
+int foo ()
+{
+ return make_unique () + not_exist ();
+}
i don't know if this is just copy & pasting, but prototypes that
do not
intend
to take args should always be (void).
-mike
Thanks for review, I fixed that in patch v3.
Ping.
Some comments:
1. The bug report is against STB_GNU_UNIQUE. But I don't see
STB_GNU_UNIQUE in
testcase. I can't tell if the original STB_GNU_UNIQUE bug is fixed.
2. Your testcase ignores dlopen error. Why should it work at all?
3. Your testcase doesn't use test-skeleton.c.
Thanks for review!
1. I added some comments in testcase to be more clear.
2. Actually the call below is expected to return NULL because of
undefined symbol in the library.
dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_NOW);
It's a goal of this testcase: I need some library with STB_GNU_UNIQUE
symbols that returns error when trying to load it.
In new patch version I especially point out to this fact by checking
return value.
3. It seems for me that template of the test-skeleton.c is not
appropriate because this testcase has two related binary - library and
executable.
-Pavel
Ping.
commit 9a5dc6c0988d9cd4329328de810097ff2b9368d3
Author: Pavel Kopyl <p.kopyl@samsung.com>
Date: Wed May 13 14:31:15 2015 +0300
2015-05-13 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-unique5.cc: Test executable. New file.
* elf/tst-unique5lib.cc: Test library. New file.
* include/dlfcn.h (_dl_close_worker): Add new parameter.
diff --git a/elf/Makefile b/elf/Makefile
index 34450ea..d0dd0a2 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -144,7 +144,7 @@ tests += loadtest restest1 preloadtest loadfail multiload origtest resolvfail \
unload3 unload4 unload5 unload6 unload7 unload8 tst-global1 order2 \
tst-audit1 tst-audit2 tst-audit8 tst-audit9 \
tst-stackguard1 tst-addr1 tst-thrlock \
- tst-unique1 tst-unique2 $(if $(CXX),tst-unique3 tst-unique4) \
+ tst-unique1 tst-unique2 $(if $(CXX),tst-unique3 tst-unique4 tst-unique5) \
tst-initorder tst-initorder2 tst-relsort1 tst-null-argv \
tst-ptrguard1 tst-tlsalign
# reldep9
@@ -205,7 +205,7 @@ 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-unique5lib) \
tst-initordera1 tst-initorderb1 \
tst-initordera2 tst-initorderb2 \
tst-initordera3 tst-initordera4 \
@@ -592,6 +592,7 @@ 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-unique5lib.so-no-z-defs = yes
ifeq ($(build-shared),yes)
# Build all the modules even when not actually running test programs.
@@ -1150,6 +1151,9 @@ $(objpfx)tst-unique3.out: $(objpfx)tst-unique3lib2.so
$(objpfx)tst-unique4: $(objpfx)tst-unique4lib.so
+$(objpfx)tst-unique5: $(libdl)
+$(objpfx)tst-unique5.out: $(objpfx)tst-unique5lib.so
+
$(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-unique5.cc b/elf/tst-unique5.cc
new file mode 100644
index 0000000..a18fb42
--- /dev/null
+++ b/elf/tst-unique5.cc
@@ -0,0 +1,21 @@
+#include "../dlfcn/dlfcn.h"
+#include <stdlib.h>
+
+/* unique5lib.so library contains both STB_GNU_UNIQUE and undefined symbols.
+ First dlopen call fails because of undefined symbols. Test checks that
+ library does not remain locked in the memory in this case. [BZ #17833] */
+
+int
+main (void)
+{
+ /* This call should return error because of undefined symbol in library. */
+ if (dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_NOW) != NULL)
+ return 1;
+
+ /* Check that library is not present in the memory after unsuccessful
+ attempt to load it. */
+ if (dlopen ("$ORIGIN/tst-unique5lib.so", RTLD_LAZY | RTLD_NOLOAD) != NULL)
+ return 1;
+
+ return 0;
+}
diff --git a/elf/tst-unique5lib.cc b/elf/tst-unique5lib.cc
new file mode 100644
index 0000000..b45ead8
--- /dev/null
+++ b/elf/tst-unique5lib.cc
@@ -0,0 +1,16 @@
+/* tst-unique5lib.so library for tst-unique5.cc. */
+
+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/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