stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8258077: Using -Xcheck:jni can lead to a double-free after JDK-8193234

Browse Source 
Reviewed-by: dcubed, hseigel
This commit is contained in:
David Holmes 2021-01-08 04:11:22 +00:00
parent 56a354eb55
commit 712014c595
5 changed files with 316 additions and 19 deletions

43
src/hotspot/share/prims/jniCheck.cpp

@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -43,6 +43,7 @@
#include "runtime/jfieldIDWorkaround.hpp"
#include "runtime/jniHandles.inline.hpp"
#include "runtime/thread.inline.hpp"
#include "utilities/formatBuffer.hpp"
#include "utilities/utf8.hpp"
// Complain every extra number of unplanned local refs
@ -399,19 +400,16 @@ static void* check_wrapped_array(JavaThread* thr, const char* fn_name,
GuardedMemory guarded(carray);
void* orig_result = guarded.get_tag();
if (!guarded.verify_guards()) {
tty->print_cr("ReleasePrimitiveArrayCritical: release array failed bounds "
"check, incorrect pointer returned ? array: " PTR_FORMAT " carray: "
PTR_FORMAT, p2i(obj), p2i(carray));
guarded.print_on(tty);
NativeReportJNIFatalError(thr, "ReleasePrimitiveArrayCritical: "
"failed bounds check");
tty->print_cr("%s: release array failed bounds check, incorrect pointer returned ? array: "
PTR_FORMAT " carray: " PTR_FORMAT, fn_name, p2i(obj), p2i(carray));
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s: failed bounds check", fn_name));
}
if (orig_result == NULL) {
tty->print_cr("ReleasePrimitiveArrayCritical: unrecognized elements. array: "
PTR_FORMAT " carray: " PTR_FORMAT, p2i(obj), p2i(carray));
guarded.print_on(tty);
NativeReportJNIFatalError(thr, "ReleasePrimitiveArrayCritical: "
"unrecognized elements");
tty->print_cr("%s: unrecognized elements. array: " PTR_FORMAT " carray: " PTR_FORMAT,
fn_name, p2i(obj), p2i(carray));
DEBUG_ONLY(guarded.print_on(tty);) // This may crash.
NativeReportJNIFatalError(thr, err_msg("%s: unrecognized elements", fn_name));
}
if (rsz != NULL) {
*rsz = guarded.get_user_size();
@ -420,24 +418,30 @@ static void* check_wrapped_array(JavaThread* thr, const char* fn_name,
}
static void* check_wrapped_array_release(JavaThread* thr, const char* fn_name,
void* obj, void* carray, jint mode) {
void* obj, void* carray, jint mode, jboolean is_critical) {
size_t sz;
void* orig_result = check_wrapped_array(thr, fn_name, obj, carray, &sz);
switch (mode) {
// As we never make copies, mode 0 and JNI_COMMIT are the same.
case 0:
memcpy(orig_result, carray, sz);
GuardedMemory::free_copy(carray);
break;
case JNI_COMMIT:
memcpy(orig_result, carray, sz);
if (is_critical) {
// For ReleasePrimitiveArrayCritical we must free the internal buffer
// allocated through GuardedMemory.
GuardedMemory::free_copy(carray);
}
break;
case JNI_ABORT:
GuardedMemory::free_copy(carray);
break;
default:
tty->print_cr("%s: Unrecognized mode %i releasing array "
PTR_FORMAT " elements " PTR_FORMAT, fn_name, mode, p2i(obj), p2i(carray));
PTR_FORMAT " elements " PTR_FORMAT, fn_name, mode, p2i(obj), p2i(carray));
NativeReportJNIFatalError(thr, "Unrecognized array release mode");
}
// We always need to release the copy we made with GuardedMemory
GuardedMemory::free_copy(carray);
return orig_result;
}
@ -1715,7 +1719,7 @@ JNI_ENTRY_CHECKED(void, \
typeArrayOop a = typeArrayOop(JNIHandles::resolve_non_null(array)); \
) \
ElementType* orig_result = (ElementType *) check_wrapped_array_release( \
thr, "checked_jni_Release"#Result"ArrayElements", array, elems, mode); \
thr, "checked_jni_Release"#Result"ArrayElements", array, elems, mode, JNI_FALSE); \
UNCHECKED()->Release##Result##ArrayElements(env, array, orig_result, mode); \
functionExit(thr); \
JNI_END
@ -1884,7 +1888,8 @@ JNI_ENTRY_CHECKED(void,
check_is_primitive_array(thr, array);
)
// Check the element array...
void* orig_result = check_wrapped_array_release(thr, "ReleasePrimitiveArrayCritical", array, carray, mode);
void* orig_result = check_wrapped_array_release(thr, "ReleasePrimitiveArrayCritical",
array, carray, mode, JNI_TRUE);
UNCHECKED()->ReleasePrimitiveArrayCritical(env, array, orig_result, mode);
functionExit(thr);
JNI_END

122
test/hotspot/jtreg/runtime/jni/checked/TestCheckedReleaseArrayElements.java Normal file

@ -0,0 +1,122 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/* @test
* @bug 8258077
* @summary verify multiple release calls on a copied array work when checked
* @library /test/lib
* @run main/native TestCheckedReleaseArrayElements launch
*/
import java.util.Arrays;
import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.Utils;
import jtreg.SkippedException;
public class TestCheckedReleaseArrayElements {
static {
System.loadLibrary("TestCheckedReleaseArrayElements");
}
public static void main(String[] args) throws Throwable {
if (args == null || args.length == 0) {
test();
} else {
OutputAnalyzer output =
ProcessTools.executeTestJvm("-Xcheck:jni",
"-Djava.library.path=" + Utils.TEST_NATIVE_PATH,
"TestCheckedReleaseArrayElements");
output.shouldHaveExitValue(0);
output.stderrShouldBeEmpty();
output.stdoutShouldNotBeEmpty();
}
}
/*
* If GetIntArrayElements returns a copy, we update the array in slices
* calling ReleaseIntArrayElements with JNI_COMMIT to write-back the
* updates, which are then checked. Finally we use JNI_ABORT to free
* the copy.
*/
public static void test() {
final int slices = 3;
final int sliceLength = 3;
int[] arr = new int[slices * sliceLength];
if (!init(arr)) {
throw new SkippedException("Test skipped as GetIntArrayElements did not make a copy");
}
System.out.println("Array before: " + Arrays.toString(arr));
// We fill the array in slices so that arr[i] = i
for (int i = 0; i < slices; i++) {
int start = i * sliceLength;
fill(arr, start, sliceLength);
System.out.println("Array during: " + Arrays.toString(arr));
check(arr, (i + 1) * sliceLength);
}
System.out.println("Array after: " + Arrays.toString(arr));
cleanup(arr);
}
/*
* Calls GetIntArrayElements and stashes the native pointer for
* use by fill() if a copy was made.
* Returns true if a copy was made else false.
*/
static native boolean init(int[] arr);
/*
* Fills in target[start] to target[start+count-1], so that
* target[i] == i. The update is done natively using the raw
* pointer into the array.
*/
static native void fill(int[] target, int start, int count);
/*
* Properly release the copied array
*/
static native void cleanup(int[] target);
static void check(int[] source, int count) {
for (int i = 0; i < count; i++) {
if (source[i] != i) {
System.out.println("Failing source array: " + Arrays.toString(source));
throw new RuntimeException("Expected source[" + i + "] == " +
i + " but got " + source[i]);
}
}
for (int i = count; i < source.length; i++) {
if (source[i] != 0) {
System.out.println("Failing source array: " + Arrays.toString(source));
throw new RuntimeException("Expected source[" + i +
"] == 0 but got " + source[i]);
}
}
}
}

60
test/hotspot/jtreg/runtime/jni/checked/TestCheckedReleaseCriticalArray.java Normal file

@ -0,0 +1,60 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
/* @test
* @bug 8193234 8258077
* @summary Check ReleasePrimitiveArrayCritical doesn't leak memory with
* Xcheck:jni and JNI_COMMIT mode
* @comment This is a manual test as you need to verify memory usage externally.
* @library /test/lib
* @run main/othervm/native/manual -Xms4m -Xmx4m -Xcheck:jni TestCheckedReleaseCriticalArray
*/
import jdk.test.lib.process.ProcessTools;
import jdk.test.lib.process.OutputAnalyzer;
import jtreg.SkippedException;
public class TestCheckedReleaseCriticalArray {
static {
System.loadLibrary("TestCheckedReleaseCriticalArray");
}
/*
* We repeatedly modify an array via the JNI critical functions, using
* JNI_COMMIT mode. No memory leak should be observed on a VM that
* provides direct array access.
*/
public static void main(String[] args) {
int[] array = new int[] { 1, 2, 3 };
if (!modifyArray(array)) {
// If the VM makes copies then we will leak them if we only ever use
// JNI_COMMIT mode.
throw new SkippedException("Test skipped as GetPrimitiveArrayCritical made a copy");
}
while (true) {
modifyArray(array);
}
}
private static native boolean modifyArray(int[] array);
}

64
test/hotspot/jtreg/runtime/jni/checked/libTestCheckedReleaseArrayElements.c Normal file

@ -0,0 +1,64 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#include <jni.h>
static jint* arr;
JNIEXPORT jboolean JNICALL
Java_TestCheckedReleaseArrayElements_init(JNIEnv *env,
jclass* clazz,
jintArray target) {
jboolean isCopy;
arr = (*env)->GetIntArrayElements(env, target, &isCopy);
if (arr == NULL) {
(*env)->FatalError(env, "Unexpected NULL return from GetIntArrayElements");
}
if (isCopy == JNI_FALSE) {
(*env)->ReleaseIntArrayElements(env, target, arr, JNI_ABORT);
}
return isCopy;
}
JNIEXPORT void JNICALL
Java_TestCheckedReleaseArrayElements_cleanup(JNIEnv *env,
jclass* clazz,
jintArray target) {
(*env)->ReleaseIntArrayElements(env, target, arr, JNI_ABORT);
}
JNIEXPORT void JNICALL
Java_TestCheckedReleaseArrayElements_fill(JNIEnv *env,
jclass* clazz,
jintArray target,
jint start,
jint count) {
// Update a slice of the raw array
int i;
for (i = start; count > 0; i++, count--) {
arr[i] = i;
}
// Write the results back to target, leaving it usable for future updates
(*env)->ReleaseIntArrayElements(env, target, arr, JNI_COMMIT);
}

46
test/hotspot/jtreg/runtime/jni/checked/libTestCheckedReleaseCriticalArray.c Normal file

@ -0,0 +1,46 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#include <jni.h>
JNIEXPORT jboolean JNICALL
Java_TestCheckedReleaseCriticalArray_modifyArray(JNIEnv *env,
jclass clazz,
jintArray iarr) {
jboolean isCopy;
jint* arr = (jint *)(*env)->GetPrimitiveArrayCritical(env, iarr, &isCopy);
if (arr == NULL) {
(*env)->FatalError(env, "Unexpected NULL return from GetPrimitiveArrayCritical");
}
if (isCopy == JNI_FALSE) {
jint len = (*env)->GetArrayLength(env, iarr);
// make arbitrary changes to the array
for (int i = 0; i < len; i++) {
arr[i] *= 2;
}
// write-back using JNI_COMMIT to test for memory leak
(*env)->ReleasePrimitiveArrayCritical(env, iarr, arr, JNI_COMMIT);
}
// we skip the test if the VM makes a copy - as it will definitely leak
return !isCopy;
}