From d5cda2bd983f3195003707ee1684b4cbecc4dc67 Mon Sep 17 00:00:00 2001 From: solohsu Date: Sun, 10 Mar 2019 17:05:06 +0800 Subject: [PATCH] Review the process of bootstrap Fix potential bootloop when using dynamic module list mode. --- .../com/elderdrivers/riru/xposed/Main.java | 5 +- .../riru/xposed/entry/Router.java | 19 ++++++- .../proxy/yahfa/BlackWhiteListProxy.java | 52 +++++++++++-------- .../riru/xposed/proxy/yahfa/NormalProxy.java | 27 ++++++---- .../de/robv/android/xposed/XposedInit.java | 10 ++-- Core/jni/main/java_hook/java_hook.cpp | 22 ++++---- 6 files changed, 82 insertions(+), 53 deletions(-) diff --git a/Bridge/src/main/java/com/elderdrivers/riru/xposed/Main.java b/Bridge/src/main/java/com/elderdrivers/riru/xposed/Main.java index 0b27948f..812ae673 100644 --- a/Bridge/src/main/java/com/elderdrivers/riru/xposed/Main.java +++ b/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); diff --git a/Bridge/src/main/java/com/elderdrivers/riru/xposed/entry/Router.java b/Bridge/src/main/java/com/elderdrivers/riru/xposed/entry/Router.java index 9dc758b4..f8f7ed8c 100644 --- a/Bridge/src/main/java/com/elderdrivers/riru/xposed/entry/Router.java +++ b/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); } } diff --git a/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/BlackWhiteListProxy.java b/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/BlackWhiteListProxy.java index 8695709f..356c830a 100644 --- a/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/BlackWhiteListProxy.java +++ b/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(); - // at last close all fds - Main.closeFilesBeforeForkNative(); + Router.loadModulesSafely(true); } 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; } - - if (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(); - } + final boolean isDynamicModulesMode = Main.isDynamicModulesEnabled(); + ConfigManager.setDynamicModulesMode(isDynamicModulesMode); + Router.prepare(isSystemServer); + PrebuiltMethodsDeopter.deoptBootMethods(); + Router.reopenFilesIfNeeded(); Router.installBootstrapHooks(isSystemServer); + if (isDynamicModulesMode) { + Router.loadModulesSafely(false); + } } } diff --git a/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/NormalProxy.java b/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/NormalProxy.java index 7303c4ce..71679a41 100644 --- a/Bridge/src/main/java/com/elderdrivers/riru/xposed/proxy/yahfa/NormalProxy.java +++ b/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 - Router.loadModulesSafely(); - Main.closeFilesBeforeForkNative(); + if (Main.closedFdTable == 0) { + // only load modules for secondary zygote + 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(); - Main.closeFilesBeforeForkNative(); + Router.loadModulesSafely(true); } 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); } } diff --git a/Bridge/src/main/java/de/robv/android/xposed/XposedInit.java b/Bridge/src/main/java/de/robv/android/xposed/XposedInit.java index e3c88eea..1e917390 100644 --- a/Bridge/src/main/java/de/robv/android/xposed/XposedInit.java +++ b/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; } diff --git a/Core/jni/main/java_hook/java_hook.cpp b/Core/jni/main/java_hook/java_hook.cpp index c24ca1e5..8cb0a7b0 100644 --- a/Core/jni/main/java_hook/java_hook.cpp +++ b/Core/jni/main/java_hook/java_hook.cpp @@ -20,20 +20,18 @@ jobject gInjectDexClassLoader; static bool isInited = false; -static FileDescriptorTable *gClosedFdTable = nullptr; - -void closeFilesBeforeForkNative(JNIEnv *, jclass) { - gClosedFdTable = FileDescriptorTable::Create(); +jlong closeFilesBeforeForkNative(JNIEnv *, jclass) { + return reinterpret_cast(FileDescriptorTable::Create()); } -void reopenFilesAfterForkNative(JNIEnv *, jclass) { - if (!gClosedFdTable) { - LOGE("gClosedFdTable is null when reopening files"); +void reopenFilesAfterForkNative(JNIEnv *, jclass, jlong fdTable) { + if (fdTable == 0) { + LOGE("fdTable is null when reopening files"); return; } - gClosedFdTable->Reopen(); - delete gClosedFdTable; - gClosedFdTable = nullptr; + auto *closedFdTable = reinterpret_cast(fdTable); + closedFdTable->Reopen(); + 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