From 082a7baa3b2eabe993016a0f2d34fdbcf2c19e3e Mon Sep 17 00:00:00 2001
From: Tim Schumacher <timschumi@gmx.de>
Date: Fri, 24 Jun 2022 10:50:34 +0200
Subject: [PATCH] LibELF: Check if initializers ran instead of trusting
 s_global_objects

The original heuristic of "a library being in `s_global_objects` means
that it was fully initialized already" doesn't hold up anymore since we
changed the loading order. This was causing us to skip parts of the
initialization of dependency libraries when running dlopen (since it was
the only user of that setting).

Instead, set a flag after we run stage 4 (which is the "run the global
initializers" stage) and check that flag when determining unfinished
dependencies. This entirely replaces the `skip_global_objects` logic.
---
 Userland/Libraries/LibELF/DynamicLinker.cpp | 22 +++++++++++----------
 Userland/Libraries/LibELF/DynamicLoader.cpp |  2 ++
 Userland/Libraries/LibELF/DynamicLoader.h   |  4 ++++
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/Userland/Libraries/LibELF/DynamicLinker.cpp b/Userland/Libraries/LibELF/DynamicLinker.cpp
index 44f60b49fc..7f8bf05f65 100644
--- a/Userland/Libraries/LibELF/DynamicLinker.cpp
+++ b/Userland/Libraries/LibELF/DynamicLinker.cpp
@@ -292,12 +292,14 @@ static void initialize_libc(DynamicObject& libc)
 }
 
 template<typename Callback>
-static void for_each_unfinished_dependency_of(String const& name, HashTable<String>& seen_names, bool first, bool skip_global_objects, Callback callback)
+static void for_each_unfinished_dependency_of(String const& name, HashTable<String>& seen_names, Callback callback)
 {
-    if (!s_loaders.contains(name))
+    auto loader = s_loaders.get(name);
+
+    if (!loader.has_value())
         return;
 
-    if (!first && skip_global_objects && s_global_objects.contains(name))
+    if (loader.value()->is_fully_initialized())
         return;
 
     if (seen_names.contains(name))
@@ -305,24 +307,24 @@ static void for_each_unfinished_dependency_of(String const& name, HashTable<Stri
     seen_names.set(name);
 
     for (auto const& needed_name : get_dependencies(name))
-        for_each_unfinished_dependency_of(get_library_name(needed_name), seen_names, false, skip_global_objects, callback);
+        for_each_unfinished_dependency_of(get_library_name(needed_name), seen_names, callback);
 
     callback(*s_loaders.get(name).value());
 }
 
-static NonnullRefPtrVector<DynamicLoader> collect_loaders_for_library(String const& name, bool skip_global_objects)
+static NonnullRefPtrVector<DynamicLoader> collect_loaders_for_library(String const& name)
 {
     HashTable<String> seen_names;
     NonnullRefPtrVector<DynamicLoader> loaders;
-    for_each_unfinished_dependency_of(name, seen_names, true, skip_global_objects, [&](auto& loader) {
+    for_each_unfinished_dependency_of(name, seen_names, [&](auto& loader) {
         loaders.append(loader);
     });
     return loaders;
 }
 
-static Result<void, DlErrorMessage> link_main_library(String const& name, int flags, bool skip_global_objects)
+static Result<void, DlErrorMessage> link_main_library(String const& name, int flags)
 {
-    auto loaders = collect_loaders_for_library(name, skip_global_objects);
+    auto loaders = collect_loaders_for_library(name);
 
     for (auto& loader : loaders) {
         auto dynamic_object = loader.map();
@@ -447,7 +449,7 @@ static Result<void*, DlErrorMessage> __dlopen(char const* filename, int flags)
         return result2.error();
     }
 
-    auto result = link_main_library(library_name, flags, true);
+    auto result = link_main_library(library_name, flags);
     if (result.is_error())
         return result.error();
 
@@ -583,7 +585,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 = link_main_library(library_name, RTLD_GLOBAL | RTLD_LAZY, false);
+        auto result = link_main_library(library_name, RTLD_GLOBAL | RTLD_LAZY);
         if (result.is_error()) {
             warnln("{}", result.error().text);
             _exit(1);
diff --git a/Userland/Libraries/LibELF/DynamicLoader.cpp b/Userland/Libraries/LibELF/DynamicLoader.cpp
index e6affe62e5..05f2c995e5 100644
--- a/Userland/Libraries/LibELF/DynamicLoader.cpp
+++ b/Userland/Libraries/LibELF/DynamicLoader.cpp
@@ -249,6 +249,8 @@ Result<NonnullRefPtr<DynamicObject>, DlErrorMessage> DynamicLoader::load_stage_3
 void DynamicLoader::load_stage_4()
 {
     call_object_init_functions();
+
+    m_fully_initialized = true;
 }
 
 void DynamicLoader::do_lazy_relocations()
diff --git a/Userland/Libraries/LibELF/DynamicLoader.h b/Userland/Libraries/LibELF/DynamicLoader.h
index 47e44056d9..c7bd90d4b5 100644
--- a/Userland/Libraries/LibELF/DynamicLoader.h
+++ b/Userland/Libraries/LibELF/DynamicLoader.h
@@ -82,6 +82,8 @@ public:
 
     DynamicObject const& dynamic_object() const;
 
+    bool is_fully_initialized() const { return m_fully_initialized; }
+
 private:
     DynamicLoader(int fd, String filename, void* file_data, size_t file_size, String filepath);
 
@@ -158,6 +160,8 @@ private:
     Vector<DynamicObject::Relocation> m_unresolved_relocations;
 
     mutable RefPtr<DynamicObject> m_cached_dynamic_object;
+
+    bool m_fully_initialized { false };
 };
 
 template<typename F>