From 27f1806b90aefa8b1335d0fdc3f109a8ac258638 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Wed, 29 Nov 2017 18:47:42 +0000 Subject: [PATCH] Revert "Fix bug with double unload on unsuccessful dlopen" This reverts commit 58554ccb8ac27fd3b5693efde2e1f7ab2a895ea2. causes /vendor/bin/qseecomd to hit the new abort: [ 8.983301] c5 603 DEBUG: Abort message: 'soinfo=0x7147894cd0 is not in soinfo_list (double unload?)' Bug: http://b/69909887 Bug: http://b/69787209 Change-Id: Ied38f797e0a071a1acc5ed41adf1b45e855143c7 --- linker/linker.cpp | 16 +++++--- tests/dlext_test.cpp | 40 -------------------- tests/libs/Android.bp | 12 ------ tests/libs/dlopen_testlib_missing_symbol.cpp | 37 ------------------ 4 files changed, 11 insertions(+), 94 deletions(-) delete mode 100644 tests/libs/dlopen_testlib_missing_symbol.cpp diff --git a/linker/linker.cpp b/linker/linker.cpp index f1ef55788..85376e0f7 100644 --- a/linker/linker.cpp +++ b/linker/linker.cpp @@ -321,7 +321,9 @@ static void soinfo_free(soinfo* si) { TRACE("name %s: freeing soinfo @ %p", si->get_realpath(), si); if (!solist_remove_soinfo(si)) { - async_safe_fatal("soinfo=%p is not in soinfo_list (double unload?)", 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; } // clear links to/from si @@ -1518,6 +1520,11 @@ bool find_libraries(android_namespace_t* ns, } }); + 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 @@ -1682,6 +1689,8 @@ bool find_libraries(android_namespace_t* ns, si->set_linked(); } }); + + failure_guard.Disable(); } return linked; @@ -1691,7 +1700,7 @@ static soinfo* find_library(android_namespace_t* ns, const char* name, int rtld_flags, const android_dlextinfo* extinfo, soinfo* needed_by) { - soinfo* si = nullptr; + soinfo* si; // readers_map is shared across recursive calls to find_libraries. // However, the map is not shared across different threads. @@ -1710,9 +1719,6 @@ static soinfo* find_library(android_namespace_t* ns, 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 31aadcad1..3f6da59ca 100644 --- a/tests/dlext_test.cpp +++ b/tests/dlext_test.cpp @@ -1051,46 +1051,6 @@ TEST(dlext, ns_unload_between_namespaces) { "\" 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 392823d58..8d0271aae 100644 --- a/tests/libs/Android.bp +++ b/tests/libs/Android.bp @@ -112,18 +112,6 @@ cc_test_library { srcs: ["dlopen_testlib_simple.cpp"], } -// ----------------------------------------------------------------------------- -// 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 // ----------------------------------------------------------------------------- diff --git a/tests/libs/dlopen_testlib_missing_symbol.cpp b/tests/libs/dlopen_testlib_missing_symbol.cpp deleted file mode 100644 index 0f73c604c..000000000 --- a/tests/libs/dlopen_testlib_missing_symbol.cpp +++ /dev/null @@ -1,37 +0,0 @@ -/* - * 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 -#include - -extern "C" void dlopen_testlib_missing_symbol(); - -extern "C" bool dlopen_testlib_simple_func() { - dlopen_testlib_missing_symbol(); - return true; -}