LibELF: Fix dynamic linking of dlopen()-ed libs

Consider the situation where two shared libraries libA and libB, both
depending (as in having a NEEDED dtag) on libC. libA is first
dlopen()-ed, which produces libC to be mapped and linked. When libB is
dlopen()-ed the DynamicLinker would re-map and re-link libC though,
causing any previous references to its old location to be invalid. And
if libA's PLT has been patched to point to libC's symbols, then any
further invocations to libA will cause the code to jump to a virtual
address that isn't mapped anymore, therefore causing a crash. This
situation was reported in #10014, although the setup was more convolved
in the ticket.

This commit fixes the issue by distinguishing between a main program
loading being performed by Loader.so, and a dlopen() call. The main
difference between these two cases is that in the former the
s_globals_objects maps is always empty, while in the latter it might
already contain dependencies for the library being dlopen()-ed. Hence,
when collecting dependencies to map and link, dlopen() should skip those
that are present in the global map to avoid the issue described above.

With this patch the original issue seen in #10014 is gone, with all
python3 modules (so far) loading correctly.

A unit test reproducing a simplified issue is also included in this
commit. The unit test includes the building of two dynamic libraries A
and B with both depending on libline.so (and B also depending on A); the
test then dlopen()s libA, invokes one its function, then does the same
with libB.

Tests/LibELF/CMakeLists.txt

@@ -1,7 +1,21 @@
+macro(add_dlopen_lib NAME FUNCTION)
+    add_library(${NAME} SHARED Dynlib.cpp)
+    target_compile_definitions(${NAME} PRIVATE -DFUNCTION=${FUNCTION})
+    # LibLine is not special, just an "external" dependency
+    target_link_libraries(${NAME} LibLine)
+	 # Avoid execution by the test runner
+    install(TARGETS ${NAME}
+            DESTINATION usr/Tests/LibELF
+            PERMISSIONS OWNER_READ GROUP_READ WORLD_READ)
+endmacro()
+add_dlopen_lib(DynlibA dynliba_function)
+add_dlopen_lib(DynlibB dynlibb_function)
+
 set(TEST_SOURCES
     test-elf.cpp
+    TestDlOpen.cpp
 )
 
 foreach(source IN LISTS TEST_SOURCES)
-    serenity_test("${source}"  LibELF)
+    serenity_test("${source}" LibELF LIBS LibDl)
 endforeach()

Tests/LibELF/Dynlib.cpp

@@ -0,0 +1,15 @@
+/*
+ * Copyright (c) 2021, Rodrigo Tobar <rtobarc@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibLine/Span.h>
+
+extern "C" {
+int FUNCTION();
+int FUNCTION()
+{
+    return (int)Line::Span(0, 0).beginning();
+}
+}

Tests/LibELF/TestDlOpen.cpp

@@ -0,0 +1,25 @@
+/*
+ * Copyright (c) 2021, Rodrigo Tobar <rtobarc@gmail.com>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <LibDl/dlfcn.h>
+#include <LibTest/TestCase.h>
+
+TEST_CASE(test_dlopen)
+{
+    auto liba = dlopen("/usr/Tests/LibELF/libDynlibA.so", 0);
+    EXPECT_NE(liba, nullptr);
+    auto libb = dlopen("/usr/Tests/LibELF/libDynlibB.so", 0);
+    EXPECT_NE(libb, nullptr);
+
+    typedef int (*dynlib_func_t)();
+    dynlib_func_t func_a = (dynlib_func_t)dlsym(liba, "dynliba_function");
+    EXPECT_NE(func_a, nullptr);
+    EXPECT_EQ(0, func_a());
+
+    dynlib_func_t func_b = (dynlib_func_t)dlsym(libb, "dynlibb_function");
+    EXPECT_NE(func_b, nullptr);
+    EXPECT_EQ(0, func_b());
+}

Userland/Libraries/LibELF/DynamicLinker.cpp

@@ -257,38 +257,41 @@ static void initialize_libc(DynamicObject& libc)
 }
 
 template<typename Callback>
-static void for_each_unfinished_dependency_of(const String& name, HashTable<String>& seen_names, Callback callback)
+static void for_each_unfinished_dependency_of(const String& name, HashTable<String>& seen_names, bool first, bool skip_global_objects, Callback callback)
 {
     if (!s_loaders.contains(name))
         return;
 
+    if (!first && skip_global_objects && s_global_objects.contains(name))
+        return;
+
     if (seen_names.contains(name))
         return;
     seen_names.set(name);
 
     for (const auto& needed_name : get_dependencies(name))
-        for_each_unfinished_dependency_of(get_library_name(needed_name), seen_names, callback);
+        for_each_unfinished_dependency_of(get_library_name(needed_name), seen_names, false, skip_global_objects, callback);
 
     callback(*s_loaders.get(name).value());
 }
 
-static NonnullRefPtrVector<DynamicLoader> collect_loaders_for_library(const String& name)
+static NonnullRefPtrVector<DynamicLoader> collect_loaders_for_library(const String& name, bool skip_global_objects)
 {
     HashTable<String> seen_names;
     NonnullRefPtrVector<DynamicLoader> loaders;
-    for_each_unfinished_dependency_of(name, seen_names, [&](auto& loader) {
+    for_each_unfinished_dependency_of(name, seen_names, true, skip_global_objects, [&](auto& loader) {
         loaders.append(loader);
     });
     return loaders;
 }
 
-static Result<NonnullRefPtr<DynamicLoader>, DlErrorMessage> load_main_library(const String& name, int flags)
+static Result<NonnullRefPtr<DynamicLoader>, DlErrorMessage> load_main_library(const String& name, int flags, bool skip_global_objects)
 {
     auto main_library_loader = *s_loaders.get(name);
     auto main_library_object = main_library_loader->map();
     s_global_objects.set(name, *main_library_object);
 
-    auto loaders = collect_loaders_for_library(name);
+    auto loaders = collect_loaders_for_library(name, skip_global_objects);
 
     for (auto& loader : loaders) {
         auto dynamic_object = loader.map();
@@ -411,7 +414,7 @@ static Result<void*, DlErrorMessage> __dlopen(const char* filename, int flags)
         return result2.error();
     }
 
-    auto result = load_main_library(library_name, flags);
+    auto result = load_main_library(library_name, flags, true);
     if (result.is_error())
         return result.error();
 
@@ -535,7 +538,7 @@ void ELF::DynamicLinker::linker_main(String&& main_program_name, int main_progra
 
     auto entry_point_function = [&main_program_name] {
         auto library_name = get_library_name(main_program_name);
-        auto result = load_main_library(library_name, RTLD_GLOBAL | RTLD_LAZY);
+        auto result = load_main_library(library_name, RTLD_GLOBAL | RTLD_LAZY, false);
         if (result.is_error()) {
             warnln("{}", result.error().text);
             _exit(1);