Skip to content

Commit

Permalink
Android JNI code leaks local references in some cases (#175)
Browse files Browse the repository at this point in the history 
* Add Google LLC to AUTHORS.

* android: Explicitly delete all JNI local references, and cache JNI method IDs at initialization.

* android: Only return ARES_ENOTINITIALIZED on failures in initialization code.
  • Loading branch information
@acthompson-google-com
acthompson-google-com authored and Brad House committed Feb 3, 2018
1 parent e8ab6b5 commit 2ee16d7
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 57 deletions.
1 change: 1 addition & 0 deletions AUTHORS
View file
Expand Up @@ -37,6 +37,7 @@ Frederic Germain
Geert Uytterhoeven
George Neill
Gisle Vanem
Google LLC
Gregor Jasny
Guenter Knauf
Guilherme Balena Versiani
Expand Down
182 changes: 125 additions & 57 deletions ares_android.c
View file
Expand Up @@ -24,6 +24,19 @@
static JavaVM *android_jvm = NULL;
static jobject android_connectivity_manager = NULL;

/* ConnectivityManager.getActiveNetwork */
static jmethodID android_cm_active_net_mid = NULL;
/* ConnectivityManager.getLinkProperties */
static jmethodID android_cm_link_props_mid = NULL;
/* LinkProperties.getDnsServers */
static jmethodID android_lp_dns_servers_mid = NULL;
/* List.size */
static jmethodID android_list_size_mid = NULL;
/* List.get */
static jmethodID android_list_get_mid = NULL;
/* InetAddress.getHostAddress */
static jmethodID android_ia_host_addr_mid = NULL;

static jclass jni_get_class(JNIEnv *env, const char *path)
{
jclass cls = NULL;
Expand Down Expand Up @@ -70,9 +83,11 @@ int ares_library_init_android(jobject connectivity_manager)
JNIEnv *env = NULL;
int need_detatch = 0;
int res;
int ret = ARES_ENOTINITIALIZED;
jclass obj_cls = NULL;

if (android_jvm == NULL)
return ARES_ENOTINITIALIZED;
goto cleanup;

res = (*android_jvm)->GetEnv(android_jvm, (void **)&env, JNI_VERSION_1_6);
if (res == JNI_EDETACHED)
Expand All @@ -82,14 +97,91 @@ int ares_library_init_android(jobject connectivity_manager)
need_detatch = 1;
}
if (res != JNI_OK || env == NULL)
return ARES_ENOTINITIALIZED;
goto cleanup;

android_connectivity_manager =
(*env)->NewGlobalRef(env, connectivity_manager);
if (android_connectivity_manager == NULL)
goto cleanup;

/* Initialization has succeeded. Now attempt to cache the methods that will be
* called by ares_get_android_server_list. */
ret = ARES_SUCCESS;

/* ConnectivityManager in API 1. */
obj_cls = jni_get_class(env, "android/net/ConnectivityManager");
if (obj_cls == NULL)
goto cleanup;

/* ConnectivityManager.getActiveNetwork in API 23. */
android_cm_active_net_mid =
jni_get_method_id(env, obj_cls, "getActiveNetwork",
"()Landroid/net/Network;");
if (android_cm_active_net_mid == NULL)
goto cleanup;

/* ConnectivityManager.getLinkProperties in API 21. */
android_cm_link_props_mid =
jni_get_method_id(env, obj_cls, "getLinkProperties",
"(Landroid/net/Network;)Landroid/net/LinkProperties;");
if (android_cm_link_props_mid == NULL)
goto cleanup;

/* LinkProperties in API 21. */
(*env)->DeleteLocalRef(env, obj_cls);
obj_cls = jni_get_class(env, "android/net/LinkProperties");
if (obj_cls == NULL)
goto cleanup;

/* getDnsServers in API 21. */
android_lp_dns_servers_mid = jni_get_method_id(env, obj_cls, "getDnsServers",
"()Ljava/util/List;");
if (android_lp_dns_servers_mid == NULL)
goto cleanup;

(*env)->DeleteLocalRef(env, obj_cls);
obj_cls = jni_get_class(env, "java/util/List");
if (obj_cls == NULL)
goto cleanup;

android_list_size_mid = jni_get_method_id(env, obj_cls, "size", "()I");
if (android_list_size_mid == NULL)
goto cleanup;

android_list_get_mid = jni_get_method_id(env, obj_cls, "get",
"(I)Ljava/lang/Object;");
if (android_list_get_mid == NULL)
goto cleanup;

(*env)->DeleteLocalRef(env, obj_cls);
obj_cls = jni_get_class(env, "java/net/InetAddress");
if (obj_cls == NULL)
goto cleanup;

android_ia_host_addr_mid = jni_get_method_id(env, obj_cls, "getHostAddress",
"()Ljava/lang/String;");
if (android_ia_host_addr_mid == NULL)
goto cleanup;

(*env)->DeleteLocalRef(env, obj_cls);
goto done;

cleanup:
if (obj_cls != NULL)
(*env)->DeleteLocalRef(env, obj_cls);

android_connectivity_manager = (*env)->NewGlobalRef(env, connectivity_manager);
android_cm_active_net_mid = NULL;
android_cm_link_props_mid = NULL;
android_lp_dns_servers_mid = NULL;
android_list_size_mid = NULL;
android_list_get_mid = NULL;
android_ia_host_addr_mid = NULL;

done:
if (need_detatch)
(*android_jvm)->DetachCurrentThread(android_jvm);

return ARES_SUCCESS;
return ret;
}

int ares_library_android_initialized(void)
Expand Down Expand Up @@ -118,6 +210,13 @@ void ares_library_cleanup_android(void)
if (res != JNI_OK || env == NULL)
return;

android_cm_active_net_mid = NULL;
android_cm_link_props_mid = NULL;
android_lp_dns_servers_mid = NULL;
android_list_size_mid = NULL;
android_list_get_mid = NULL;
android_ia_host_addr_mid = NULL;

(*env)->DeleteGlobalRef(env, android_connectivity_manager);
android_connectivity_manager = NULL;

Expand All @@ -133,11 +232,7 @@ char **ares_get_android_server_list(size_t max_servers,
jobject link_properties = NULL;
jobject server_list = NULL;
jobject server = NULL;
jstring str;
jclass obj_cls;
jmethodID obj_mid;
jclass list_cls;
jmethodID list_mid;
jstring str = NULL;
jint nserv;
const char *ch_server_address;
int res;
Expand All @@ -151,6 +246,13 @@ char **ares_get_android_server_list(size_t max_servers,
return NULL;
}

if (android_cm_active_net_mid == NULL || android_cm_link_props_mid == NULL ||
android_lp_dns_servers_mid == NULL || android_list_size_mid == NULL ||
android_list_get_mid == NULL || android_ia_host_addr_mid == NULL)
{
return NULL;
}

res = (*android_jvm)->GetEnv(android_jvm, (void **)&env, JNI_VERSION_1_6);
if (res == JNI_EDETACHED)
{
Expand Down Expand Up @@ -178,84 +280,50 @@ char **ares_get_android_server_list(size_t max_servers,
String ha = ia.getHostAddress();
}
Note: The JNI ConnectivityManager object was previously initialized in
ares_library_init_android.
Note: The JNI ConnectivityManager object and all method IDs were previously
initialized in ares_library_init_android.
*/

/* ConnectivityManager in API 1. */
obj_cls = jni_get_class(env, "android/net/ConnectivityManager");
if (obj_cls == NULL)
goto done;
/* ConnectivityManager.getActiveNetwork in API 23. */
obj_mid = jni_get_method_id(env, obj_cls, "getActiveNetwork",
"()Landroid/net/Network;");
if (obj_mid == NULL)
goto done;
active_network = (*env)->CallObjectMethod(env, android_connectivity_manager,
obj_mid);
android_cm_active_net_mid);
if (active_network == NULL)
goto done;

/* ConnectivityManager.getLinkProperties in API 21. */
obj_mid = jni_get_method_id(env, obj_cls, "getLinkProperties",
"(Landroid/net/Network;)Landroid/net/LinkProperties;");
if (obj_mid == NULL)
goto done;
link_properties = (*env)->CallObjectMethod(env, android_connectivity_manager,
obj_mid, active_network);
link_properties =
(*env)->CallObjectMethod(env, android_connectivity_manager,
android_cm_link_props_mid, active_network);
if (link_properties == NULL)
goto done;

/* LinkProperties in API 21. */
obj_cls = jni_get_class(env, "android/net/LinkProperties");
if (obj_cls == NULL)
goto done;
/* getDnsServers in API 21. */
obj_mid = jni_get_method_id(env, obj_cls, "getDnsServers",
"()Ljava/util/List;");
if (obj_mid == NULL)
goto done;
server_list = (*env)->CallObjectMethod(env, link_properties, obj_mid);
server_list = (*env)->CallObjectMethod(env, link_properties,
android_lp_dns_servers_mid);
if (server_list == NULL)
goto done;

list_cls = jni_get_class(env, "java/util/List");
if (list_cls == NULL)
goto done;
list_mid = jni_get_method_id(env, list_cls, "size", "()I");
if (list_mid == NULL)
goto done;
nserv = (*env)->CallIntMethod(env, server_list, list_mid);
nserv = (*env)->CallIntMethod(env, server_list, android_list_size_mid);
if (nserv > (jint)max_servers)
nserv = (jint)max_servers;
if (nserv <= 0)
goto done;
*num_servers = (size_t)nserv;
list_mid = jni_get_method_id(env, list_cls, "get", "(I)Ljava/lang/Object;");
if (list_mid == NULL)
goto done;

obj_cls = jni_get_class(env, "java/net/InetAddress");
if (obj_cls == NULL)
goto done;
obj_mid = jni_get_method_id(env, obj_cls, "getHostAddress",
"()Ljava/lang/String;");
if (obj_mid == NULL)
goto done;
dns_list = ares_malloc(sizeof(*dns_list)*(*num_servers));
for (i=0; i<*num_servers; i++)
{
server = (*env)->CallObjectMethod(env, server_list, list_mid, (jint)i);
server = (*env)->CallObjectMethod(env, server_list, android_list_get_mid,
(jint)i);
dns_list[i] = ares_malloc(64);
dns_list[i][0] = 0;
if (server == NULL)
{
continue;
}
str = (*env)->CallObjectMethod(env, server, obj_mid);
str = (*env)->CallObjectMethod(env, server, android_ia_host_addr_mid);
ch_server_address = (*env)->GetStringUTFChars(env, str, 0);
strncpy(dns_list[i], ch_server_address, 64);
(*env)->ReleaseStringUTFChars(env, str, ch_server_address);
(*env)->DeleteLocalRef(env, str);
(*env)->DeleteLocalRef(env, server);
}

done:
Expand Down

0 comments on commit 2ee16d7

Please sign in to comment.