improve: SoInfo hiding code complexity

This commit improves the SoInfo hiding code by reducing the complexity of it, using dlclose directly when possible to make it more future proof and simple.

Co-Authored-By: 4h9fbZ <176179231+4h9fbZ@users.noreply.github.com>

loader/src/injector/entry.cpp

@@ -1,5 +1,6 @@
 #include "daemon.h"
 #include "logging.h"
+#include "solist.h"
 #include "zygisk.hpp"
 
 using namespace std;
@@ -8,7 +9,7 @@ void *start_addr = nullptr;
 size_t block_size = 0;
 
 extern "C" [[gnu::visibility("default")]]
-void entry(void* addr, size_t size, const char* path) {
+void entry(void *addr, size_t size) {
     LOGD("Zygisk library injected, version %s", ZKSU_VERSION);
 
     start_addr = addr;
@@ -23,7 +24,10 @@ void entry(void* addr, size_t size, const char* path) {
     LOGD("start plt hooking");
     hook_functions();
 
-    void *module_addrs[1] = { addr };
-    clean_trace(path, module_addrs, 1, 1, 0);
+    solist_drop_so_path(addr, true);
+    solist_reset_counters(1, 1);
+
     send_seccomp_event();
+
+    LOGD("Zygisk library execution done, addr: %p, size: %zu", addr, size);
 }

loader/src/injector/hook.cpp

@@ -734,24 +734,23 @@ void ZygiskContext::run_modules_post() {
         if (flags[APP_SPECIALIZE]) m.postAppSpecialize(args.app);
         else if (flags[SERVER_FORK_AND_SPECIALIZE]) m.postServerSpecialize(args.server);
 
+        /* INFO: If module is unloaded by dlclose, there's no need to
+                   hide it from soinfo manually. */
         if (m.tryUnload()) modules_unloaded++;
+        else {
+            bool has_dropped = solist_drop_so_path(m.getEntry(), false);
+            if (!has_dropped) continue;
+
+            LOGD("Dropped solist record for %p", m.getEntry());
+        }
     }
 
     if (modules.size() > 0) {
-        LOGD("modules unloaded: %zu/%zu", modules_unloaded, modules.size());
+        LOGD("Modules unloaded: %zu/%zu", modules_unloaded, modules.size());
 
-        /* INFO: While Variable Length Arrays (VLAs) aren't usually
-                   recommended due to the ease of using too much of the
-                   stack, this should be fine since it should not be
-                   possible to exhaust the stack with only a few addresses. */
-        void *module_addrs[modules.size() * sizeof(void *)];
+        solist_reset_counters(modules.size(), modules_unloaded);
 
-        size_t i = 0;
-        for (const auto &m : modules) {
-            module_addrs[i++] = m.getEntry();
-        }
-
-        clean_trace("/data/adb", module_addrs, modules.size(), modules.size(), modules_unloaded);
+        LOGD("Returned global counters to their original values");
     }
 }
 
@@ -1006,22 +1005,6 @@ static void hook_register(dev_t dev, ino_t inode, const char *symbol, void *new_
 #define PLT_HOOK_REGISTER(DEV, INODE, NAME) \
     PLT_HOOK_REGISTER_SYM(DEV, INODE, #NAME, NAME)
 
-/* INFO: module_addrs_length is always the same as "load" */
-void clean_trace(const char *path, void **module_addrs, size_t module_addrs_length, size_t load, size_t unload) {
-    LOGD("cleaning trace for path %s", path);
-
-    if (load > 0 || unload > 0) solist_reset_counters(load, unload);
-
-    LOGD("Dropping solist record for %s", path);
-
-    for (size_t i = 0; i < module_addrs_length; i++) {
-        bool has_dropped = solist_drop_so_path(module_addrs[i]);
-        if (!has_dropped) continue;
-
-        LOGD("Dropped solist record for %p", module_addrs[i]);
-    }
-}
-
 void hook_functions() {
     plt_hook_list = new vector<tuple<dev_t, ino_t, const char *, void **>>();
     jni_hook_list = new map<string, vector<JNINativeMethod>>();

loader/src/injector/solist.c

@@ -1,8 +1,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <string.h>
-
-#include <android/dlext.h>
+#include <dlfcn.h>
 
 #include <linux/limits.h>
 
@@ -24,6 +23,7 @@
 static const char *(*get_realpath_sym)(SoInfo *) = NULL;
 static void (*soinfo_free)(SoInfo *) = NULL;
 static SoInfo *(*find_containing_library)(const void *p) = NULL;
+static void (*purge_unused_memory)(void) = NULL;
 
 static inline const char *get_path(SoInfo *self) {
   if (get_realpath_sym)
@@ -104,6 +104,8 @@ static bool solist_init() {
 
     ElfImg_destroy(linker);
 
+    somain = NULL;
+
     return false;
   }
 
@@ -115,6 +117,8 @@ static bool solist_init() {
 
     ElfImg_destroy(linker);
 
+    somain = NULL;
+
     return false;
   }
 
@@ -126,9 +130,26 @@ static bool solist_init() {
 
     ElfImg_destroy(linker);
 
+    somain = NULL;
+
     return false;
   }
 
+  LOGD("%p is find_containing_library", (void *)find_containing_library);
+
+  purge_unused_memory = (void (*)())getSymbAddress(linker, "__dl__Z19purge_unused_memoryv");
+  if (purge_unused_memory == NULL) {
+    LOGE("Failed to find purge_unused_memory __dl__Z19purge_unused_memoryv");
+
+    ElfImg_destroy(linker);
+
+    somain = NULL;
+
+    return false;
+  }
+
+  LOGD("%p is purge_unused_memory", (void *)purge_unused_memory);
+
   g_module_load_counter = (size_t *)getSymbAddress(linker, "__dl__ZL21g_module_load_counter");
   if (g_module_load_counter != NULL) LOGD("found symbol g_module_load_counter");
 
@@ -154,7 +175,7 @@ static bool solist_init() {
 
 /* INFO: find_containing_library returns the SoInfo for the library that contains
            that memory inside its limits, hence why named "lib_memory" in ReZygisk. */
-bool solist_drop_so_path(void *lib_memory) {
+bool solist_drop_so_path(void *lib_memory, bool unload) {
   if (somain == NULL && !solist_init()) {
     LOGE("Failed to initialize solist");
 
@@ -165,6 +186,8 @@ bool solist_drop_so_path(void *lib_memory) {
   if (found == NULL) {
     LOGD("Could not find containing library for %p", lib_memory);
 
+    purge_unused_memory();
+
     return false;
   }
 
@@ -176,16 +199,44 @@ bool solist_drop_so_path(void *lib_memory) {
 
     return false;
   }
-  strcpy(path, get_path(found));
+  strncpy(path, get_path(found), sizeof(path) - 1);
 
+  /* INFO: This area is guarded. Must unprotect first. */
   pdg_unprotect();
-
   set_size(found, 0);
-  soinfo_free(found);
+  if (unload) pdg_protect();
 
-  pdg_protect();
+  LOGD("Set size of %p to 0", (void *)found);
 
-  LOGD("Successfully dropped so path for: %s", path);
+  /* INFO: We know that as libzygisk.so our limits, but modules are arbitrary, so
+             calling deconstructors might break them. To avoid that, we manually call
+             the separated structures, that however won't clean all traces in soinfo,
+             not for now, at least. */
+  if (unload && dlclose((void *)found) == -1) {
+    LOGE("Failed to dlclose so path for %s: %s", path, dlerror());
+
+    return false;
+  } else if (!unload) {
+    LOGD("Not unloading so path for %s, only dropping it", path);
+
+    /* TODO: call notify_gdb_of_unload(found); (it is static) to avoid leaving traces in
+               r_debug_tail.
+       SOURCES:
+        - https://android.googlesource.com/platform/bionic/+/refs/heads/main/linker/linker_gdb_support.cpp#94
+    */
+    /* INFO: unregister_soinfo_tls cannot be used since module might use JNI which may
+               require TLS, so we cannot remove it. */
+    soinfo_free(found);
+
+    pdg_protect();
+  }
+
+  LOGD("Successfully hidden soinfo traces for %s", path);
+
+  /* INFO: Avoid leaks by ensuring the freed places are munmapped */
+  purge_unused_memory();
+
+  LOGD("Purged unused memory successfully");
 
   /* INFO: Let's avoid trouble regarding detections */
   memset(path, strlen(path), 0);

loader/src/injector/solist.h

@@ -5,11 +5,7 @@
 extern "C" {
 #endif /* __cplusplus */
 
-typedef struct SoInfo SoInfo;
-
-struct SoInfo {
-  char data[0];
-};
+typedef void SoInfo;
 
 #define FuncType(name) void (*name)
 
@@ -28,14 +24,20 @@ struct pdg {
           libzygisk.so, so that it doesn't create gaps between current module info
           and the next (soinfo).
 
-        To do that, we use 2 functions: soinfo_free, and set_size, which will
-          zero the region size, and then remove all traces of that library (libzygisk.so)
-          which was previously loaded.
+        To do that, we use 2 functions: set_size and dlclose, which will first zero
+          zero the size that the linker believes the shared library is, and then dlclose.
+          Because the size is 0, it won't munmap the library, allowing us to keep loaded while
+          having all other traces removed.
+
+        For the case of modules, which are arbitrary, we won't call dlclose, as it could break
+          the module. Instead of using dlclose, we separately call soinfo_free, which will free
+          the soinfo structure. That will allow to keep the data initialized by constructors
+          mmaped, hence properly dropping most traces without breaking the module.
 
   SOURCES:
    - https://android.googlesource.com/platform/bionic/+/refs/heads/android15-release/linker/linker.cpp#1712
 */
-bool solist_drop_so_path(void *lib_memory);
+bool solist_drop_so_path(void *lib_memory, bool unload);
 
 /* 
   INFO: When dlopen'ing a library, the system will increment 1 to a global

loader/src/injector/zygisk.hpp

@@ -7,6 +7,4 @@ extern size_t block_size;
 
 void hook_functions();
 
-void clean_trace(const char *path, void **module_addrs, size_t module_addrs_length, size_t load, size_t unload);
-
 extern "C" void send_seccomp_event();

loader/src/ptracer/ptracer.c

@@ -368,10 +368,8 @@ bool inject_on_main(int pid, const char *lib_path) {
     /* call injector entry(start_addr, block_size, path) */
     args[0] = (uintptr_t)start_addr;
     args[1] = block_size;
-    str = push_string(pid, &regs, rezygiskd_get_path());
-    args[2] = (uintptr_t)str;
 
-    remote_call(pid, &regs, injector_entry, (uintptr_t)libc_return_addr, args, 3);
+    remote_call(pid, &regs, injector_entry, (uintptr_t)libc_return_addr, args, 2);
 
     free(args);