From ee2a3f5d021621358722d56515afabb61953a39a Mon Sep 17 00:00:00 2001 From: solohsu Date: Thu, 20 Jun 2019 20:37:29 +0800 Subject: [PATCH] Fix crash when hooked static method invokes class initialization --- appveyor.yml | 2 +- .../riru/edxp/art/ClassLinker.java | 8 ++++ .../riru/edxp/config/BaseHookProvider.java | 6 +++ .../elderdrivers/riru/edxp/core/Yahfa.java | 2 + .../riru/edxp/proxy/NormalProxy.java | 24 ++++++------ .../riru/edxp/util/ClassUtils.java | 3 +- edxp-core/build.gradle | 4 +- .../cpp/external/yahfa/include/HookMain.h | 2 + .../main/cpp/external/yahfa/src/HookMain.c | 29 +++++++++++---- .../main/include/art/runtime/class_linker.h | 14 ++++++- .../src/main/cpp/main/src/edxp_context.cpp | 19 ++++++++-- .../src/main/cpp/main/src/edxp_context.h | 5 +++ .../cpp/main/src/jni/edxp_pending_hooks.cpp | 30 +++++++++++++++ .../cpp/main/src/jni/edxp_pending_hooks.h | 12 ++++++ .../src/main/cpp/main/src/jni/edxp_yahfa.cpp | 14 +++++++ .../riru/edxp/hook/HookProvider.java | 3 ++ .../de/robv/android/xposed/PendingHooks.java | 37 ++++++++++++++++++- 17 files changed, 186 insertions(+), 28 deletions(-) create mode 100644 edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.cpp create mode 100644 edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.h diff --git a/appveyor.yml b/appveyor.yml index 1d2c9d20..f6a0aa0e 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,4 +1,4 @@ -version: '0.4.4.7_alpha({build})' +version: '0.4.5.0_alpha({build})' environment: ANDROID_HOME: C:\android-sdk-windows diff --git a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/art/ClassLinker.java b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/art/ClassLinker.java index a99cf092..16e169f0 100644 --- a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/art/ClassLinker.java +++ b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/art/ClassLinker.java @@ -1,8 +1,10 @@ package com.elderdrivers.riru.edxp.art; import com.elderdrivers.riru.common.KeepAll; +import com.elderdrivers.riru.edxp.core.Yahfa; import java.lang.reflect.Member; +import java.util.function.Consumer; import de.robv.android.xposed.PendingHooks; @@ -10,7 +12,13 @@ public class ClassLinker implements KeepAll { public static native void setEntryPointsToInterpreter(Member method); + public static void onPreFixupStaticTrampolines(Class clazz) { + // remove modified native flags to let FixupStaticTrampolines fill in right entrypoints + PendingHooks.removeNativeFlags(clazz); + } + public static void onPostFixupStaticTrampolines(Class clazz) { + // native flags will be re-set in hooking logic PendingHooks.hookPendingMethod(clazz); } } diff --git a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/config/BaseHookProvider.java b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/config/BaseHookProvider.java index 9b747f7d..0bb18589 100644 --- a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/config/BaseHookProvider.java +++ b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/config/BaseHookProvider.java @@ -1,5 +1,6 @@ package com.elderdrivers.riru.edxp.config; +import com.elderdrivers.riru.edxp.core.Yahfa; import com.elderdrivers.riru.edxp.deopt.PrebuiltMethodsDeopter; import com.elderdrivers.riru.edxp.hook.HookProvider; @@ -37,4 +38,9 @@ public abstract class BaseHookProvider implements HookProvider { public boolean initXResourcesNative() { return false; } + + @Override + public void setNativeFlag(Member hookMethod, boolean isNative) { + Yahfa.setNativeFlag(hookMethod, isNative); + } } diff --git a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/core/Yahfa.java b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/core/Yahfa.java index f9e3cce6..d439dd55 100644 --- a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/core/Yahfa.java +++ b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/core/Yahfa.java @@ -15,4 +15,6 @@ public class Yahfa { public static native void init(int SDK_version); public static native void setMethodNonCompilable(Member member); + + public static native boolean setNativeFlag(Member member, boolean isNative); } diff --git a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/proxy/NormalProxy.java b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/proxy/NormalProxy.java index 1777fb98..f8510561 100644 --- a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/proxy/NormalProxy.java +++ b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/proxy/NormalProxy.java @@ -29,14 +29,8 @@ public class NormalProxy extends BaseProxy { } public void forkAndSpecializePost(int pid, String appDataDir, String niceName) { - // TODO consider processes without forkAndSpecializePost called - ConfigManager.appDataDir = appDataDir; - ConfigManager.niceName = niceName; - mRouter.prepare(false); - mRouter.onEnterChildProcess(); - // load modules for each app process on its forked if dynamic modules mode is on - mRouter.loadModulesSafely(false, true); - mRouter.onForkFinish(); + // TODO consider processes without forkAndSpecializePost being called + forkPostCommon(pid, false, appDataDir, niceName); } public void forkSystemServerPre(int uid, int gid, int[] gids, int debugFlags, int[][] rlimits, @@ -58,12 +52,20 @@ public class NormalProxy extends BaseProxy { public void forkSystemServerPost(int pid) { // in system_server process - ConfigManager.appDataDir = getDataPathPrefix() + "android"; - ConfigManager.niceName = "system_server"; - mRouter.prepare(true); + forkPostCommon(pid, true, + getDataPathPrefix() + "android", "system_server"); + } + + + private void forkPostCommon(int pid, boolean isSystem, String appDataDir, String niceName) { + ConfigManager.appDataDir = appDataDir; + ConfigManager.niceName = niceName; + mRouter.prepare(isSystem); mRouter.onEnterChildProcess(); // reload module list if dynamic mode is on if (ConfigManager.isDynamicModulesEnabled()) { + // FIXME this could be error-prone because hooks installed inside old versions + // of initZygote instances of same module are not unhooked mRouter.loadModulesSafely(false, true); } mRouter.onForkFinish(); diff --git a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/util/ClassUtils.java b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/util/ClassUtils.java index 872d7b21..a5eb8c9e 100644 --- a/edxp-common/src/main/java/com/elderdrivers/riru/edxp/util/ClassUtils.java +++ b/edxp-common/src/main/java/com/elderdrivers/riru/edxp/util/ClassUtils.java @@ -2,6 +2,7 @@ package com.elderdrivers.riru.edxp.util; import android.os.Build; +import java.lang.reflect.Constructor; import java.lang.reflect.Member; import java.lang.reflect.Modifier; @@ -37,7 +38,7 @@ public class ClassUtils { } public static boolean shouldDelayHook(Member hookMethod) { - if (hookMethod == null) { + if (hookMethod == null || hookMethod instanceof Constructor) { return false; } Class declaringClass = hookMethod.getDeclaringClass(); diff --git a/edxp-core/build.gradle b/edxp-core/build.gradle index 8ad002e6..d1b174ad 100644 --- a/edxp-core/build.gradle +++ b/edxp-core/build.gradle @@ -4,7 +4,7 @@ import org.gradle.internal.os.OperatingSystem apply plugin: 'com.android.library' // Values set here will be overriden by AppVeyor, feel free to modify during development. -def buildVersionName = 'v0.4.4.7_alpha' +def buildVersionName = 'v0.4.5.0_alpha' def buildVersionCode = 10000 if (System.env.APPVEYOR_BUILD_VERSION != null) { @@ -165,7 +165,7 @@ afterEvaluate { into "${zipPathMagiskRelease}/common" filter { line -> line .replaceAll('%VERSION%', "$version") - .replaceAll('%VERSION_CODE%', "$versionCode") + .replaceAll('%VERSION_CODE%', "$versionCode") .replaceAll('%BACKEND%', "$backendCapped") } } copy { diff --git a/edxp-core/src/main/cpp/external/yahfa/include/HookMain.h b/edxp-core/src/main/cpp/external/yahfa/include/HookMain.h index 6847da10..b309a46e 100644 --- a/edxp-core/src/main/cpp/external/yahfa/include/HookMain.h +++ b/edxp-core/src/main/cpp/external/yahfa/include/HookMain.h @@ -23,6 +23,8 @@ void Java_lab_galaxy_yahfa_HookMain_ensureMethodCached(JNIEnv *env, jclass clazz void setNonCompilable(void *method); +bool setNativeFlag(void *method, bool isNative); + static void *getResolvedMethodsAddr(JNIEnv *, jobject); #ifdef __cplusplus diff --git a/edxp-core/src/main/cpp/external/yahfa/src/HookMain.c b/edxp-core/src/main/cpp/external/yahfa/src/HookMain.c index 8cad76f1..fc49eed4 100644 --- a/edxp-core/src/main/cpp/external/yahfa/src/HookMain.c +++ b/edxp-core/src/main/cpp/external/yahfa/src/HookMain.c @@ -2,6 +2,7 @@ #include #include #include +#include #include "common.h" #include "env.h" @@ -128,6 +129,26 @@ void setNonCompilable(void *method) { ); } +bool setNativeFlag(void *method, bool isNative) { + int access_flags = read32((char *) method + OFFSET_access_flags_in_ArtMethod); + LOGI("setNativeFlag: access flags is 0x%x", access_flags); + int old_access_flags = access_flags; + if (isNative) { + access_flags |= kAccNative; + } else { + access_flags &= ~kAccNative; + } + if (access_flags != old_access_flags) { + memcpy( + (char *) method + OFFSET_access_flags_in_ArtMethod, + &access_flags, + 4 + ); + return true; + } + return false; +} + static int doBackupAndHook(JNIEnv *env, void *targetMethod, void *hookMethod, void *backupMethod) { if (hookCount >= hookCap) { LOGI("not enough capacity. Allocating..."); @@ -182,13 +203,7 @@ static int doBackupAndHook(JNIEnv *env, void *targetMethod, void *hookMethod, vo // set the target method to native so that Android O wouldn't invoke it with interpreter if (SDKVersion >= ANDROID_O) { - int access_flags = read32((char *) targetMethod + OFFSET_access_flags_in_ArtMethod); - access_flags |= kAccNative; - memcpy( - (char *) targetMethod + OFFSET_access_flags_in_ArtMethod, - &access_flags, - 4 - ); + setNativeFlag(targetMethod, true); LOGI("access flags is 0x%x", access_flags); } diff --git a/edxp-core/src/main/cpp/main/include/art/runtime/class_linker.h b/edxp-core/src/main/cpp/main/include/art/runtime/class_linker.h index 6f1749a6..bb1e34d7 100644 --- a/edxp-core/src/main/cpp/main/include/art/runtime/class_linker.h +++ b/edxp-core/src/main/cpp/main/include/art/runtime/class_linker.h @@ -3,9 +3,12 @@ #include #include +#include +#include #include "runtime.h" #include "jni_env_ext.h" #include "edxp_context.h" +#include "jni/edxp_pending_hooks.h" namespace art { @@ -28,8 +31,17 @@ namespace art { } CREATE_HOOK_STUB_ENTRIES(void, FixupStaticTrampolines, void *thiz, void *clazz_ptr) { + art::mirror::Class clazz(clazz_ptr); + std::string storage; + const char *desc = clazz.GetDescriptor(&storage); + bool should_intercept = edxp::IsClassPending(desc); + if (UNLIKELY(should_intercept)) { + edxp::Context::GetInstance()->CallOnPreFixupStaticTrampolines(clazz_ptr); + } FixupStaticTrampolinesBackup(thiz, clazz_ptr); - edxp::Context::GetInstance()->CallOnPostFixupStaticTrampolines(clazz_ptr); + if (UNLIKELY(should_intercept)) { + edxp::Context::GetInstance()->CallOnPostFixupStaticTrampolines(clazz_ptr); + } } public: diff --git a/edxp-core/src/main/cpp/main/src/edxp_context.cpp b/edxp-core/src/main/cpp/main/src/edxp_context.cpp index 6b49baf6..d62a7643 100644 --- a/edxp-core/src/main/cpp/main/src/edxp_context.cpp +++ b/edxp-core/src/main/cpp/main/src/edxp_context.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include "edxp_context.h" #include "config_manager.h" @@ -43,8 +44,8 @@ namespace edxp { return inject_class_loader_; } - void Context::CallOnPostFixupStaticTrampolines(void *class_ptr) { - if (UNLIKELY(!post_fixup_static_mid_ || !class_linker_class_)) { + void Context::CallPostFixupStaticTrampolinesCallback(void *class_ptr, jmethodID callback_mid) { + if (UNLIKELY(!callback_mid || !class_linker_class_)) { return; } if (!class_ptr) { @@ -55,10 +56,18 @@ namespace edxp { art::JNIEnvExt env_ext(env); ScopedLocalRef clazz(env, env_ext.NewLocalRefer(class_ptr)); if (clazz != nullptr) { - JNI_CallStaticVoidMethod(env, class_linker_class_, post_fixup_static_mid_, clazz.get()); + JNI_CallStaticVoidMethod(env, class_linker_class_, callback_mid, clazz.get()); } } + void Context::CallOnPreFixupStaticTrampolines(void *class_ptr) { + CallPostFixupStaticTrampolinesCallback(class_ptr, pre_fixup_static_mid_); + } + + void Context::CallOnPostFixupStaticTrampolines(void *class_ptr) { + CallPostFixupStaticTrampolinesCallback(class_ptr, post_fixup_static_mid_); + } + void Context::LoadDexAndInit(JNIEnv *env, const char *dex_path) { if (LIKELY(initialized_)) { return; @@ -97,6 +106,9 @@ namespace edxp { env->GetJavaVM(&vm_); class_linker_class_ = (jclass) env->NewGlobalRef( FindClassFromLoader(env, kClassLinkerClassName)); + pre_fixup_static_mid_ = JNI_GetStaticMethodID(env, class_linker_class_, + "onPreFixupStaticTrampolines", + "(Ljava/lang/Class;)V"); post_fixup_static_mid_ = JNI_GetStaticMethodID(env, class_linker_class_, "onPostFixupStaticTrampolines", "(Ljava/lang/Class;)V"); @@ -110,6 +122,7 @@ namespace edxp { RegisterArtClassLinker(env); RegisterArtHeap(env); RegisterEdxpYahfa(env); + RegisterPendingHooks(env); // must call entry class's methods after all native methods registered if (LIKELY(entry_class_)) { diff --git a/edxp-core/src/main/cpp/main/src/edxp_context.h b/edxp-core/src/main/cpp/main/src/edxp_context.h index e8480ac8..c5cc8042 100644 --- a/edxp-core/src/main/cpp/main/src/edxp_context.h +++ b/edxp-core/src/main/cpp/main/src/edxp_context.h @@ -22,6 +22,8 @@ namespace edxp { jobject GetCurrentClassLoader() const; + void CallOnPreFixupStaticTrampolines(void *class_ptr); + void CallOnPostFixupStaticTrampolines(void *class_ptr); void PrepareJavaEnv(JNIEnv *env); @@ -70,6 +72,7 @@ namespace edxp { jstring nice_name_ = nullptr; JavaVM *vm_ = nullptr; jclass class_linker_class_ = nullptr; + jmethodID pre_fixup_static_mid_ = nullptr; jmethodID post_fixup_static_mid_ = nullptr; Context() {} @@ -79,6 +82,8 @@ namespace edxp { void LoadDexAndInit(JNIEnv *env, const char *dex_path); jclass FindClassFromLoader(JNIEnv *env, jobject class_loader, const char *class_name) const; + + void CallPostFixupStaticTrampolinesCallback(void *class_ptr, jmethodID mid); }; } diff --git a/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.cpp b/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.cpp new file mode 100644 index 00000000..398a5d95 --- /dev/null +++ b/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.cpp @@ -0,0 +1,30 @@ + +#include +#include +#include +#include "jni.h" +#include "native_util.h" +#include "edxp_pending_hooks.h" + +namespace edxp { + + static std::set class_descs_; + + bool IsClassPending(const char *class_desc) { + return class_descs_.find(class_desc) != class_descs_.end(); + } + + static void PendingHooks_recordPendingMethodNative(JNI_START, jstring class_desc) { + const char *class_desc_chars = env->GetStringUTFChars(class_desc, JNI_FALSE); + class_descs_.insert(class_desc_chars); + } + + static JNINativeMethod gMethods[] = { + NATIVE_METHOD(PendingHooks, recordPendingMethodNative, "(Ljava/lang/String;)V"), + }; + + void RegisterPendingHooks(JNIEnv *env) { + REGISTER_EDXP_NATIVE_METHODS("de.robv.android.xposed.PendingHooks"); + } + +} \ No newline at end of file diff --git a/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.h b/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.h new file mode 100644 index 00000000..51c40d45 --- /dev/null +++ b/edxp-core/src/main/cpp/main/src/jni/edxp_pending_hooks.h @@ -0,0 +1,12 @@ + +#pragma once + +#include "jni.h" + +namespace edxp { + + bool IsClassPending(const char *); + + void RegisterPendingHooks(JNIEnv *); + +} // namespace edxp diff --git a/edxp-core/src/main/cpp/main/src/jni/edxp_yahfa.cpp b/edxp-core/src/main/cpp/main/src/jni/edxp_yahfa.cpp index f9b4cbcb..ef8fd37f 100644 --- a/edxp-core/src/main/cpp/main/src/jni/edxp_yahfa.cpp +++ b/edxp-core/src/main/cpp/main/src/jni/edxp_yahfa.cpp @@ -39,6 +39,19 @@ namespace edxp { setNonCompilable(art_method); } + static jboolean Yahfa_setNativeFlag(JNI_START, jobject member, jboolean is_native) { + if (!member) { + LOGE("setNativeFlagNative: member is null"); + return JNI_FALSE; + } + void *art_method = env->FromReflectedMethod(member); + if (!art_method) { + LOGE("setNativeFlagNative: art_method is null"); + return JNI_FALSE; + } + return (jboolean) setNativeFlag(art_method, is_native); + } + static JNINativeMethod gMethods[] = { NATIVE_METHOD(Yahfa, init, "(I)V"), NATIVE_METHOD(Yahfa, findMethodNative, @@ -48,6 +61,7 @@ namespace edxp { NATIVE_METHOD(Yahfa, ensureMethodCached, "(Ljava/lang/reflect/Method;Ljava/lang/reflect/Method;)V"), NATIVE_METHOD(Yahfa, setMethodNonCompilable, "(Ljava/lang/reflect/Member;)V"), + NATIVE_METHOD(Yahfa, setNativeFlag, "(Ljava/lang/reflect/Member;Z)Z"), }; void RegisterEdxpYahfa(JNIEnv *env) { diff --git a/xposed-bridge/src/main/java/com/elderdrivers/riru/edxp/hook/HookProvider.java b/xposed-bridge/src/main/java/com/elderdrivers/riru/edxp/hook/HookProvider.java index 2a5539c6..48f093a0 100644 --- a/xposed-bridge/src/main/java/com/elderdrivers/riru/edxp/hook/HookProvider.java +++ b/xposed-bridge/src/main/java/com/elderdrivers/riru/edxp/hook/HookProvider.java @@ -25,4 +25,7 @@ public interface HookProvider { boolean initXResourcesNative(); boolean removeFinalFlagNative(Class clazz); + + void setNativeFlag(Member hookMethod, boolean isNative); + } diff --git a/xposed-bridge/src/main/java/de/robv/android/xposed/PendingHooks.java b/xposed-bridge/src/main/java/de/robv/android/xposed/PendingHooks.java index ea843829..ae3f9457 100644 --- a/xposed-bridge/src/main/java/de/robv/android/xposed/PendingHooks.java +++ b/xposed-bridge/src/main/java/de/robv/android/xposed/PendingHooks.java @@ -1,14 +1,21 @@ package de.robv.android.xposed; +import com.elderdrivers.riru.edxp.config.EdXpConfigGlobal; + import java.lang.reflect.Member; +import java.lang.reflect.Modifier; +import java.util.HashSet; import java.util.concurrent.ConcurrentHashMap; import static de.robv.android.xposed.XposedBridge.hookMethodNative; public final class PendingHooks { + // GuardedBy("PendingHooks.class") private static final ConcurrentHashMap sPendingHookMethods = new ConcurrentHashMap<>(); + // GuardedBy("PendingHooks.class") + private static final HashSet sNonNativeMethods = new HashSet<>(); public synchronized static void hookPendingMethod(Class clazz) { for (Member member : sPendingHookMethods.keySet()) { @@ -18,7 +25,33 @@ public final class PendingHooks { } } - public synchronized static void recordPendingMethod(Member hookMethod, XposedBridge.AdditionalHookInfo additionalInfo) { - sPendingHookMethods.put(hookMethod, additionalInfo); + public synchronized static void removeNativeFlags(Class clazz) { + for (Member member : sPendingHookMethods.keySet()) { + if (member.getDeclaringClass().equals(clazz) && sNonNativeMethods.contains(member)) { + EdXpConfigGlobal.getHookProvider().setNativeFlag(member, false); + } + } } + + public synchronized static void recordPendingMethod(Member hookMethod, + XposedBridge.AdditionalHookInfo additionalInfo) { + if (!Modifier.isNative(hookMethod.getModifiers())) { + // record non-native methods for later native flag temporary removing + sNonNativeMethods.add(hookMethod); + } + EdXpConfigGlobal.getHookProvider().setNativeFlag(hookMethod, true); + sPendingHookMethods.put(hookMethod, additionalInfo); + recordPendingMethodNative("L" + + hookMethod.getDeclaringClass().getName().replace(".", "/") + ";"); + } + + public synchronized void cleanUp() { + sPendingHookMethods.clear(); + // sNonNativeMethods should be cleared very carefully because their + // pre-set native flag have to be removed if its hooking is cancelled + // before its class is initialized +// sNonNativeMethods.clear(); + } + + private static native void recordPendingMethodNative(String classDesc); }