Review the process of bootstrap

Fix potential bootloop when using dynamic module list mode.

Bridge/src/main/java/com/elderdrivers/riru/xposed/Main.java

@@ -18,6 +18,7 @@ public class Main implements KeepAll {
 
     public static String appDataDir = "";
     public static String appProcessName = "";
+    public static long closedFdTable = 0;
     private static String forkAndSpecializePramsStr = "";
     private static String forkSystemServerPramsStr = "";
 
@@ -120,9 +121,9 @@ public class Main implements KeepAll {
 
     // prevent from fatal error caused by holding not whitelisted file descriptors when forking zygote
     // https://github.com/rovo89/Xposed/commit/b3ba245ad04cd485699fb1d2ebde7117e58214ff
-    public static native void closeFilesBeforeForkNative();
+    public static native long closeFilesBeforeForkNative();
 
-    public static native void reopenFilesAfterForkNative();
+    public static native void reopenFilesAfterForkNative(long fdTable);
 
     public static native void deoptMethodNative(Object object);
 

Bridge/src/main/java/com/elderdrivers/riru/xposed/entry/Router.java

@@ -2,6 +2,7 @@ package com.elderdrivers.riru.xposed.entry;
 
 import android.text.TextUtils;
 
+import com.elderdrivers.riru.xposed.Main;
 import com.elderdrivers.riru.xposed.core.HookMain;
 import com.elderdrivers.riru.xposed.dexmaker.DynamicBridge;
 import com.elderdrivers.riru.xposed.entry.bootstrap.AppBootstrapHookInfo;
@@ -50,12 +51,26 @@ public class Router {
         }
     }
 
-    public static void loadModulesSafely() {
+    public static void loadModulesSafely(boolean isInZygote) {
+        boolean loadedByMe;
         try {
             // FIXME some coredomain app can't reading modules.list
-            XposedInit.loadModules();
+            loadedByMe = XposedInit.loadModules();
         } catch (Exception exception) {
             Utils.logE("error loading module list", exception);
+            // return true in case there are files opened...
+            loadedByMe = true;
+        }
+        // at last close all fds
+        if (isInZygote && loadedByMe) {
+            Main.closedFdTable = Main.closeFilesBeforeForkNative();
+        }
+    }
+
+    public static void reopenFilesIfNeeded() {
+        long closedFdTable = Main.closedFdTable;
+        if (closedFdTable != 0) {
+            Main.reopenFilesAfterForkNative(closedFdTable);
         }
     }
 

Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/BlackWhiteListProxy.java

@@ -8,6 +8,25 @@ import com.elderdrivers.riru.xposed.util.PrebuiltMethodsDeopter;
 import static com.elderdrivers.riru.xposed.Main.isAppNeedHook;
 import static com.elderdrivers.riru.xposed.util.FileUtils.getDataPathPrefix;
 
+/**
+ * 1. Non dynamic mode
+ * - system_server is whitelisted
+ * * for all child processes of main zygote
+ * What've been done in main zygote pre-forking system_server
+ * 1) non dynamic flag set (no need to reset)
+ * 2) boot image methods deopted (no need to redo)
+ * 3) startSystemServer flag set to true (need to reset)
+ * 4) workaround hooks installed (need to redo)
+ * 5) module list loaded and initZygote called (no need to redo)
+ * 6) close all fds (no need to redo because of 5))
+ * * for all child processes of secondary zygote
+ * 1) do the same things pre-forking first child process
+ * - system_server is blacklisted:
+ * * for all child processes of both main zygote and secondary zygote
+ * 1) do the same things pre-forking first child process
+ * 2. Dynamic mode:
+ *  to be continued
+ */
 public class BlackWhiteListProxy {
 
     public static void forkAndSpecializePre(int uid, int gid, int[] gids, int debugFlags,
@@ -49,43 +68,32 @@ public class BlackWhiteListProxy {
      */
     private static void onForkPreForNonDynamicMode(boolean isSystemServer) {
         ConfigManager.setDynamicModulesMode(false);
-        // deoptBootMethods once for all child processes of zygote
-        PrebuiltMethodsDeopter.deoptBootMethods();
         // set startsSystemServer flag used when loadModules
         Router.prepare(isSystemServer);
+        // deoptBootMethods once for all child processes of zygote
+        PrebuiltMethodsDeopter.deoptBootMethods();
         // we never install bootstrap hooks here in black/white list mode except workaround hooks
         // because installed hooks would be propagated to all child processes of zygote
         Router.startWorkAroundHook();
         // loadModules once for all child processes of zygote
-        Router.loadModulesSafely();
+        Router.loadModulesSafely(true);
-        // at last close all fds
-        Main.closeFilesBeforeForkNative();
     }
 
     private static void onForkPostCommon(boolean isSystemServer, String appDataDir) {
-        final boolean isDynamicModulesMode = Main.isDynamicModulesEnabled();
-        // set common flags
-        ConfigManager.setDynamicModulesMode(isDynamicModulesMode);
         Main.appDataDir = appDataDir;
         Router.onEnterChildProcess();
-
-        if (!isDynamicModulesMode) {
-            // initial stuffs have been done in forkSystemServerPre
-            Main.reopenFilesAfterForkNative();
-        }
-
         if (!isAppNeedHook(Main.appDataDir)) {
             // if is blacklisted, just stop here
             return;
         }
-
+        final boolean isDynamicModulesMode = Main.isDynamicModulesEnabled();
-        if (isDynamicModulesMode) {
+        ConfigManager.setDynamicModulesMode(isDynamicModulesMode);
-            // nothing has been done in forkSystemServerPre, we have to do the same here
-            // except some workarounds specific for forkSystemServerPre
-            PrebuiltMethodsDeopter.deoptBootMethods();
         Router.prepare(isSystemServer);
-            Router.loadModulesSafely();
+        PrebuiltMethodsDeopter.deoptBootMethods();
-        }
+        Router.reopenFilesIfNeeded();
         Router.installBootstrapHooks(isSystemServer);
+        if (isDynamicModulesMode) {
+            Router.loadModulesSafely(false);
+        }
     }
 }

Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/NormalProxy.java

@@ -14,31 +14,33 @@ public class NormalProxy {
                                             String niceName, int[] fdsToClose, int[] fdsToIgnore,
                                             boolean startChildZygote, String instructionSet,
                                             String appDataDir) {
+        // mainly for secondary zygote
         final boolean isDynamicModulesMode = Main.isDynamicModulesEnabled();
-        Main.appDataDir = appDataDir;
         ConfigManager.setDynamicModulesMode(isDynamicModulesMode);
-        PrebuiltMethodsDeopter.deoptBootMethods(); // do it once for secondary zygote
         // call this to ensure the flag is set to false ASAP
         Router.prepare(false);
+        PrebuiltMethodsDeopter.deoptBootMethods(); // do it once for secondary zygote
         // install bootstrap hooks for secondary zygote
         Router.installBootstrapHooks(false);
-        // load modules for secondary zygote
+        if (Main.closedFdTable == 0) {
-        Router.loadModulesSafely();
+            // only load modules for secondary zygote
-        Main.closeFilesBeforeForkNative();
+            Router.loadModulesSafely(true);
+        }
     }
 
     public static void forkAndSpecializePost(int pid, String appDataDir) {
         // TODO consider processes without forkAndSpecializePost called
-        Main.reopenFilesAfterForkNative();
+        Main.appDataDir = appDataDir;
+        Router.prepare(false);
+        Router.reopenFilesIfNeeded();
         Router.onEnterChildProcess();
         // load modules for each app process on its forked if dynamic modules mode is on
-        Router.loadModulesSafely();
+        Router.loadModulesSafely(false);
     }
 
     public static void forkSystemServerPre(int uid, int gid, int[] gids, int debugFlags, int[][] rlimits,
                                            long permittedCapabilities, long effectiveCapabilities) {
         final boolean isDynamicModulesMode = Main.isDynamicModulesEnabled();
-        Main.appDataDir = getDataPathPrefix() + "android";
         ConfigManager.setDynamicModulesMode(isDynamicModulesMode);
         // set startsSystemServer flag used when loadModules
         Router.prepare(true);
@@ -50,14 +52,17 @@ public class NormalProxy {
         // loadModules have to be executed in zygote even isDynamicModules is false
         // because if not global hooks installed in initZygote might not be
         // propagated to processes not forked via forkAndSpecialize
-        Router.loadModulesSafely();
+        Router.loadModulesSafely(true);
-        Main.closeFilesBeforeForkNative();
     }
 
     public static void forkSystemServerPost(int pid) {
         // in system_server process
-        Main.reopenFilesAfterForkNative();
+        Main.appDataDir = getDataPathPrefix() + "android";
+        Router.prepare(true);
+        Router.reopenFilesIfNeeded();
         Router.onEnterChildProcess();
+        // reload module list if dynamic mode is on
+        Router.loadModulesSafely(false);
     }
 
 }

Bridge/src/main/java/de/robv/android/xposed/XposedInit.java

@@ -57,11 +57,12 @@ public final class XposedInit {
             return;
         }
         Router.startBootstrapHook(isSystem);
+
+        // TODO Are these still needed for us?
         // MIUI
         if (findFieldIfExists(ZygoteInit.class, "BOOT_START_TIME") != null) {
             setStaticLongField(ZygoteInit.class, "BOOT_START_TIME", XposedBridge.BOOT_START_TIME);
         }
-
         // Samsung
         if (Build.VERSION.SDK_INT >= 24) {
             Class<?> zygote = findClass("com.android.internal.os.Zygote", null);
@@ -87,10 +88,10 @@ public final class XposedInit {
      */
     private static volatile AtomicBoolean modulesLoaded = new AtomicBoolean(false);
 
-    public static void loadModules() throws IOException {
+    public static boolean loadModules() throws IOException {
         if (!modulesLoaded.compareAndSet(false, true)
                 && !ConfigManager.isDynamicModulesMode()) {
-            return;
+            return false;
         }
         // FIXME module list is cleared but never could be reload again when using dynamic-module-list under multi-user environment
         XposedBridge.clearLoadedPackages();
@@ -98,7 +99,7 @@ public final class XposedInit {
         BaseService service = SELinuxHelper.getAppDataFileService();
         if (!service.checkFileExists(filename)) {
             Log.e(TAG, "Cannot load any modules because " + filename + " was not found");
-            return;
+            return false;
         }
 
         ClassLoader topClassLoader = XposedBridge.BOOTCLASSLOADER;
@@ -114,6 +115,7 @@ public final class XposedInit {
             loadModule(apk, topClassLoader);
         }
         apks.close();
+        return true;
     }
 
 

Core/jni/main/java_hook/java_hook.cpp

@@ -20,20 +20,18 @@ jobject gInjectDexClassLoader;
 
 static bool isInited = false;
 
-static FileDescriptorTable *gClosedFdTable = nullptr;
+jlong closeFilesBeforeForkNative(JNIEnv *, jclass) {
-
+    return reinterpret_cast<jlong>(FileDescriptorTable::Create());
-void closeFilesBeforeForkNative(JNIEnv *, jclass) {
-    gClosedFdTable = FileDescriptorTable::Create();
 }
 
-void reopenFilesAfterForkNative(JNIEnv *, jclass) {
+void reopenFilesAfterForkNative(JNIEnv *, jclass, jlong fdTable) {
-    if (!gClosedFdTable) {
+    if (fdTable == 0) {
-        LOGE("gClosedFdTable is null when reopening files");
+        LOGE("fdTable is null when reopening files");
         return;
     }
-    gClosedFdTable->Reopen();
+    auto *closedFdTable = reinterpret_cast<FileDescriptorTable *>(fdTable);
-    delete gClosedFdTable;
+    closedFdTable->Reopen();
-    gClosedFdTable = nullptr;
+    delete closedFdTable;
 }
 
 jlong suspendAllThreads(JNIEnv *, jclass) {
@@ -92,10 +90,10 @@ static JNINativeMethod hookMethods[] = {
                 "getInstallerPkgName", "()Ljava/lang/String;", (void *) get_installer_pkg_name
         },
         {
-                "closeFilesBeforeForkNative", "()V", (void *) closeFilesBeforeForkNative
+                "closeFilesBeforeForkNative", "()J", (void *) closeFilesBeforeForkNative
         },
         {
-                "reopenFilesAfterForkNative", "()V", (void *) reopenFilesAfterForkNative
+                "reopenFilesAfterForkNative", "(J)V", (void *) reopenFilesAfterForkNative
         },
         {
                 "deoptMethodNative", "(Ljava/lang/Object;)V", (void *) deoptimize_method