Fix bug with double unload on unsuccessful dlopen
In the case of unsuccessful dlopen the failure guard is triggered
for two namespaces which leads to double unload.
Also update soinfo_free to abort in case when linker tries to free same
soinfo for the second time - this makes linker behavior less undefined.
Test: bionic-unit-tests
Bug: http://b/69787209
Change-Id: I886787ee021b050667f967bce7aa2708390886ea
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 85376e0..f1ef557 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -321,9 +321,7 @@
TRACE("name %s: freeing soinfo @ %p", si->get_realpath(), si);
if (!solist_remove_soinfo(si)) {
- // TODO (dimitry): revisit this - for now preserving the logic
- // but it does not look right, abort if soinfo is not in the list instead?
- return;
+ async_safe_fatal("soinfo=%p is not in soinfo_list (double unload?)", si);
}
// clear links to/from si
@@ -1520,11 +1518,6 @@
}
});
- auto failure_guard = android::base::make_scope_guard([&]() {
- // Housekeeping
- soinfo_unload(soinfos, soinfos_count);
- });
-
ZipArchiveCache zip_archive_cache;
// Step 1: expand the list of load_tasks to include
@@ -1689,8 +1682,6 @@
si->set_linked();
}
});
-
- failure_guard.Disable();
}
return linked;
@@ -1700,7 +1691,7 @@
const char* name, int rtld_flags,
const android_dlextinfo* extinfo,
soinfo* needed_by) {
- soinfo* si;
+ soinfo* si = nullptr;
// readers_map is shared across recursive calls to find_libraries.
// However, the map is not shared across different threads.
@@ -1719,6 +1710,9 @@
false /* add_as_children */,
true /* search_linked_namespaces */,
readers_map)) {
+ if (si != nullptr) {
+ soinfo_unload(si);
+ }
return nullptr;
}
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 3f6da59..31aadca 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -1051,6 +1051,46 @@
"\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror());
}
+TEST(dlext, ns_unload_between_namespaces_missing_symbol) {
+ ASSERT_TRUE(android_init_anonymous_namespace(g_core_shared_libs.c_str(), nullptr));
+
+ const std::string public_ns_search_path = get_testlib_root() + "/public_namespace_libs";
+ const std::string private_ns_search_path = get_testlib_root() + "/private_namespace_libs";
+
+ android_namespace_t* ns_public =
+ android_create_namespace("public",
+ nullptr,
+ public_ns_search_path.c_str(),
+ ANDROID_NAMESPACE_TYPE_ISOLATED,
+ nullptr,
+ nullptr);
+
+ ASSERT_TRUE(android_link_namespaces(ns_public, nullptr, g_core_shared_libs.c_str())) << dlerror();
+
+ android_namespace_t* ns_private =
+ android_create_namespace("private",
+ nullptr,
+ private_ns_search_path.c_str(),
+ ANDROID_NAMESPACE_TYPE_ISOLATED,
+ nullptr,
+ nullptr);
+
+ ASSERT_TRUE(android_link_namespaces(ns_private, ns_public, "libtest_missing_symbol.so")) << dlerror();
+ ASSERT_TRUE(android_link_namespaces(ns_private, nullptr, g_core_shared_libs.c_str())) << dlerror();
+
+ android_dlextinfo extinfo;
+ extinfo.flags = ANDROID_DLEXT_USE_NAMESPACE;
+ extinfo.library_namespace = ns_private;
+
+ void* handle = android_dlopen_ext((public_ns_search_path + "/libtest_missing_symbol.so").c_str(),
+ RTLD_NOW,
+ &extinfo);
+ ASSERT_TRUE(handle == nullptr);
+ ASSERT_EQ(std::string("dlopen failed: cannot locate symbol \"dlopen_testlib_missing_symbol\" referenced by \"") +
+ public_ns_search_path + "/libtest_missing_symbol.so\"...",
+ dlerror());
+}
+
TEST(dlext, ns_greylist_enabled) {
ASSERT_TRUE(android_init_anonymous_namespace(g_core_shared_libs.c_str(), nullptr));
diff --git a/tests/libs/Android.bp b/tests/libs/Android.bp
index 8d0271a..392823d 100644
--- a/tests/libs/Android.bp
+++ b/tests/libs/Android.bp
@@ -113,6 +113,18 @@
}
// -----------------------------------------------------------------------------
+// Library used by dlfcn unload tests
+// -----------------------------------------------------------------------------
+cc_test_library {
+ name: "libtest_missing_symbol",
+ defaults: ["bionic_testlib_defaults"],
+ srcs: ["dlopen_testlib_missing_symbol.cpp"],
+ allow_undefined_symbols: true,
+ relative_install_path: "bionic-loader-test-libs/public_namespace_libs",
+}
+
+// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------
// Library used by dlfcn nodelete tests
// -----------------------------------------------------------------------------
cc_test_library {
diff --git a/tests/libs/dlopen_testlib_missing_symbol.cpp b/tests/libs/dlopen_testlib_missing_symbol.cpp
new file mode 100644
index 0000000..0f73c60
--- /dev/null
+++ b/tests/libs/dlopen_testlib_missing_symbol.cpp
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdint.h>
+#include <stdlib.h>
+
+extern "C" void dlopen_testlib_missing_symbol();
+
+extern "C" bool dlopen_testlib_simple_func() {
+ dlopen_testlib_missing_symbol();
+ return true;
+}