From 0cfcafaea98fa27b5b3e8bdb0a57fb652fba8895 Mon Sep 17 00:00:00 2001 From: emanuele-f Date: Sat, 13 Aug 2022 14:26:56 +0200 Subject: [PATCH] Fix JNI local references leaks --- .../model/ConnectionDescriptor.java | 1 - app/src/main/jni/common/jni_utils.c | 25 ++++++++++++- app/src/main/jni/common/jni_utils.h | 2 + app/src/main/jni/core/jni_impl.c | 37 ++++++++++++++----- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/emanuelef/remote_capture/model/ConnectionDescriptor.java b/app/src/main/java/com/emanuelef/remote_capture/model/ConnectionDescriptor.java index 46e060dc..fe5736d3 100644 --- a/app/src/main/java/com/emanuelef/remote_capture/model/ConnectionDescriptor.java +++ b/app/src/main/java/com/emanuelef/remote_capture/model/ConnectionDescriptor.java @@ -134,7 +134,6 @@ public class ConnectionDescriptor { src_port = _src_port; dst_port = _dst_port; local_port = _local_port; - Log.d("ConnectionDescriptor", "local_port=" + local_port); uid = _uid; ifidx = _ifidx; first_seen = last_seen = when; diff --git a/app/src/main/jni/common/jni_utils.c b/app/src/main/jni/common/jni_utils.c index a498b72d..d83effc7 100644 --- a/app/src/main/jni/common/jni_utils.c +++ b/app/src/main/jni/common/jni_utils.c @@ -19,7 +19,7 @@ #ifdef ANDROID -#include +#include "jni_utils.h" #include "common/utils.h" /* ******************************************************* */ @@ -37,6 +37,16 @@ int jniCheckException(JNIEnv *env) { /* ******************************************************* */ +// Dumps JNI reference tables to logcat to detect possible reference leaks +void jniDumpReferences(JNIEnv *env) { + jclass vm_class = jniFindClass(env, "dalvik/system/VMDebug"); + jmethodID dump_mid = jniGetStaticMethodID(env, vm_class, "dumpReferenceTables", "()V" ); + (*env)->CallStaticVoidMethod(env, vm_class, dump_mid); + (*env)->DeleteLocalRef(env, vm_class); +} + +/* ******************************************************* */ + jclass jniFindClass(JNIEnv *env, const char *name) { jclass cls = (*env)->FindClass(env, name); if (cls == NULL) @@ -60,6 +70,18 @@ jmethodID jniGetMethodID(JNIEnv *env, jclass cls, const char *name, const char * /* ******************************************************* */ +jmethodID jniGetStaticMethodID(JNIEnv *env, jclass cls, const char *name, const char *signature) { + jmethodID method = (*env)->GetStaticMethodID(env, cls, name, signature); + if (method == NULL) { + log_e("Static method %s %s not found", name, signature); + jniCheckException(env); + } + + return method; +} + +/* ******************************************************* */ + jfieldID jniFieldID(JNIEnv *env, jclass cls, const char *name, const char *type) { jfieldID field = (*env)->GetFieldID(env, cls, name, type); if(field == NULL) { @@ -93,6 +115,7 @@ jobject jniEnumVal(JNIEnv *env, const char *class_name, const char *enum_key) { jniCheckException(env); } + (*env)->DeleteLocalRef(env, cls); return val; } diff --git a/app/src/main/jni/common/jni_utils.h b/app/src/main/jni/common/jni_utils.h index 0514c311..214fe93a 100644 --- a/app/src/main/jni/common/jni_utils.h +++ b/app/src/main/jni/common/jni_utils.h @@ -26,9 +26,11 @@ jclass jniFindClass(JNIEnv *env, const char *name); jmethodID jniGetMethodID(JNIEnv *env, jclass cls, const char *name, const char *signature); +jmethodID jniGetStaticMethodID(JNIEnv *env, jclass cls, const char *name, const char *signature); jfieldID jniFieldID(JNIEnv *env, jclass cls, const char *name, const char *type); jobject jniEnumVal(JNIEnv *env, const char *class_name, const char *enum_key); int jniCheckException(JNIEnv *env); +void jniDumpReferences(JNIEnv *env); #else // if ANDROID diff --git a/app/src/main/jni/core/jni_impl.c b/app/src/main/jni/core/jni_impl.c index 05f61ea9..42e72735 100644 --- a/app/src/main/jni/core/jni_impl.c +++ b/app/src/main/jni/core/jni_impl.c @@ -315,7 +315,7 @@ cleanup: /* ******************************************************* */ -// Load information about the blacklists to use (pd->malware_detection.bls_info) +// Load information about the blacklists to use (into pd->malware_detection.bls_info) static int loadBlacklistsInfo(pcapdroid_t *pd) { int rv = 0; JNIEnv *env = pd->env; @@ -353,6 +353,7 @@ static int loadBlacklistsInfo(pcapdroid_t *pd) { (*pd->env)->DeleteLocalRef(pd->env, bl_type); //log_d("[+] Blacklist: %s (%s)", blinfo->fname, (blinfo->type == IP_BLACKLIST) ? "IP" : "domain"); + (*pd->env)->DeleteLocalRef(pd->env, bl_descr); } } @@ -380,19 +381,21 @@ static void notifyBlacklistsLoaded(pcapdroid_t *pd, bl_status_arr_t *status_arr) jobject stats = (*env)->NewObject(env, cls.blacklist_status, mids.blacklistStatusInit, fname, st->num_rules); - if((stats == NULL) || jniCheckException(env)) { - (*env)->DeleteLocalRef(env, fname); + (*env)->DeleteLocalRef(env, fname); + + if((stats == NULL) || jniCheckException(env)) break; - } (*env)->SetObjectArrayElement(env, status_obj, i, stats); + (*env)->DeleteLocalRef(env, stats); + if(jniCheckException(env)) { - (*env)->DeleteLocalRef(env, stats); break; } } - (*pd->env)->CallVoidMethod(pd->env, pd->capture_service, mids.notifyBlacklistsLoaded, status_obj); + (*env)->CallVoidMethod(env, pd->capture_service, mids.notifyBlacklistsLoaded, status_obj); + (*env)->DeleteLocalRef(env, status_obj); } /* ******************************************************* */ @@ -402,7 +405,7 @@ static bool dumpPayloadChunk(struct pcapdroid *pd, const pkt_context_t *pctx, in bool rv = false; if(pctx->data->payload_chunks == NULL) { - pctx->data->payload_chunks = (*pd->env)->NewObject(pd->env, cls.arraylist, mids.arraylistNew); + pctx->data->payload_chunks = (*env)->NewObject(env, cls.arraylist, mids.arraylistNew); if((pctx->data->payload_chunks == NULL) || jniCheckException(env)) return false; } @@ -413,10 +416,10 @@ static bool dumpPayloadChunk(struct pcapdroid *pd, const pkt_context_t *pctx, in jobject chunk_type = (pctx->data->l7proto == NDPI_PROTOCOL_HTTP) ? enums.chunktype_http : enums.chunktype_raw; - jobject chunk = (*pd->env)->NewObject(pd->env, cls.payload_chunk, mids.payloadChunkInit, barray, chunk_type, pctx->is_tx, pctx->ms); + jobject chunk = (*env)->NewObject(env, cls.payload_chunk, mids.payloadChunkInit, barray, chunk_type, pctx->is_tx, pctx->ms); if(chunk && !jniCheckException(env)) { (*env)->SetByteArrayRegion(env, barray, 0, dump_size, (jbyte*)pctx->pkt->l7); - rv = (*pd->env)->CallBooleanMethod(pd->env, pctx->data->payload_chunks, mids.arraylistAdd, chunk); + rv = (*env)->CallBooleanMethod(env, pctx->data->payload_chunks, mids.arraylistAdd, chunk); } //log_d("Dump chunk [size=%d]: %d", rv, dump_size); @@ -441,6 +444,7 @@ static void getLibprogPath(pcapdroid_t *pd, const char *prog_name, char *buf, in } jstring obj = (*env)->CallObjectMethod(env, pd->capture_service, mids.getLibprogPath, prog_str); + (*env)->DeleteLocalRef(env, prog_str); if(!jniCheckException(env)) { const char *value = (*env)->GetStringUTFChars(env, obj, 0); @@ -603,6 +607,21 @@ Java_com_emanuelef_remote_1capture_CaptureService_runPacketLoop(JNIEnv *env, jcl global_pd = NULL; logcallback = NULL; +#if 0 + // free JNI local objects to ease references leak detection + for(int i=0; iDeleteLocalRef(env, cur); + } + for(int i=0; iDeleteLocalRef(env, cur); + } + + // at this point the local reference table should only contain 2 entries (VMDebug + Thread) + jniDumpReferences(env); +#endif + #ifdef PCAPDROID_TRACK_ALLOCS log_i(get_allocs_summary()); #endif