]> xenbits.xensource.com Git - libvirt.git/commitdiff
Prevent crash from dlclose() of libvirt.so
authorDaniel P. Berrange <berrange@redhat.com>
Thu, 1 Sep 2011 16:57:06 +0000 (17:57 +0100)
committerEric Blake <eblake@redhat.com>
Fri, 16 Sep 2011 21:51:31 +0000 (15:51 -0600)
When libvirt calls virInitialize it creates a thread local
for the virErrorPtr storage, and registers a callback to
cleanup memory when a thread exits. When libvirt is dlclose()d
or otherwise made non-resident, the callback function is
removed from memory, but the thread local may still exist
and if a thread later exists, it will invoke the callback
and SEGV. There may also be other thread locals with callbacks
pointing to libvirt code, so it is in general never safe to
unload libvirt.so from memory once initialized.

To allow dlclose() to succeed, but keep libvirt.so resident
in memory, link with '-z nodelete'. This issue was first
found with the libvirt CIM provider, but can potentially
hit many of the dynamic language bindings which all ultimately
involve dlopen() in some way, either on libvirt.so itself,
or on the glue code for the binding which in turns links
to libvirt

* configure.ac, src/Makefile.am: Ensure libvirt.so is linked
  with -z nodelete
* cfg.mk, .gitignore, tests/Makefile.am, tests/shunloadhelper.c,
  tests/shunloadtest.c: A test case to unload libvirt while
  a thread is still running.

.gitignore
cfg.mk
configure.ac
src/Makefile.am
tests/Makefile.am
tests/shunloadhelper.c [new file with mode: 0644]
tests/shunloadtest.c [new file with mode: 0644]

index 06c3d0b6d10d7ffde2cacdad99d00c0f4db6826f..41fa50f5d280fabff0f57ed9d9a4d1eb79f53a3e 100644 (file)
@@ -5,6 +5,8 @@
 *.gcda
 *.gcno
 *.gcov
+*.la
+*.lo
 *.o
 *.orig
 *.rej
@@ -72,6 +74,7 @@
 /tests/networkxml2argvtest
 /tests/nwfilterxml2xmltest
 /tests/openvzutilstest
+/tests/shunloadtest
 /update.log
 Makefile
 Makefile.in
diff --git a/cfg.mk b/cfg.mk
index 0a2a5b1d313c7d48fb63cbc1d8b1a57d8b03709f..95c5eff63c217d15a6ac2686bf07ecec588e378d 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -672,7 +672,7 @@ exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.c$$
 
 _src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket
 exclude_file_name_regexp--sc_avoid_write = \
-  ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/virnettlscontexttest)\.c$$
+  ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/(shunload|virnettlscontext)test)\.c$$
 
 exclude_file_name_regexp--sc_bindtextdomain = ^(tests|examples)/
 
index 5e3539f7ac8082eb95fffbb85de99ddab641902c..0c6d5248443c5db00798a9303938d68e32b2cf3b 100644 (file)
@@ -97,9 +97,18 @@ AM_PROG_LIBTOOL
 AM_PROG_CC_C_O
 AM_PROG_LD
 
+AC_MSG_CHECKING([for how to mark DSO non-deletable at runtime])
+LIBVIRT_NODELETE=
+`$LD --help 2>&1 | grep -- "-z nodelete" >/dev/null` && \
+    LIBVIRT_NODELETE="-Wl,-z -Wl,nodelete"
+AC_MSG_RESULT([$LIBVIRT_NODELETE])
+AC_SUBST([LIBVIRT_NODELETE])
+
+AC_MSG_CHECKING([for how to set DSO symbol versions])
 VERSION_SCRIPT_FLAGS=-Wl,--version-script=
 `$LD --help 2>&1 | grep -- --version-script >/dev/null` || \
     VERSION_SCRIPT_FLAGS="-Wl,-M -Wl,"
+AC_MSG_RESULT([$VERSION_SCRIPT_FLAGS])
 
 LIBVIRT_COMPILE_WARNINGS([maximum])
 
index 14f09e4c8c4471497527a3d5ca490d7becfbd977..9a903ea6b503f3fb3ca77de50c88f45420685409 100644 (file)
@@ -1232,7 +1232,7 @@ libvirt_qemu.def: $(srcdir)/libvirt_qemu.syms
 libvirt_la_SOURCES =
 libvirt_la_LDFLAGS = $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \
                     -version-info $(LIBVIRT_VERSION_INFO) \
-                   $(AM_LDFLAGS) \
+                   $(LIBVIRT_NODELETE) $(AM_LDFLAGS) \
                    $(CYGWIN_EXTRA_LDFLAGS) $(MINGW_EXTRA_LDFLAGS)
 libvirt_la_BUILT_LIBADD += ../gnulib/lib/libgnu.la
 libvirt_la_LIBADD += $(LIBXML_LIBS) \
index f4afcb9150dd683b0662a3f60d458081223fc49b..cbbbc6f6ca4354676e4d49e992564538b622a895 100644 (file)
@@ -89,7 +89,9 @@ check_PROGRAMS = virshtest conftest sockettest \
        nodeinfotest qparamtest virbuftest \
        commandtest commandhelper seclabeltest \
        hashtest virnetmessagetest virnetsockettest ssh \
-       utiltest virnettlscontexttest
+       utiltest virnettlscontexttest shunloadtest
+
+check_LTLIBRARIES = libshunload.la
 
 # This is a fake SSH we use from virnetsockettest
 ssh_SOURCES = ssh.c
@@ -207,6 +209,7 @@ TESTS = virshtest \
        virnetmessagetest \
        virnetsockettest \
        virnettlscontexttest \
+       shunloadtest \
        utiltest \
        $(test_scripts)
 
@@ -503,6 +506,15 @@ eventtest_SOURCES = \
 eventtest_LDADD = -lrt $(LDADDS)
 endif
 
+libshunload_la_SOURCES = shunloadhelper.c
+libshunload_la_LIBADD = ../src/libvirt.la
+libshunload_la_LDFLAGS = -module -avoid-version -rpath /evil/libtool/hack/to/force/shared/lib/creation
+
+shunloadtest_SOURCES = \
+       shunloadtest.c
+shunloadtest_LDADD = -lpthread
+shunloadtest_DEPENDENCIES = libshunload.la
+
 if WITH_CIL
 CILOPTFLAGS =
 CILOPTINCS =
diff --git a/tests/shunloadhelper.c b/tests/shunloadhelper.c
new file mode 100644 (file)
index 0000000..efd8af0
--- /dev/null
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2011 Red Hat, Inc.
+ *
+ * This 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.
+ *
+ * This 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 this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ */
+
+/*
+ * This is a helper for shunloadtest.c. This function is built into
+ * a shared library and linked with libvirto.so
+ *
+ * The function initializes libvirt and primes the thread local with
+ * an error which needs to be freed at thread exit
+ */
+
+#include <config.h>
+#include "internal.h"
+
+#include <libvirt/libvirt.h>
+#include <libvirt/virterror.h>
+#include <stdlib.h>
+
+static void shunloadError(void *userData ATTRIBUTE_UNUSED,
+                          virErrorPtr error ATTRIBUTE_UNUSED)
+{
+}
+
+void shunloadStart(void);
+
+void shunloadStart(void) {
+    virConnectPtr conn;
+
+    virSetErrorFunc(NULL, shunloadError);
+    virInitialize();
+
+    conn = virConnectOpen("test:///default");
+    virDomainDestroy(NULL);
+    if (conn)
+        virConnectClose(conn);
+}
diff --git a/tests/shunloadtest.c b/tests/shunloadtest.c
new file mode 100644 (file)
index 0000000..2cdb8b8
--- /dev/null
@@ -0,0 +1,153 @@
+/*
+ * Copyright (C) 2011 Red Hat, Inc.
+ *
+ * This 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.
+ *
+ * This 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 this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
+ *
+ */
+
+/*
+ * When libvirt initializes, it creates a thread local for storing
+ * the last virErrorPtr instance. It also registers a cleanup
+ * callback for the thread local that will be invoked whenever
+ * a thread exits.
+ *
+ * If the libvirt.so library was dlopen()'d and is dlclose()'d
+ * while there is still a thread present, then when that thread
+ * later exits, the libvirt cleanup callback will be invoked.
+ * Unfortunately libvirt.so will no longer be in memory so the
+ * callback SEGVs (if you're lucky), or invokes unlreated
+ * code at the same address as the old callback (if you're
+ * unlucky).
+ *
+ * To fix the problem libvirt is linked '-z nodelete' which
+ * prevents the code being removed from memory at dlclose().
+ *
+ * This test case demonstrates this SEGV scenario. If this
+ * test does not SEGV, then the '-z nodelete' fix is working
+ */
+
+#include <config.h>
+
+#ifdef linux
+
+# include <dlfcn.h>
+# include <pthread.h>
+# include <stdbool.h>
+# include <stdio.h>
+# include <unistd.h>
+# include <signal.h>
+
+# include "internal.h"
+# include "ignore-value.h"
+# include "testutils.h"
+
+pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
+pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
+bool running = false;
+bool quit = false;
+
+static void *threadMain(void *arg)
+{
+    void (*startup)(void) = arg;
+
+    startup();
+
+    pthread_mutex_lock(&lock);
+    running = true;
+    pthread_cond_signal(&cond);
+
+    while (!quit) {
+        pthread_cond_wait(&cond, &lock);
+    }
+    pthread_mutex_unlock(&lock);
+
+    return NULL;
+}
+
+static void sigHandler(int sig)
+{
+    ignore_value(write(STDERR_FILENO, "FAIL\n", 5));
+    signal(sig, SIG_DFL);
+    raise(sig);
+}
+
+/* We're not using the testutils.c main() wrapper because
+ * we don't want  'shunloadtest' itself to link against
+ * libvirt.so. We need to test dlopen()'ing of libvirt.so
+ */
+int main(int argc ATTRIBUTE_UNUSED, char **argv)
+{
+    void (*startup)(void);
+    pthread_t t;
+    void *lib;
+    char *theprogname;
+
+    theprogname = argv[0];
+    if (STRPREFIX(theprogname, "./"))
+        theprogname += 2;
+
+    fprintf(stderr, "TEST: %s\n", theprogname);
+    fprintf(stderr, "      .%*s 1   ", 39, "");
+    signal(SIGSEGV, sigHandler);
+
+    if (!(lib = dlopen("./.libs/libshunload.so", RTLD_NOW))) {
+        fprintf(stderr, "Cannot load ./.libs/libshunload.so %s\n", dlerror());
+        return 1;
+    }
+    if (!(startup = dlsym(lib, "shunloadStart"))) {
+        fprintf(stderr, "Cannot find shunloadStart %s\n", dlerror());
+        return 1;
+    }
+
+    /*
+     * Create a thread which is going to initialize libvirt
+     * and raise an error
+     */
+    pthread_create(&t, NULL, threadMain, startup);
+
+    /* Wait for the thread to start and call libvirt */
+    pthread_mutex_lock(&lock);
+    while (!running) {
+        pthread_cond_wait(&cond, &lock);
+    }
+
+    /* Close the shared library (and thus make libvirt.so
+     * non-resident */
+    dlclose(lib);
+
+    /* Tell the thread to quit */
+    quit = true;
+    pthread_cond_signal(&cond);
+    pthread_mutex_unlock(&lock);
+
+    pthread_join(t, NULL);
+
+    /* If we got to here the thread successfully exited without
+     * causing a SEGV !
+     */
+
+    fprintf(stderr, "OK\n");
+
+    return 0;
+}
+
+#else
+
+int main(void)
+{
+    return EXIT_AM_SKIP;
+}
+
+#endif