# HG changeset patch # User sla # Date 1291720545 28800 # Node ID 017cd8bce8a8aa718ed61e400191a880ce3edb75 # Parent 684faacebf20557ff647cd4f043ddeeb84af179b 6539281: -Xcheck:jni should validate char* argument to ReleaseStringUTFChars Summary: Tag allocated memory with a magic value and verify when releasing. Reviewed-by: phh, stefank diff -r 684faacebf20 -r 017cd8bce8a8 src/share/vm/classfile/javaClasses.cpp --- a/src/share/vm/classfile/javaClasses.cpp Mon Dec 06 20:21:15 2010 -0500 +++ b/src/share/vm/classfile/javaClasses.cpp Tue Dec 07 03:15:45 2010 -0800 @@ -313,6 +313,14 @@ return UNICODE::as_utf8(position, length); } +char* java_lang_String::as_utf8_string(oop java_string, char* buf, int buflen) { + typeArrayOop value = java_lang_String::value(java_string); + int offset = java_lang_String::offset(java_string); + int length = java_lang_String::length(java_string); + jchar* position = (length == 0) ? NULL : value->char_at_addr(offset); + return UNICODE::as_utf8(position, length, buf, buflen); +} + char* java_lang_String::as_utf8_string(oop java_string, int start, int len) { typeArrayOop value = java_lang_String::value(java_string); int offset = java_lang_String::offset(java_string); diff -r 684faacebf20 -r 017cd8bce8a8 src/share/vm/classfile/javaClasses.hpp --- a/src/share/vm/classfile/javaClasses.hpp Mon Dec 06 20:21:15 2010 -0500 +++ b/src/share/vm/classfile/javaClasses.hpp Tue Dec 07 03:15:45 2010 -0800 @@ -104,6 +104,7 @@ // String converters static char* as_utf8_string(oop java_string); + static char* as_utf8_string(oop java_string, char* buf, int buflen); static char* as_utf8_string(oop java_string, int start, int len); static char* as_platform_dependent_str(Handle java_string, TRAPS); static jchar* as_unicode_string(oop java_string, int& length); diff -r 684faacebf20 -r 017cd8bce8a8 src/share/vm/prims/jni.cpp --- a/src/share/vm/prims/jni.cpp Mon Dec 06 20:21:15 2010 -0500 +++ b/src/share/vm/prims/jni.cpp Tue Dec 07 03:15:45 2010 -0800 @@ -2113,11 +2113,10 @@ JNI_ENTRY(const char*, jni_GetStringUTFChars(JNIEnv *env, jstring string, jboolean *isCopy)) JNIWrapper("GetStringUTFChars"); DTRACE_PROBE3(hotspot_jni, GetStringUTFChars__entry, env, string, isCopy); - ResourceMark rm; - char* str = java_lang_String::as_utf8_string(JNIHandles::resolve_non_null(string)); - int length = (int)strlen(str); - char* result = AllocateHeap(length+1, "GetStringUTFChars"); - strcpy(result, str); + oop java_string = JNIHandles::resolve_non_null(string); + size_t length = java_lang_String::utf8_length(java_string); + char* result = AllocateHeap(length + 1, "GetStringUTFChars"); + java_lang_String::as_utf8_string(java_string, result, (int) length + 1); if (isCopy != NULL) *isCopy = JNI_TRUE; DTRACE_PROBE1(hotspot_jni, GetStringUTFChars__return, result); return result; diff -r 684faacebf20 -r 017cd8bce8a8 src/share/vm/prims/jniCheck.cpp --- a/src/share/vm/prims/jniCheck.cpp Mon Dec 06 20:21:15 2010 -0500 +++ b/src/share/vm/prims/jniCheck.cpp Tue Dec 07 03:15:45 2010 -0800 @@ -1288,6 +1288,9 @@ return result; JNI_END +// Arbitrary (but well-known) tag +const jint STRING_TAG = 0x47114711; + JNI_ENTRY_CHECKED(const jchar *, checked_jni_GetStringChars(JNIEnv *env, jstring str, @@ -1297,8 +1300,19 @@ checkString(thr, str); ) const jchar *result = UNCHECKED()->GetStringChars(env,str,isCopy); + assert (isCopy == NULL || *isCopy == JNI_TRUE, "GetStringChars didn't return a copy as expected"); + + size_t len = UNCHECKED()->GetStringLength(env,str) + 1; // + 1 for NULL termination + jint* tagLocation = (jint*) AllocateHeap(len * sizeof(jchar) + sizeof(jint), "checked_jni_GetStringChars"); + *tagLocation = STRING_TAG; + jchar* newResult = (jchar*) (tagLocation + 1); + memcpy(newResult, result, len * sizeof(jchar)); + // Avoiding call to UNCHECKED()->ReleaseStringChars() since that will fire unexpected dtrace probes + // Note that the dtrace arguments for the allocated memory will not match up with this solution. + FreeHeap((char*)result); + functionExit(env); - return result; + return newResult; JNI_END JNI_ENTRY_CHECKED(void, @@ -1309,11 +1323,17 @@ IN_VM( checkString(thr, str); ) - /* cannot check validity of copy, unless every request is logged by - * checking code. Implementation of this check is deferred until a - * subsequent release. - */ - UNCHECKED()->ReleaseStringChars(env,str,chars); + if (chars == NULL) { + // still do the unchecked call to allow dtrace probes + UNCHECKED()->ReleaseStringChars(env,str,chars); + } + else { + jint* tagLocation = ((jint*) chars) - 1; + if (*tagLocation != STRING_TAG) { + NativeReportJNIFatalError(thr, "ReleaseStringChars called on something not allocated by GetStringChars"); + } + UNCHECKED()->ReleaseStringChars(env,str,(const jchar*)tagLocation); + } functionExit(env); JNI_END @@ -1338,6 +1358,9 @@ return result; JNI_END +// Arbitrary (but well-known) tag - different than GetStringChars +const jint STRING_UTF_TAG = 0x48124812; + JNI_ENTRY_CHECKED(const char *, checked_jni_GetStringUTFChars(JNIEnv *env, jstring str, @@ -1347,8 +1370,19 @@ checkString(thr, str); ) const char *result = UNCHECKED()->GetStringUTFChars(env,str,isCopy); + assert (isCopy == NULL || *isCopy == JNI_TRUE, "GetStringUTFChars didn't return a copy as expected"); + + size_t len = strlen(result) + 1; // + 1 for NULL termination + jint* tagLocation = (jint*) AllocateHeap(len + sizeof(jint), "checked_jni_GetStringUTFChars"); + *tagLocation = STRING_UTF_TAG; + char* newResult = (char*) (tagLocation + 1); + strcpy(newResult, result); + // Avoiding call to UNCHECKED()->ReleaseStringUTFChars() since that will fire unexpected dtrace probes + // Note that the dtrace arguments for the allocated memory will not match up with this solution. + FreeHeap((char*)result); + functionExit(env); - return result; + return newResult; JNI_END JNI_ENTRY_CHECKED(void, @@ -1359,11 +1393,17 @@ IN_VM( checkString(thr, str); ) - /* cannot check validity of copy, unless every request is logged by - * checking code. Implementation of this check is deferred until a - * subsequent release. - */ - UNCHECKED()->ReleaseStringUTFChars(env,str,chars); + if (chars == NULL) { + // still do the unchecked call to allow dtrace probes + UNCHECKED()->ReleaseStringUTFChars(env,str,chars); + } + else { + jint* tagLocation = ((jint*) chars) - 1; + if (*tagLocation != STRING_UTF_TAG) { + NativeReportJNIFatalError(thr, "ReleaseStringUTFChars called on something not allocated by GetStringUTFChars"); + } + UNCHECKED()->ReleaseStringUTFChars(env,str,(const char*)tagLocation); + } functionExit(env); JNI_END