# HG changeset patch # User rriggs # Date 1446217940 14400 # Node ID 051df634418cbdf10dbd1a8b1e3e8a3e29f1b6a1 # Parent 7884ecc0cc6f27ab70c95c7f5ab1424b9e9e2656 8139390: Very long classname in jimage causes SIGSEGV Summary: Correct issues with ImageNativeSubstrate and JImageReadTest Reviewed-by: mchung diff -r 7884ecc0cc6f -r 051df634418c jdk/src/java.base/share/native/libjimage/ImageNativeSubstrate.cpp --- 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 #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) { diff -r 7884ecc0cc6f -r 051df634418c jdk/src/java.base/share/native/libjimage/jimage.cpp --- 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; diff -r 7884ecc0cc6f -r 051df634418c jdk/test/jdk/internal/jimage/JImageReadTest.java --- 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)