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
--- a/hotspot/src/share/vm/classfile/javaClasses.cpp Mon Dec 06 20:21:15 2010 -0500
+++ b/hotspot/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);
--- a/hotspot/src/share/vm/classfile/javaClasses.hpp Mon Dec 06 20:21:15 2010 -0500
+++ b/hotspot/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);
--- a/hotspot/src/share/vm/prims/jni.cpp Mon Dec 06 20:21:15 2010 -0500
+++ b/hotspot/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;
--- a/hotspot/src/share/vm/prims/jniCheck.cpp Mon Dec 06 20:21:15 2010 -0500
+++ b/hotspot/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