Check object types when native code returns.
Object references returned by native code were not being type checked,
so it was possible to (for example) return a byte[] when an InetAddress
was expected. This sort of thing can lead to extremely strange behavior.
Now, when "check JNI" is enabled, we insert another layer in the JNI call
sequence that verifies the type of the returned object. This makes calls
to native methods returning an object a bit slower on engineering builds,
but the added type safety is helpful.
I found two failures with this. One fairly harmless one was fixed in
master 1594, the other is filed as internal bug 1851257. The latter does
not seem to cause problems for anything other than the socks5 app that
uncovered it (but it does render SOCKS less than useful).
diff --git a/vm/CheckJni.c b/vm/CheckJni.c
index 15398d2..cece49a 100644
--- a/vm/CheckJni.c
+++ b/vm/CheckJni.c
@@ -35,6 +35,116 @@
#include <zlib.h>
+static void abortMaybe(void); // fwd
+
+
+/*
+ * ===========================================================================
+ * JNI call bridge wrapper
+ * ===========================================================================
+ */
+
+/*
+ * Check the result of a native method call that returns an object reference.
+ *
+ * The primary goal here is to verify that native code is returning the
+ * correct type of object. If it's declared to return a String but actually
+ * returns a byte array, things will fail in strange ways later on.
+ *
+ * This can be a fairly expensive operation, since we have to look up the
+ * return type class by name in method->clazz' class loader. We take a
+ * shortcut here and allow the call to succeed if the descriptor strings
+ * match. This will allow some false-positives when a class is redefined
+ * by a class loader, but that's rare enough that it doesn't seem worth
+ * testing for.
+ */
+static void checkCallCommon(const u4* args, JValue* pResult,
+ const Method* method, Thread* self)
+{
+ assert(pResult->l != NULL);
+ ClassObject* objClazz = ((Object*)pResult->l)->clazz;
+
+ /*
+ * Make sure that pResult->l is an instance of the type this
+ * method was expected to return.
+ */
+ const char* declType = dexProtoGetReturnType(&method->prototype);
+ const char* objType = objClazz->descriptor;
+ if (strcmp(declType, objType) == 0) {
+ /* names match; ignore class loader issues and allow it */
+ LOGV("Check %s.%s: %s io %s (FAST-OK)\n",
+ method->clazz->descriptor, method->name, objType, declType);
+ } else {
+ /*
+ * Names didn't match. We need to resolve declType in the context
+ * of method->clazz->classLoader, and compare the class objects
+ * for equality.
+ *
+ * Since we're returning an instance of declType, it's safe to
+ * assume that it has been loaded and initialized (or, for the case
+ * of an array, generated), so we can just look for it in the
+ * loaded-classes list.
+ */
+ ClassObject* declClazz;
+
+ declClazz = dvmLookupClass(declType, method->clazz->classLoader, false);
+ if (declClazz == NULL) {
+ LOGW("JNI WARNING: method declared to return '%s' returned '%s'\n",
+ declType, objType);
+ LOGW(" failed in %s.%s ('%s' not found)\n",
+ method->clazz->descriptor, method->name, declType);
+ abortMaybe();
+ return;
+ }
+ if (!dvmInstanceof(objClazz, declClazz)) {
+ LOGW("JNI WARNING: method declared to return '%s' returned '%s'\n",
+ declType, objType);
+ LOGW(" failed in %s.%s\n",
+ method->clazz->descriptor, method->name);
+ abortMaybe();
+ return;
+ } else {
+ LOGV("Check %s.%s: %s io %s (SLOW-OK)\n",
+ method->clazz->descriptor, method->name, objType, declType);
+ }
+ }
+}
+
+/*
+ * Check a call into native code.
+ */
+void dvmCheckCallJNIMethod(const u4* args, JValue* pResult,
+ const Method* method, Thread* self)
+{
+ dvmCallJNIMethod(args, pResult, method, self);
+ if (method->shorty[0] == 'L' && !dvmCheckException(self) &&
+ pResult->l != NULL)
+ {
+ checkCallCommon(args, pResult, method, self);
+ }
+}
+
+/*
+ * Check a synchronized call into native code.
+ */
+void dvmCheckCallSynchronizedJNIMethod(const u4* args, JValue* pResult,
+ const Method* method, Thread* self)
+{
+ dvmCallSynchronizedJNIMethod(args, pResult, method, self);
+ if (method->shorty[0] == 'L' && !dvmCheckException(self) &&
+ pResult->l != NULL)
+ {
+ checkCallCommon(args, pResult, method, self);
+ }
+}
+
+
+/*
+ * ===========================================================================
+ * JNI function helpers
+ * ===========================================================================
+ */
+
#define JNI_ENTER() dvmChangeStatus(NULL, THREAD_RUNNING)
#define JNI_EXIT() dvmChangeStatus(NULL, THREAD_NATIVE)
@@ -143,7 +253,7 @@
/*
* Abort if we are configured to bail out on JNI warnings.
*/
-static inline void abortMaybe()
+static void abortMaybe(void)
{
JavaVMExt* vm = (JavaVMExt*) gDvm.vmList;
if (vm->warnError) {
diff --git a/vm/Jni.c b/vm/Jni.c
index cb1821f..39b773a 100644
--- a/vm/Jni.c
+++ b/vm/Jni.c
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* Dalvik implementation of JNI interfaces.
*/
@@ -859,14 +860,7 @@
/* keep going, I guess */
}
- /*
- * Point "nativeFunc" at the JNI bridge, and overload "insns" to
- * point at the actual function.
- */
- if (dvmIsSynchronizedMethod(method))
- dvmSetNativeFunc(method, dvmCallSynchronizedJNIMethod, fnPtr);
- else
- dvmSetNativeFunc(method, dvmCallJNIMethod, fnPtr);
+ dvmUseJNIBridge(method, fnPtr);
LOGV("JNI-registered %s.%s %s\n", clazz->descriptor, methodName,
signature);
@@ -877,6 +871,34 @@
}
/*
+ * Returns "true" if CheckJNI is enabled in the VM.
+ */
+static bool dvmIsCheckJNIEnabled(void)
+{
+ JavaVMExt* vm = (JavaVMExt*) gDvm.vmList;
+ return vm->useChecked;
+}
+
+/*
+ * Point "method->nativeFunc" at the JNI bridge, and overload "method->insns"
+ * to point at the actual function.
+ */
+void dvmUseJNIBridge(Method* method, void* func)
+{
+ if (dvmIsCheckJNIEnabled()) {
+ if (dvmIsSynchronizedMethod(method))
+ dvmSetNativeFunc(method, dvmCheckCallSynchronizedJNIMethod, func);
+ else
+ dvmSetNativeFunc(method, dvmCheckCallJNIMethod, func);
+ } else {
+ if (dvmIsSynchronizedMethod(method))
+ dvmSetNativeFunc(method, dvmCallSynchronizedJNIMethod, func);
+ else
+ dvmSetNativeFunc(method, dvmCallJNIMethod, func);
+ }
+}
+
+/*
* Get the method currently being executed by examining the interp stack.
*/
const Method* dvmGetCurrentJNIMethod(void)
@@ -3375,6 +3397,10 @@
/*
* Enable "checked JNI" after the VM has partially started. This must
* only be called in "zygote" mode, when we have one thread running.
+ *
+ * This doesn't attempt to rewrite the JNI call bridge associated with
+ * native methods, so we won't get those checks for any methods that have
+ * already been resolved.
*/
void dvmLateEnableCheckedJni(void)
{
diff --git a/vm/JniInternal.h b/vm/JniInternal.h
index a890934..e60b088 100644
--- a/vm/JniInternal.h
+++ b/vm/JniInternal.h
@@ -113,11 +113,23 @@
/*
* JNI call bridges. Not usually called directly.
+ *
+ * The "Check" versions are used when CheckJNI is enabled.
*/
void dvmCallJNIMethod(const u4* args, JValue* pResult, const Method* method,
Thread* self);
void dvmCallSynchronizedJNIMethod(const u4* args, JValue* pResult,
const Method* method, Thread* self);
+void dvmCheckCallJNIMethod(const u4* args, JValue* pResult,
+ const Method* method, Thread* self);
+void dvmCheckCallSynchronizedJNIMethod(const u4* args, JValue* pResult,
+ const Method* method, Thread* self);
+
+/*
+ * Configure "method" to use the JNI bridge to call "func".
+ */
+void dvmUseJNIBridge(Method* method, void* func);
+
/*
* Enable the "checked" versions.
diff --git a/vm/Native.c b/vm/Native.c
index 4fb9795..93bbc07 100644
--- a/vm/Native.c
+++ b/vm/Native.c
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* Native method resolution.
*
@@ -61,6 +62,10 @@
* Initializes method's class if necessary.
*
* An exception is thrown on resolution failure.
+ *
+ * (This should not be taking "const Method*", because it modifies the
+ * structure, but the declaration needs to match the DalvikBridgeFunc
+ * type definition.)
*/
void dvmResolveNativeMethod(const u4* args, JValue* pResult,
const Method* method, Thread* self)
@@ -107,10 +112,8 @@
/* now scan any DLLs we have loaded for JNI signatures */
func = lookupSharedLibMethod(method);
if (func != NULL) {
- if (dvmIsSynchronizedMethod(method))
- dvmSetNativeFunc(method, dvmCallSynchronizedJNIMethod, func);
- else
- dvmSetNativeFunc(method, dvmCallJNIMethod, func);
+ /* found it, point it at the JNI bridge and then call it */
+ dvmUseJNIBridge((Method*) method, func);
dvmCallJNIMethod(args, pResult, method, self);
return;
}
