From 4e6cc3e400e23093fecc5161c6c30d9653105a3e Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Sun, 2 Aug 2026 23:20:45 +0200 Subject: [PATCH] Stop leaving JNI exceptions pending across the native boundary A Java method that throws does not unwind into C++. The exception is left pending on the thread, and the runtime asserts on that at the next transition: on a device, a throwing framework entry aborted system_server with "No pending exception expected" and the phone boot looped. CheckJNI is not needed for this; the assert is always on. Six places let that happen. FindAndCall handed the framework entry to Java and looked at nothing afterwards, so the abort was all anyone got, while the log line above it still said the framework had been injected. Two lookups in resources_hook returned JNI_FALSE to Java with NoSuchMethodError pending, so a caller that asked for a boolean got a throw. RegisterNatives, LogcatMonitor's refreshFd lookup and dex2oat's string read did the same on their failure paths, and the obfuscation map builder returned null on a failed FindClass without clearing, then fed two unchecked method ids to NewObject. Most of them are now the lsplant JNI wrappers, which clear the exception, log the Java stack behind it, and return scoped references -- that last part also releases the local reference the obfuscation map leaked per entry. Where the caller has to know the outcome, the check stays explicit, because a wrapper clears the exception before anyone can ask. The trace is rendered through Log.getStackTraceString rather than ExceptionDescribe. ExceptionDescribe writes to stderr, which in a process forked from the zygote goes nowhere: measured on a device, it produced no output at all, which would have traded an aborting-but-informative tombstone for a survivable process and no stack. SetAllowUnload(false) deliberately stays unconditional: the ART and JNI hooks are installed before the entry runs and their trampolines point into this library, so a failed entry is not a reason to let it be unloaded. hook_bridge is untouched. It implements Method.invoke semantics and has to leave a target's exception pending so it can wrap it in InvocationTargetException. --- daemon/src/main/jni/dex2oat.cpp | 6 +++++ daemon/src/main/jni/logcat.cpp | 14 +++++++++-- daemon/src/main/jni/obfuscation.cpp | 36 ++++++++++++++--------------- native/include/core/context.h | 31 ++++++++++++++++++++----- native/include/jni/jni_bridge.h | 4 +++- native/src/jni/resources_hook.cpp | 34 ++++++++++++++++----------- zygisk/src/main/cpp/module.cpp | 30 ++++++++++++++++-------- 7 files changed, 106 insertions(+), 49 deletions(-) diff --git a/daemon/src/main/jni/dex2oat.cpp b/daemon/src/main/jni/dex2oat.cpp index 32cf77d07..a45777ddf 100644 --- a/daemon/src/main/jni/dex2oat.cpp +++ b/daemon/src/main/jni/dex2oat.cpp @@ -106,6 +106,12 @@ extern "C" JNIEXPORT jboolean JNICALL Java_org_matrix_vector_daemon_env_Dex2OatServer_setSockCreateContext(JNIEnv *env, jclass, jstring contextStr) { const char *context = contextStr ? env->GetStringUTFChars(contextStr, nullptr) : nullptr; + if (contextStr && !context) { + // Only OutOfMemoryError puts us here, and it is pending: returning into Java with it still + // set would surface it at the next unrelated call. + env->ExceptionClear(); + return false; + } int ret = setsockcreatecon_raw(context); if (context) env->ReleaseStringUTFChars(contextStr, context); return ret == 0; diff --git a/daemon/src/main/jni/logcat.cpp b/daemon/src/main/jni/logcat.cpp index ac754ab9c..ea350e9dc 100644 --- a/daemon/src/main/jni/logcat.cpp +++ b/daemon/src/main/jni/logcat.cpp @@ -1,5 +1,9 @@ #include "logcat.h" +#include + +#include "logging.h" + #include #include #include @@ -241,8 +245,14 @@ void Logcat::Run() { extern "C" JNIEXPORT void JNICALL Java_org_matrix_vector_daemon_env_LogcatMonitor_runLogcat(JNIEnv* env, jobject thiz) { - jclass clazz = env->GetObjectClass(thiz); - jmethodID method = env->GetMethodID(clazz, "refreshFd", "(Z)I"); + auto clazz = lsplant::JNI_GetObjectClass(env, thiz); + auto method = lsplant::JNI_GetMethodID(env, clazz, "refreshFd", "(Z)I"); + if (!method) { + // The wrapper has already logged and cleared the NoSuchMethodError. Running with a null + // method id would abort inside the first refresh instead of saying why. + LOGE("LogcatMonitor.refreshFd is missing; not starting the log reader"); + return; + } Logcat daemon(env, thiz, method); daemon.Run(); } diff --git a/daemon/src/main/jni/obfuscation.cpp b/daemon/src/main/jni/obfuscation.cpp index d6ecc83fa..42492fa11 100644 --- a/daemon/src/main/jni/obfuscation.cpp +++ b/daemon/src/main/jni/obfuscation.cpp @@ -119,30 +119,30 @@ static void ensureInitialized(JNIEnv *env) { }); } +// Through the lsplant wrappers rather than raw JNI: each one clears a pending exception and logs +// the Java stack behind it, which is what a failed lookup here would otherwise cost. Returning a +// null jclass while leaving NoClassDefFoundError pending -- as the raw form did -- hands the next +// JNI call undefined behaviour. They also return scoped references, so the local refs this loop +// used to leak per entry are released on the spot. static jobject stringMapToJavaHashMap(JNIEnv *env, const std::map &map) { - jclass mapClass = env->FindClass("java/util/HashMap"); - if (mapClass == nullptr) return nullptr; + auto map_class = lsplant::JNI_FindClass(env, "java/util/HashMap"); + if (!map_class) return nullptr; - jmethodID init = env->GetMethodID(mapClass, "", "()V"); - jobject hashMap = env->NewObject(mapClass, init); - jmethodID put = env->GetMethodID(mapClass, "put", - "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); + auto init = lsplant::JNI_GetMethodID(env, map_class, "", "()V"); + auto put = lsplant::JNI_GetMethodID(env, map_class, "put", + "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); + if (!init || !put) return nullptr; - for (const auto &[key, value] : map) { - jstring keyJava = env->NewStringUTF(key.c_str()); - jstring valueJava = env->NewStringUTF(value.c_str()); - - env->CallObjectMethod(hashMap, put, keyJava, valueJava); + auto hash_map = lsplant::JNI_NewObject(env, map_class, init); + if (!hash_map) return nullptr; - env->DeleteLocalRef(keyJava); - env->DeleteLocalRef(valueJava); + for (const auto &[key, value] : map) { + auto key_java = lsplant::JNI_NewStringUTF(env, key); + auto value_java = lsplant::JNI_NewStringUTF(env, value); + lsplant::JNI_CallObjectMethod(env, hash_map, put, key_java, value_java); } - jobject hashMapGlobal = env->NewGlobalRef(hashMap); - env->DeleteLocalRef(hashMap); - env->DeleteLocalRef(mapClass); - - return hashMapGlobal; + return lsplant::JNI_NewGlobalRef(env, hash_map); } extern "C" JNIEXPORT jobject JNICALL diff --git a/native/include/core/context.h b/native/include/core/context.h index 230a05460..117770f68 100644 --- a/native/include/core/context.h +++ b/native/include/core/context.h @@ -145,27 +145,46 @@ class Context { * * A utility for internal communication between the native and Java layers. * + * A Java method that throws does not unwind into C++: the exception is left *pending* on this + * thread, and until it is cleared almost every JNI function is illegal to call. With CheckJNI + * on, the next one aborts the process; without it, the exception stays pending until control + * returns to Java and is then thrown somewhere with nothing to do with us — typically inside + * the starting application, where no stack frame points back here. So the exception is reported + * where it happened, with the Java stack that only exists at this moment, and whether the call + * arrived is something the caller can act on. + * * @tparam Args Argument types for the method call. * @param env The JNI environment. * @param method_name The name of the static method. * @param method_sig The JNI signature of the method. * @param args The arguments to pass to the method. + * @return Whether the method was found and returned without throwing. */ template - void FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig, + bool FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig, Args &&...args) const { if (!entry_class_) { LOGE("Cannot call method '{}', entry class is null", method_name.data()); - return; + return false; } jmethodID mid = lsplant::JNI_GetStaticMethodID(env, entry_class_, method_name, method_sig); - if (mid) { - env->CallStaticVoidMethod(entry_class_, mid, - lsplant::UnwrapScope(std::forward(args))...); - } else { + if (!mid) { LOGE("Static method '{}' with signature '{}' not found", method_name.data(), method_sig.data()); + return false; + } + env->CallStaticVoidMethod(entry_class_, mid, + lsplant::UnwrapScope(std::forward(args))...); + // ClearException, not ExceptionDescribe: the latter writes the trace to stderr, and a + // process forked from the zygote has nowhere for stderr to go, so the trace is simply lost. + // This asks Java to render it and logs the result under our own tag, which is the only + // place the stack still exists to be read. + if (auto trace = lsplant::ClearException(env)) { + LOGE("Java entry '{}' threw:\n{}", method_name.data(), + lsplant::JUTFString(env, trace.get()).get()); + return false; } + return true; } // --- Virtual methods for platform-specific implementations --- diff --git a/native/include/jni/jni_bridge.h b/native/include/jni/jni_bridge.h index a999eb980..fb9f78973 100644 --- a/native/include/jni/jni_bridge.h +++ b/native/include/jni/jni_bridge.h @@ -71,7 +71,9 @@ inline bool RegisterNativeMethodsInternal(JNIEnv *env, std::string_view class_na LOGF("JNI class not found: {}", class_name.data()); return false; } - return env->RegisterNatives(clazz.get(), methods, method_count) == JNI_OK; + // Wrapped: a failed registration throws NoSuchMethodError, and returning false while that + // exception is still pending would hand the next JNI call undefined behaviour. + return lsplant::JNI_RegisterNatives(env, clazz, methods, method_count) == JNI_OK; } // A helper cast for the native method function pointers. diff --git a/native/src/jni/resources_hook.cpp b/native/src/jni/resources_hook.cpp index a3e6b3f5c..da073b56f 100644 --- a/native/src/jni/resources_hook.cpp +++ b/native/src/jni/resources_hook.cpp @@ -123,17 +123,20 @@ VECTOR_DEF_NATIVE_METHOD(jboolean, ResourcesHook, initXResourcesNative) { std::string x_resources_jni_name = "L" + x_resources_class_name + ";"; std::replace(x_resources_jni_name.begin(), x_resources_jni_name.end(), '.', '/'); - methodXResourcesTranslateResId = env->GetStaticMethodID( - classXResources, "translateResId", - fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name).c_str()); + // Wrapped, like the lookup below: a missing method throws NoSuchMethodError, and the raw form + // returned JNI_FALSE to Java with that exception still pending, so the caller saw a throw where + // it had asked for a boolean. + methodXResourcesTranslateResId = lsplant::JNI_GetStaticMethodID( + env, classXResources, "translateResId", + fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name)); if (!methodXResourcesTranslateResId) { LOGE("Failed to find method: XResources.translateResId"); return JNI_FALSE; } - methodXResourcesTranslateAttrId = env->GetStaticMethodID( - classXResources, "translateAttrId", - fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name).c_str()); + methodXResourcesTranslateAttrId = lsplant::JNI_GetStaticMethodID( + env, classXResources, "translateAttrId", + fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name)); if (!methodXResourcesTranslateAttrId) { LOGE("Failed to find method: XResources.translateAttrId"); return JNI_FALSE; @@ -173,9 +176,10 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject // Cache the class and constructor for InMemoryDexClassLoader. static auto in_memory_classloader = - (jclass)env->NewGlobalRef(env->FindClass("dalvik/system/InMemoryDexClassLoader")); - static jmethodID initMid = env->GetMethodID(in_memory_classloader, "", - "(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V"); + lsplant::JNI_NewGlobalRef(env, lsplant::JNI_FindClass(env, "dalvik/system/InMemoryDexClassLoader")); + static jmethodID initMid = lsplant::JNI_GetMethodID( + env, in_memory_classloader, "", "(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V"); + if (!in_memory_classloader || !initMid) return nullptr; DexBuilder dex_file; @@ -195,10 +199,14 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject slicer::MemView image{dex_file.CreateImage()}; // Wrap the memory buffer in a Java ByteBuffer. - auto dex_buffer = env->NewDirectByteBuffer(const_cast(image.ptr()), image.size()); - - // Create and return a new InMemoryDexClassLoader instance. - return env->NewObject(in_memory_classloader, initMid, dex_buffer, parent); + auto dex_buffer = lsplant::JNI_NewDirectByteBuffer(env, const_cast(image.ptr()), + image.size()); + if (!dex_buffer) return nullptr; + + // Create and return a new InMemoryDexClassLoader instance. Released from its scope because it + // is handed straight back to Java. + return lsplant::JNI_NewObject(env, in_memory_classloader, initMid, dex_buffer, parent) + .release(); } /** diff --git a/zygisk/src/main/cpp/module.cpp b/zygisk/src/main/cpp/module.cpp index 0dbb21472..84ff88312 100644 --- a/zygisk/src/main/cpp/module.cpp +++ b/zygisk/src/main/cpp/module.cpp @@ -361,12 +361,19 @@ void VectorModule::postAppSpecialize(const zygisk::AppSpecializeArgs *args) { this->SetupEntryClass(env_); // Hand off control to the Java side of the framework. - this->FindAndCall( + bool entered = this->FindAndCall( env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V", JNI_FALSE, JNI_FALSE, args->nice_name, args->app_data_dir, binder.get(), is_manager_app_); - LOGV("Injected Vector framework into '{}'.", nice_name_str.get()); - SetAllowUnload(false); // We are injected, PREVENT module unloading. + if (entered) { + LOGV("Injected Vector framework into '{}'.", nice_name_str.get()); + } else { + LOGE("Framework entry failed in '{}'; this process runs without Xposed.", + nice_name_str.get()); + } + // Unconditionally: the ART and JNI hooks were installed before the entry ran, and their + // trampolines point into this library. Letting it be unloaded now would leave them dangling. + SetAllowUnload(false); } void VectorModule::preServerSpecialize(zygisk::ServerSpecializeArgs *args) { @@ -448,13 +455,18 @@ void VectorModule::postServerSpecialize(const zygisk::ServerSpecializeArgs *args this->SetupEntryClass(env_); auto system_name = lsplant::ScopedLocalRef(env_, env_->NewStringUTF("system")); - this->FindAndCall(env_, "forkCommon", - "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V", JNI_TRUE, - is_late_inject, system_name.get(), nullptr, manager_binder.get(), - is_manager_app_); + bool entered = this->FindAndCall( + env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V", + JNI_TRUE, is_late_inject, system_name.get(), nullptr, manager_binder.get(), + is_manager_app_); - LOGI("Injected Vector framework into system_server."); - SetAllowUnload(false); // We are injected, PREVENT module unloading. + if (entered) { + LOGI("Injected Vector framework into system_server."); + } else { + LOGE("Framework entry failed in system_server; it runs without Xposed."); + } + // See postAppSpecialize: the hooks outlive a failed entry, so the library must stay. + SetAllowUnload(false); } void VectorModule::SetAllowUnload(bool unload) {