8139390: Very long classname in jimage causes SIGSEGV
Summary: Correct issues with ImageNativeSubstrate and JImageReadTest
Reviewed-by: mchung
--- a/jdk/src/java.base/share/native/libjimage/ImageNativeSubstrate.cpp Thu Oct 29 12:39:08 2015 -0700
+++ b/jdk/src/java.base/share/native/libjimage/ImageNativeSubstrate.cpp Fri Oct 30 11:12:20 2015 -0400
@@ -26,8 +26,7 @@
#include <string.h>
#include "jni.h"
-#include "jni_util.h"
-#include "jdk_util.h"
+
#include "endian.hpp"
#include "imageDecompressor.hpp"
#include "imageFile.hpp"
@@ -39,6 +38,17 @@
extern bool MemoryMapImage;
+/////////////////////////////////////////////////////////////////////////////
+
+// Static function for primitive throw since libjimage is not linked with libjava
+static void JNICALL ThrowByName(JNIEnv *env, const char *name, const char *msg)
+{
+ jclass cls = (env)->FindClass(name);
+
+ if (cls != 0) /* Otherwise an exception has already been thrown */
+ (env)->ThrowNew(cls, msg);
+}
+
// jdk.internal.jimage /////////////////////////////////////////////////////////
// Java entry to open an image file for sharing.
@@ -446,6 +456,23 @@
jlong size = 0;
jlong ret = 0;
+ if (moduleName == NULL) {
+ ThrowByName(env, "java/lang/NullPointerException", "moduleName");
+ return 0;
+ }
+ if (version == NULL) {
+ ThrowByName(env, "java/lang/NullPointerException", "version");
+ return 0;
+ }
+ if (path == NULL) {
+ ThrowByName(env, "java/lang/NullPointerException", "path");
+ return 0;
+ }
+ if (output_size == NULL) {
+ ThrowByName(env, "java/lang/NullPointerException", "size");
+ return 0;
+ }
+
do {
native_module = env->GetStringUTFChars(moduleName, NULL);
if (native_module == NULL)
@@ -529,25 +556,47 @@
// Store if there is room in the array
// Concatenate to get full path
char fullpath[IMAGE_MAX_PATH];
- fullpath[0] = '\0';
- if (*module != '\0') {
- strncpy(fullpath, "/", IMAGE_MAX_PATH - 1);
- strncat(fullpath, module, IMAGE_MAX_PATH - 1);
- strncat(fullpath, "/", IMAGE_MAX_PATH - 1);
+ size_t moduleLen = strlen(module);
+ size_t packageLen = strlen(package);
+ size_t nameLen = strlen(name);
+ size_t extLen = strlen(extension);
+ size_t index;
+
+ if (1 + moduleLen + 1 + packageLen + 1 + nameLen + 1 + extLen + 1 > IMAGE_MAX_PATH) {
+ ThrowByName(env, "java/lang/InternalError", "concatenated name too long");
+ return true;
}
- if (*package != '\0') {
- strncat(fullpath, package, IMAGE_MAX_PATH - 1);
- strncat(fullpath, "/", IMAGE_MAX_PATH - 1);
+
+ index = 0;
+ if (moduleLen > 0) {
+ fullpath[index++] = '/';
+ memcpy(&fullpath[index], module, moduleLen);
+ index += moduleLen;
+ fullpath[index++] = '/';
}
- strncat(fullpath, name, IMAGE_MAX_PATH - 1);
- if (*extension != '\0') {
- strncat(fullpath, ".", IMAGE_MAX_PATH - 1);
- strncat(fullpath, extension, IMAGE_MAX_PATH - 1);
+ if (packageLen > 0) {
+ memcpy(&fullpath[index], package, packageLen);
+ index += packageLen;
+ fullpath[index++] = '/';
+ }
+ memcpy(&fullpath[index], name, nameLen);
+ index += nameLen;
+ if (extLen > 0) {
+ fullpath[index++] = '.';
+ memcpy(&fullpath[index], extension, extLen);
+ index += extLen;
}
+ fullpath[index++] = '\0';
+
jobject str = env->NewStringUTF(fullpath);
- JNU_CHECK_EXCEPTION_RETURN(env, true);
+ if (env->ExceptionCheck()) {
+ return true;
+ }
+
env->SetObjectArrayElement(vdata->array, vdata->size, str);
- JNU_CHECK_EXCEPTION_RETURN(env, true);
+ if (env->ExceptionCheck()) {
+ return true;
+ }
}
vdata->size++; // always count so the total size is returned
return true;
@@ -584,7 +633,10 @@
jstring module = NULL;
native_package = env->GetStringUTFChars(package_name, NULL);
- JNU_CHECK_EXCEPTION_RETURN(env, NULL);
+ if (env->ExceptionCheck()) {
+ return NULL;
+ }
+
native_module = JIMAGE_PackageToModule((JImageFile*) jimageHandle, native_package);
if (native_module != NULL) {
--- a/jdk/src/java.base/share/native/libjimage/jimage.cpp Thu Oct 29 12:39:08 2015 -0700
+++ b/jdk/src/java.base/share/native/libjimage/jimage.cpp Fri Oct 30 11:12:20 2015 -0400
@@ -102,14 +102,29 @@
extern "C" JImageLocationRef JIMAGE_FindResource(JImageFile* image,
const char* module_name, const char* version, const char* name,
jlong* size) {
- ImageLocation location;
+ // Concatenate to get full path
char fullpath[IMAGE_MAX_PATH];
+ size_t moduleNameLen = strlen(module_name);
+ size_t nameLen = strlen(name);
+ size_t index;
+
+ // TBD: assert(moduleNameLen > 0 && "module name must be non-empty");
+ assert(nameLen > 0 && "name must non-empty");
- // Concatenate to get full path
- strncpy(fullpath, "/", IMAGE_MAX_PATH - 1);
- strncat(fullpath, module_name, IMAGE_MAX_PATH - 1);
- strncat(fullpath, "/", IMAGE_MAX_PATH - 1);
- strncat(fullpath, name, IMAGE_MAX_PATH - 1);
+ // If the concatenated string is too long for the buffer, return not found
+ if (1 + moduleNameLen + 1 + nameLen + 1 > IMAGE_MAX_PATH) {
+ return 0L;
+ }
+
+ index = 0;
+ fullpath[index++] = '/';
+ memcpy(&fullpath[index], module_name, moduleNameLen);
+ index += moduleNameLen;
+ fullpath[index++] = '/';
+ memcpy(&fullpath[index], name, nameLen);
+ index += nameLen;
+ fullpath[index++] = '\0';
+
JImageLocationRef loc =
(JImageLocationRef) ((ImageFileReader*) image)->find_location_index(fullpath, (u8*) size);
return loc;
--- a/jdk/test/jdk/internal/jimage/JImageReadTest.java Thu Oct 29 12:39:08 2015 -0700
+++ b/jdk/test/jdk/internal/jimage/JImageReadTest.java Fri Oct 30 11:12:20 2015 -0400
@@ -22,7 +22,9 @@
*/
/*
+ * @test
* @modules java.base/jdk.internal.jimage
+ * @run testng JImageReadTest
* @summary Unit test for libjimage JIMAGE_Open/Read/Close
*/
@@ -57,8 +59,7 @@
{"java.base", "java/lang/String.class"},
{"java.base", "java/lang/Object.class"},
{"java.base", "sun/reflect/generics/tree/TypeArgument.class"},
- {"jdk.jdeps", "com/sun/tools/javap/StackMapWriter$StackMapBuilder.class"},
- {"jdk.hotspot.agent", "sa.properties"},
+ {"java.base", "sun/net/www/content-types.properties"},
{"java.logging", "java/util/logging/Logger.class"},
{"java.base", "java/NOSUCHCLASS/yyy.class"}, // non-existent
{"NOSUCHMODULE", "java/lang/Class.class"}, // non-existent
@@ -165,8 +166,10 @@
int count = ImageNativeSubstrate.JIMAGE_Resources(jimageHandle,
names);
System.out.printf(" count: %d, a class: %s\n", count, names[0]);
- Assert.assertTrue(max > 31000,
- "missing entries, should be more than 31000, reported: " + count);
+ int minEntryCount = 16000;
+ Assert.assertTrue(max > minEntryCount,
+ "missing entries, should be more than " + minEntryCount +
+ ", reported: " + count);
Assert.assertTrue(count == max,
"unexpected count of entries, count: " + count
+ ", max: " + max);
@@ -310,6 +313,7 @@
static boolean isMetaName(String name) {
return name.startsWith("/modules")
|| name.startsWith("/packages")
+ || name.startsWith("META-INF/services")
|| name.equals("bootmodules.jdata");
}
@@ -362,6 +366,24 @@
System.out.printf(" %s: %d names%n", fname, names.length);
}
+ @Test
+ static void test5_nameTooLong() throws IOException {
+ long[] size = new long[1];
+ String moduleName = "FictiousModuleName";
+ String className = String.format("A%09999d", 1);
+
+ long jimageHandle = ImageNativeSubstrate.JIMAGE_Open(imageFile);
+ Assert.assertTrue(jimageHandle != 0, "JIMAGE_Open failed: id: " + jimageHandle);
+
+ long locationHandle =
+ ImageNativeSubstrate.JIMAGE_FindResource(jimageHandle,
+ moduleName, "9.0", className, size);
+
+ Assert.assertEquals(0, locationHandle, "Too long name should have failed");
+
+ ImageNativeSubstrate.JIMAGE_Close(jimageHandle);
+ }
+
// main method to run standalone from jtreg
@Test(enabled=false)