Skip to content

Commit 4e6cc3e

Browse files
committed
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.
1 parent b09a978 commit 4e6cc3e

7 files changed

Lines changed: 106 additions & 49 deletions

File tree

daemon/src/main/jni/dex2oat.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,12 @@ extern "C" JNIEXPORT jboolean JNICALL
106106
Java_org_matrix_vector_daemon_env_Dex2OatServer_setSockCreateContext(JNIEnv *env, jclass,
107107
jstring contextStr) {
108108
const char *context = contextStr ? env->GetStringUTFChars(contextStr, nullptr) : nullptr;
109+
if (contextStr && !context) {
110+
// Only OutOfMemoryError puts us here, and it is pending: returning into Java with it still
111+
// set would surface it at the next unrelated call.
112+
env->ExceptionClear();
113+
return false;
114+
}
109115
int ret = setsockcreatecon_raw(context);
110116
if (context) env->ReleaseStringUTFChars(contextStr, context);
111117
return ret == 0;

daemon/src/main/jni/logcat.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#include "logcat.h"
22

3+
#include <utils/jni_helper.hpp>
4+
5+
#include "logging.h"
6+
37
#include <android/log.h>
48
#include <jni.h>
59
#include <sys/system_properties.h>
@@ -241,8 +245,14 @@ void Logcat::Run() {
241245

242246
extern "C" JNIEXPORT void JNICALL
243247
Java_org_matrix_vector_daemon_env_LogcatMonitor_runLogcat(JNIEnv* env, jobject thiz) {
244-
jclass clazz = env->GetObjectClass(thiz);
245-
jmethodID method = env->GetMethodID(clazz, "refreshFd", "(Z)I");
248+
auto clazz = lsplant::JNI_GetObjectClass(env, thiz);
249+
auto method = lsplant::JNI_GetMethodID(env, clazz, "refreshFd", "(Z)I");
250+
if (!method) {
251+
// The wrapper has already logged and cleared the NoSuchMethodError. Running with a null
252+
// method id would abort inside the first refresh instead of saying why.
253+
LOGE("LogcatMonitor.refreshFd is missing; not starting the log reader");
254+
return;
255+
}
246256
Logcat daemon(env, thiz, method);
247257
daemon.Run();
248258
}

daemon/src/main/jni/obfuscation.cpp

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -119,30 +119,30 @@ static void ensureInitialized(JNIEnv *env) {
119119
});
120120
}
121121

122+
// Through the lsplant wrappers rather than raw JNI: each one clears a pending exception and logs
123+
// the Java stack behind it, which is what a failed lookup here would otherwise cost. Returning a
124+
// null jclass while leaving NoClassDefFoundError pending -- as the raw form did -- hands the next
125+
// JNI call undefined behaviour. They also return scoped references, so the local refs this loop
126+
// used to leak per entry are released on the spot.
122127
static jobject stringMapToJavaHashMap(JNIEnv *env, const std::map<std::string, std::string> &map) {
123-
jclass mapClass = env->FindClass("java/util/HashMap");
124-
if (mapClass == nullptr) return nullptr;
128+
auto map_class = lsplant::JNI_FindClass(env, "java/util/HashMap");
129+
if (!map_class) return nullptr;
125130

126-
jmethodID init = env->GetMethodID(mapClass, "<init>", "()V");
127-
jobject hashMap = env->NewObject(mapClass, init);
128-
jmethodID put = env->GetMethodID(mapClass, "put",
129-
"(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
131+
auto init = lsplant::JNI_GetMethodID(env, map_class, "<init>", "()V");
132+
auto put = lsplant::JNI_GetMethodID(env, map_class, "put",
133+
"(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;");
134+
if (!init || !put) return nullptr;
130135

131-
for (const auto &[key, value] : map) {
132-
jstring keyJava = env->NewStringUTF(key.c_str());
133-
jstring valueJava = env->NewStringUTF(value.c_str());
134-
135-
env->CallObjectMethod(hashMap, put, keyJava, valueJava);
136+
auto hash_map = lsplant::JNI_NewObject(env, map_class, init);
137+
if (!hash_map) return nullptr;
136138

137-
env->DeleteLocalRef(keyJava);
138-
env->DeleteLocalRef(valueJava);
139+
for (const auto &[key, value] : map) {
140+
auto key_java = lsplant::JNI_NewStringUTF(env, key);
141+
auto value_java = lsplant::JNI_NewStringUTF(env, value);
142+
lsplant::JNI_CallObjectMethod(env, hash_map, put, key_java, value_java);
139143
}
140144

141-
jobject hashMapGlobal = env->NewGlobalRef(hashMap);
142-
env->DeleteLocalRef(hashMap);
143-
env->DeleteLocalRef(mapClass);
144-
145-
return hashMapGlobal;
145+
return lsplant::JNI_NewGlobalRef(env, hash_map);
146146
}
147147

148148
extern "C" JNIEXPORT jobject JNICALL

native/include/core/context.h

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,27 +145,46 @@ class Context {
145145
*
146146
* A utility for internal communication between the native and Java layers.
147147
*
148+
* A Java method that throws does not unwind into C++: the exception is left *pending* on this
149+
* thread, and until it is cleared almost every JNI function is illegal to call. With CheckJNI
150+
* on, the next one aborts the process; without it, the exception stays pending until control
151+
* returns to Java and is then thrown somewhere with nothing to do with us — typically inside
152+
* the starting application, where no stack frame points back here. So the exception is reported
153+
* where it happened, with the Java stack that only exists at this moment, and whether the call
154+
* arrived is something the caller can act on.
155+
*
148156
* @tparam Args Argument types for the method call.
149157
* @param env The JNI environment.
150158
* @param method_name The name of the static method.
151159
* @param method_sig The JNI signature of the method.
152160
* @param args The arguments to pass to the method.
161+
* @return Whether the method was found and returned without throwing.
153162
*/
154163
template <typename... Args>
155-
void FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig,
164+
bool FindAndCall(JNIEnv *env, std::string_view method_name, std::string_view method_sig,
156165
Args &&...args) const {
157166
if (!entry_class_) {
158167
LOGE("Cannot call method '{}', entry class is null", method_name.data());
159-
return;
168+
return false;
160169
}
161170
jmethodID mid = lsplant::JNI_GetStaticMethodID(env, entry_class_, method_name, method_sig);
162-
if (mid) {
163-
env->CallStaticVoidMethod(entry_class_, mid,
164-
lsplant::UnwrapScope(std::forward<Args>(args))...);
165-
} else {
171+
if (!mid) {
166172
LOGE("Static method '{}' with signature '{}' not found", method_name.data(),
167173
method_sig.data());
174+
return false;
175+
}
176+
env->CallStaticVoidMethod(entry_class_, mid,
177+
lsplant::UnwrapScope(std::forward<Args>(args))...);
178+
// ClearException, not ExceptionDescribe: the latter writes the trace to stderr, and a
179+
// process forked from the zygote has nowhere for stderr to go, so the trace is simply lost.
180+
// This asks Java to render it and logs the result under our own tag, which is the only
181+
// place the stack still exists to be read.
182+
if (auto trace = lsplant::ClearException(env)) {
183+
LOGE("Java entry '{}' threw:\n{}", method_name.data(),
184+
lsplant::JUTFString(env, trace.get()).get());
185+
return false;
168186
}
187+
return true;
169188
}
170189

171190
// --- Virtual methods for platform-specific implementations ---

native/include/jni/jni_bridge.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ inline bool RegisterNativeMethodsInternal(JNIEnv *env, std::string_view class_na
7171
LOGF("JNI class not found: {}", class_name.data());
7272
return false;
7373
}
74-
return env->RegisterNatives(clazz.get(), methods, method_count) == JNI_OK;
74+
// Wrapped: a failed registration throws NoSuchMethodError, and returning false while that
75+
// exception is still pending would hand the next JNI call undefined behaviour.
76+
return lsplant::JNI_RegisterNatives(env, clazz, methods, method_count) == JNI_OK;
7577
}
7678

7779
// A helper cast for the native method function pointers.

native/src/jni/resources_hook.cpp

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,17 +123,20 @@ VECTOR_DEF_NATIVE_METHOD(jboolean, ResourcesHook, initXResourcesNative) {
123123
std::string x_resources_jni_name = "L" + x_resources_class_name + ";";
124124
std::replace(x_resources_jni_name.begin(), x_resources_jni_name.end(), '.', '/');
125125

126-
methodXResourcesTranslateResId = env->GetStaticMethodID(
127-
classXResources, "translateResId",
128-
fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name).c_str());
126+
// Wrapped, like the lookup below: a missing method throws NoSuchMethodError, and the raw form
127+
// returned JNI_FALSE to Java with that exception still pending, so the caller saw a throw where
128+
// it had asked for a boolean.
129+
methodXResourcesTranslateResId = lsplant::JNI_GetStaticMethodID(
130+
env, classXResources, "translateResId",
131+
fmt::format("(I{}Landroid/content/res/Resources;)I", x_resources_jni_name));
129132
if (!methodXResourcesTranslateResId) {
130133
LOGE("Failed to find method: XResources.translateResId");
131134
return JNI_FALSE;
132135
}
133136

134-
methodXResourcesTranslateAttrId = env->GetStaticMethodID(
135-
classXResources, "translateAttrId",
136-
fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name).c_str());
137+
methodXResourcesTranslateAttrId = lsplant::JNI_GetStaticMethodID(
138+
env, classXResources, "translateAttrId",
139+
fmt::format("(Ljava/lang/String;{})I", x_resources_jni_name));
137140
if (!methodXResourcesTranslateAttrId) {
138141
LOGE("Failed to find method: XResources.translateAttrId");
139142
return JNI_FALSE;
@@ -173,9 +176,10 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject
173176

174177
// Cache the class and constructor for InMemoryDexClassLoader.
175178
static auto in_memory_classloader =
176-
(jclass)env->NewGlobalRef(env->FindClass("dalvik/system/InMemoryDexClassLoader"));
177-
static jmethodID initMid = env->GetMethodID(in_memory_classloader, "<init>",
178-
"(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V");
179+
lsplant::JNI_NewGlobalRef(env, lsplant::JNI_FindClass(env, "dalvik/system/InMemoryDexClassLoader"));
180+
static jmethodID initMid = lsplant::JNI_GetMethodID(
181+
env, in_memory_classloader, "<init>", "(Ljava/nio/ByteBuffer;Ljava/lang/ClassLoader;)V");
182+
if (!in_memory_classloader || !initMid) return nullptr;
179183

180184
DexBuilder dex_file;
181185

@@ -195,10 +199,14 @@ VECTOR_DEF_NATIVE_METHOD(jobject, ResourcesHook, buildDummyClassLoader, jobject
195199
slicer::MemView image{dex_file.CreateImage()};
196200

197201
// Wrap the memory buffer in a Java ByteBuffer.
198-
auto dex_buffer = env->NewDirectByteBuffer(const_cast<void *>(image.ptr()), image.size());
199-
200-
// Create and return a new InMemoryDexClassLoader instance.
201-
return env->NewObject(in_memory_classloader, initMid, dex_buffer, parent);
202+
auto dex_buffer = lsplant::JNI_NewDirectByteBuffer(env, const_cast<void *>(image.ptr()),
203+
image.size());
204+
if (!dex_buffer) return nullptr;
205+
206+
// Create and return a new InMemoryDexClassLoader instance. Released from its scope because it
207+
// is handed straight back to Java.
208+
return lsplant::JNI_NewObject(env, in_memory_classloader, initMid, dex_buffer, parent)
209+
.release();
202210
}
203211

204212
/**

zygisk/src/main/cpp/module.cpp

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -361,12 +361,19 @@ void VectorModule::postAppSpecialize(const zygisk::AppSpecializeArgs *args) {
361361
this->SetupEntryClass(env_);
362362

363363
// Hand off control to the Java side of the framework.
364-
this->FindAndCall(
364+
bool entered = this->FindAndCall(
365365
env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V",
366366
JNI_FALSE, JNI_FALSE, args->nice_name, args->app_data_dir, binder.get(), is_manager_app_);
367367

368-
LOGV("Injected Vector framework into '{}'.", nice_name_str.get());
369-
SetAllowUnload(false); // We are injected, PREVENT module unloading.
368+
if (entered) {
369+
LOGV("Injected Vector framework into '{}'.", nice_name_str.get());
370+
} else {
371+
LOGE("Framework entry failed in '{}'; this process runs without Xposed.",
372+
nice_name_str.get());
373+
}
374+
// Unconditionally: the ART and JNI hooks were installed before the entry ran, and their
375+
// trampolines point into this library. Letting it be unloaded now would leave them dangling.
376+
SetAllowUnload(false);
370377
}
371378

372379
void VectorModule::preServerSpecialize(zygisk::ServerSpecializeArgs *args) {
@@ -448,13 +455,18 @@ void VectorModule::postServerSpecialize(const zygisk::ServerSpecializeArgs *args
448455
this->SetupEntryClass(env_);
449456

450457
auto system_name = lsplant::ScopedLocalRef(env_, env_->NewStringUTF("system"));
451-
this->FindAndCall(env_, "forkCommon",
452-
"(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V", JNI_TRUE,
453-
is_late_inject, system_name.get(), nullptr, manager_binder.get(),
454-
is_manager_app_);
458+
bool entered = this->FindAndCall(
459+
env_, "forkCommon", "(ZZLjava/lang/String;Ljava/lang/String;Landroid/os/IBinder;)V",
460+
JNI_TRUE, is_late_inject, system_name.get(), nullptr, manager_binder.get(),
461+
is_manager_app_);
455462

456-
LOGI("Injected Vector framework into system_server.");
457-
SetAllowUnload(false); // We are injected, PREVENT module unloading.
463+
if (entered) {
464+
LOGI("Injected Vector framework into system_server.");
465+
} else {
466+
LOGE("Framework entry failed in system_server; it runs without Xposed.");
467+
}
468+
// See postAppSpecialize: the hooks outlive a failed entry, so the library must stay.
469+
SetAllowUnload(false);
458470
}
459471

460472
void VectorModule::SetAllowUnload(bool unload) {

0 commit comments

Comments
 (0)