This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH][BZ #18778] Clear DF_1_NODELETE flag only for dlopen failed library.
- From: Maxim Ostapenko <m dot ostapenko at partner dot samsung dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: Andreas Schwab <schwab at suse dot de>, Pavel Kopyl <p dot kopyl at samsung dot com>, 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: Mon, 10 Aug 2015 14:56:33 +0300
- Subject: [PATCH][BZ #18778] Clear DF_1_NODELETE flag only for dlopen failed library.
- Authentication-results: sourceware.org; auth=none
- References: <54BD4F65 dot 2090108 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> <559BFDDC dot 4010604 at samsung dot com> <mvmpp30o2nw dot fsf at hawking dot suse dot de> <55C4D58D dot 8010307 at partner dot samsung dot com> <CAMe9rOq8_3A0ogN2riC32r+QazmJ5kW+5jg22x9_1z1uZaShVQ at mail dot gmail dot com>
On 07/08/15 19:14, H.J. Lu wrote:
On Fri, Aug 7, 2015 at 8:58 AM, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
Hi!
On 06/08/15 18:30, Andreas Schwab wrote:
Pavel Kopyl <p.kopyl@samsung.com> writes:
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;
This will remove the NODELETE flag from *all* loaded objects. That
doesn't make sense.
Andreas.
Indeed, we shouldn't remove NODELETE from all loaded objects, only for buggy
library. Here a draft patch that should fix the issue. Andreas, does this
look reasonable for you? If yes, I'll reformat it (e.g. add proper ChangeLog
entry etc) and send for review as BZ#18778 fix.
Please include a testcase to verify that the bug is fixed.
This patch fixes BZ #18778 issue by moving l->l_flags_1 &=
~DF_1_NODELETE out of loop through all loaded libraries and performs
this action only on inconsistent one.
No regressions on x86_64-unknown-linux-gnu, testcase is attached, OK for
master?
-Maxim
>From 828eae1cbaa389b9931dbe0d2111169eede6caf3 Mon Sep 17 00:00:00 2001
From: Maxim Ostapenko <m.ostapenko@partner.samsung.com>
Date: Mon, 10 Aug 2015 10:47:54 +0300
Subject: [PATCH] Clear DF_1_NODELETE flag only for failed to load library.
https://sourceware.org/bugzilla/show_bug.cgi?id=18778
If dlopen fails to load an object that has triggered loading libpthread it
causes ld.so to unload libpthread because its DF_1_NODELETE flags has been
forcefully cleared. The next call to __rtdl_unlock_lock_recursive will crash
since pthread_mutex_unlock no longer exists.
This patch moves l->l_flags_1 &= ~DF_1_NODELETE out of loop through all loaded
libraries and performs the action only on inconsistent one.
[BZ #18778]
* elf/Makefile (tests): Add Add tst-nodelete2.
(modules-names): Add tst-nodelete2mod.
(tst-nodelete2mod.so-no-z-defs): New.
($(objpfx)tst-nodelete2): Likewise.
($(objpfx)tst-nodelete2.out): Likewise.
(LDFLAGS-tst-nodelete2): Likewise.
* elf/dl-close.c (_dl_close_worker): Move DF_1_NODELETE clearing
out of loop through all loaded libraries.
* elf/tst-nodelete2.c: New file.
* elf/tst-nodelete2mod.c: Likewise.
---
ChangeLog | 13 +++++++++++++
NEWS | 2 +-
elf/Makefile | 11 +++++++++--
elf/dl-close.c | 15 ++++++++-------
elf/tst-nodelete2.c | 37 +++++++++++++++++++++++++++++++++++++
elf/tst-nodelete2mod.c | 7 +++++++
elf/tst-znodelete-zlib.cc | 6 ------
7 files changed, 75 insertions(+), 16 deletions(-)
create mode 100644 elf/tst-nodelete2.c
create mode 100644 elf/tst-nodelete2mod.c
delete mode 100644 elf/tst-znodelete-zlib.cc
diff --git a/ChangeLog b/ChangeLog
index d4c9d79..4e93c73 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2015-08-10 Maxim Ostapenko <m.ostapenko@partner.samsung.com>
+ [BZ #18778]
+ * elf/Makefile (tests): Add Add tst-nodelete2.
+ (modules-names): Add tst-nodelete2mod.
+ (tst-nodelete2mod.so-no-z-defs): New.
+ ($(objpfx)tst-nodelete2): Likewise.
+ ($(objpfx)tst-nodelete2.out): Likewise.
+ (LDFLAGS-tst-nodelete2): Likewise.
+ * elf/dl-close.c (_dl_close_worker): Move DF_1_NODELETE clearing
+ out of loop through all loaded libraries.
+ * elf/tst-nodelete2.c: New file.
+ * elf/tst-nodelete2mod.c: Likewise.
+
2015-08-09 H.J. Lu <hongjiu.lu@intel.com>
[BZ #18674]
diff --git a/NEWS b/NEWS
index 7bd785a..ae761ed 100644
--- a/NEWS
+++ b/NEWS
@@ -10,7 +10,7 @@ Version 2.23
* The following bugs are resolved with this release:
16517, 16519, 17905, 18265, 18480, 18525, 18618, 18647, 18661, 18674,
- 18787.
+ 18787, 18778.
Version 2.22
diff --git a/elf/Makefile b/elf/Makefile
index 4ceeaf8..71a18a1 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -148,7 +148,8 @@ tests += loadtest 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-ptrguard1 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened
+ tst-ptrguard1 tst-tlsalign tst-tlsalign-extern tst-nodelete-opened \
+ tst-nodelete2
# reldep9
ifeq ($(build-hardcoded-path-in-tests),yes)
tests += tst-dlopen-aout
@@ -218,7 +219,7 @@ modules-names = testobj1 testobj2 testobj3 testobj4 testobj5 testobj6 \
tst-initorder2d \
tst-relsort1mod1 tst-relsort1mod2 tst-array2dep \
tst-array5dep tst-null-argv-lib \
- tst-tlsalign-lib tst-nodelete-opened-lib
+ tst-tlsalign-lib tst-nodelete-opened-lib tst-nodelete2mod
ifeq (yes,$(have-protected-data))
modules-names += tst-protected1moda tst-protected1modb
tests += tst-protected1a tst-protected1b
@@ -594,6 +595,7 @@ 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
+tst-nodelete2mod.so-no-z-defs = yes
ifeq ($(build-shared),yes)
# Build all the modules even when not actually running test programs.
@@ -1164,6 +1166,11 @@ $(objpfx)tst-nodelete.out: $(objpfx)tst-nodelete-uniquemod.so \
LDFLAGS-tst-nodelete = -rdynamic
LDFLAGS-tst-nodelete-zmod.so = -Wl,--enable-new-dtags,-z,nodelete
+$(objpfx)tst-nodelete2: $(libdl)
+$(objpfx)tst-nodelete2.out: $(objpfx)tst-nodelete2mod.so
+
+LDFLAGS-tst-nodelete2 = -rdynamic
+
$(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 9105277..c897247 100644
--- a/elf/dl-close.c
+++ b/elf/dl-close.c
@@ -144,6 +144,14 @@ _dl_close_worker (struct link_map *map, bool force)
char done[nloaded];
struct link_map *maps[nloaded];
+ /* Clear DF_1_NODELETE to force object deletion. We don't need to touch
+ l_tls_dtor_count because forced object deletion only happens when an
+ error occurs during object load. Destructor registration for TLS
+ non-POD objects should not have happened till then for this
+ object. */
+ if (force)
+ map->l_flags_1 &= ~DF_1_NODELETE;
+
/* Run over the list and assign indexes to the link maps and enter
them into the MAPS array. */
int idx = 0;
@@ -153,13 +161,6 @@ _dl_close_worker (struct link_map *map, bool force)
maps[idx] = l;
++idx;
- /* Clear DF_1_NODELETE to force object deletion. We don't need to touch
- l_tls_dtor_count because forced object deletion only happens when an
- error occurs during object load. Destructor registration for TLS
- non-POD objects should not have happened till then for this
- object. */
- if (force)
- l->l_flags_1 &= ~DF_1_NODELETE;
}
assert (idx == nloaded);
diff --git a/elf/tst-nodelete2.c b/elf/tst-nodelete2.c
new file mode 100644
index 0000000..388e8af
--- /dev/null
+++ b/elf/tst-nodelete2.c
@@ -0,0 +1,37 @@
+#include "../dlfcn/dlfcn.h"
+#include <stdio.h>
+#include <stdlib.h>
+#include <gnu/lib-names.h>
+
+static int
+do_test (void)
+{
+ int result = 0;
+
+ printf ("\nOpening pthread library.\n");
+ void *pthread = dlopen (LIBPTHREAD_SO, RTLD_LAZY);
+
+ /* This is a test for correct DF_1_NODELETE clearing when dlopen failure
+ happens. We should clear DF_1_NODELETE for failed library only, because
+ doing this for others (e.g. libpthread) might cause them to be unloaded,
+ that may lead to some global references (e.g. __rtld_lock_unlock) to be
+ broken. The dlopen should fail because of undefined symbols in shared
+ library, that cause DF_1_NODELETE to be cleared. For libpthread, this
+ flag should be set, because if not, SIGSEGV will happen in dlclose. */
+ if (dlopen ("tst-nodelete2mod.so", RTLD_NOW) != NULL)
+ {
+ printf ("Unique symbols test failed\n");
+ result = 1;
+ }
+
+ if (pthread)
+ dlclose (pthread);
+
+ if (result == 0)
+ printf ("SUCCESS\n");
+
+ return result;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
diff --git a/elf/tst-nodelete2mod.c b/elf/tst-nodelete2mod.c
new file mode 100644
index 0000000..e88c756
--- /dev/null
+++ b/elf/tst-nodelete2mod.c
@@ -0,0 +1,7 @@
+/* Undefined symbol. */
+extern int not_exist (void);
+
+int foo (void)
+{
+ return not_exist ();
+}
diff --git a/elf/tst-znodelete-zlib.cc b/elf/tst-znodelete-zlib.cc
deleted file mode 100644
index 1e8f368..0000000
--- a/elf/tst-znodelete-zlib.cc
+++ /dev/null
@@ -1,6 +0,0 @@
-extern int not_exist (void);
-
-int foo (void)
-{
- return not_exist ();
-}
--
1.9.1