JDK-5097131 : ClassFileLoadHook can be called with classname==NULL, hprof & demos could SEGV
  • Type: Bug
  • Component: core-svc
  • Sub-Component: tools
  • Affected Version: 5.0
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2004-09-04
  • Updated: 2004-10-11
  • Resolved: 2004-10-09
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other JDK 6
5.0u2Fixed 6 b08Fixed
Related Reports
Relates :  
Description
ClassFileLoadHook can be called with classname==NULL, hprof & demos could SEGV
when this happens.

Need to add in some null pointer checks into the hprof, mtrace, and heapTracker
demos.

Comments
CONVERTED DATA BugTraq+ Release Management Values COMMIT TO FIX: mustang FIXED IN: mustang
28-09-2004

SUGGESTED FIX HeapTracker demo, hprof demo, and java_crw_demo changes: ---------------------------------------------------------------------------- heapTracker demo: ------- heapTracker.c ------- *** /tmp/sccs.efaiBv Thu Sep 23 16:01:22 2004 --- heapTracker.c Fri Sep 17 09:21:48 2004 *************** *** 1,5 **** /* ! * @(#)heapTracker.c 1.7 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)heapTracker.c 1.10 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 791,801 **** /* It's possible we get here right after VmDeath event, be careful */ if ( !gdata->vmDead ) { *new_class_data_len = 0; *new_class_data = NULL; /* The tracker class itself? */ ! if ( strcmp(name, STRING(HEAP_TRACKER_class)) != 0 ) { jint cnum; int systemClass; unsigned char *newImage; --- 791,817 ---- /* It's possible we get here right after VmDeath event, be careful */ if ( !gdata->vmDead ) { + const char * classname; + + /* Name can be NULL, make sure we avoid SEGV's */ + if ( name == NULL ) { + classname = java_crw_demo_classname(class_data, class_data_len, + NULL); + if ( classname == NULL ) { + fatal_error("ERROR: No classname in classfile\n"); + } + } else { + classname = strdup(name); + if ( classname == NULL ) { + fatal_error("ERROR: Ran out of malloc() space\n"); + } + } + *new_class_data_len = 0; *new_class_data = NULL; /* The tracker class itself? */ ! if ( strcmp(classname, STRING(HEAP_TRACKER_class)) != 0 ) { jint cnum; int systemClass; unsigned char *newImage; *************** *** 818,824 **** /* Call the class file reader/write demo code */ java_crw_demo(cnum, ! name, class_data, class_data_len, systemClass, --- 834,840 ---- /* Call the class file reader/write demo code */ java_crw_demo(cnum, ! classname, class_data, class_data_len, systemClass, *************** *** 850,855 **** --- 866,873 ---- (void)free((void*)newImage); /* Free malloc() space with free() */ } } + + (void)free((void*)classname); } } exitCriticalSection(jvmti); } ---------------------------------------------------------------------------- Hprof: ------- hprof.h ------- *** /tmp/sccs.4iaqCv Thu Sep 23 16:01:23 2004 --- hprof.h Fri Sep 17 10:58:08 2004 *************** *** 1,5 **** /* ! * @(#)hprof.h 1.39 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)hprof.h 1.41 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 357,362 **** --- 367,373 ---- /* Handles to java_crw_demo library */ void * java_crw_demo_library; void * java_crw_demo_function; + void * java_crw_demo_classname_function; } GlobalData; ------- hprof_init.c ------- *** /tmp/sccs.9oaGEv Thu Sep 23 16:01:23 2004 --- hprof_init.c Fri Sep 17 09:22:42 2004 *************** *** 1,5 **** /* ! * @(#)hprof_init.c 1.80 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)hprof_init.c 1.83 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 1373,1379 **** /* WARNING: This will be called before VM_INIT. */ ! LOG2("cbClassFileLoadHook:",name); if (!gdata->bci) { return; --- 1373,1379 ---- /* WARNING: This will be called before VM_INIT. */ ! LOG2("cbClassFileLoadHook:",(name==NULL?"Unknown":name)); if (!gdata->bci) { return; *************** *** 1381,1386 **** --- 1381,1387 ---- BEGIN_CALLBACK() { rawMonitorEnter(gdata->data_access_lock); { + const char *classname; if ( gdata->bci_counter == 0 ) { /* Prime the system classes */ *************** *** 1392,1399 **** *new_class_data_len = 0; *new_class_data = NULL; /* The tracker class itself? */ ! if ( strcmp(name,"sun/tools/hprof/Tracker") != 0 ) { ClassIndex cnum; int system_class; unsigned char * new_image; --- 1393,1415 ---- *new_class_data_len = 0; *new_class_data = NULL; + /* Name could be NULL */ + if ( name == NULL ) { + classname = ((JavaCrwDemoClassname) + (gdata->java_crw_demo_classname_function)) + (class_data, class_data_len, &my_crw_fatal_error_handler); + if ( classname == NULL ) { + HPROF_ERROR(JNI_TRUE, "No classname in classfile"); + } + } else { + classname = strdup(name); + if ( classname == NULL ) { + HPROF_ERROR(JNI_TRUE, "Ran out of malloc() space"); + } + } + /* The tracker class itself? */ ! if ( strcmp(classname,"sun/tools/hprof/Tracker") != 0 ) { ClassIndex cnum; int system_class; unsigned char * new_image; *************** *** 1402,1414 **** char *signature; LoaderIndex loader_index; ! LOG2("cbClassFileLoadHook injecting class" , name); /* Define a unique class number for this class */ ! len = (int)strlen(name); signature = HPROF_MALLOC(len+3); signature[0] = JVM_SIGNATURE_CLASS; ! (void)memcpy(signature+1, name, len); signature[len+1] = JVM_SIGNATURE_ENDCLASS; signature[len+2] = 0; loader_index = loader_find_or_create(env,loader); --- 1418,1430 ---- char *signature; LoaderIndex loader_index; ! LOG2("cbClassFileLoadHook injecting class" , classname); /* Define a unique class number for this class */ ! len = (int)strlen(classname); signature = HPROF_MALLOC(len+3); signature[0] = JVM_SIGNATURE_CLASS; ! (void)memcpy(signature+1, classname, len); signature[len+1] = JVM_SIGNATURE_ENDCLASS; signature[len+2] = 0; loader_index = loader_find_or_create(env,loader); *************** *** 1430,1436 **** && ( ( class_get_status(cnum) & CLASS_SYSTEM) != 0 || gdata->bci_counter < 8 ) ) { system_class = 1; ! LOG2(name, " is a system class"); } new_image = NULL; --- 1446,1452 ---- && ( ( class_get_status(cnum) & CLASS_SYSTEM) != 0 || gdata->bci_counter < 8 ) ) { system_class = 1; ! LOG2(classname, " is a system class"); } new_image = NULL; *************** *** 1439,1445 **** /* Call the class file reader/write demo code */ ((JavaCrwDemo)(gdata->java_crw_demo_function))( cnum, ! name, class_data, class_data_len, system_class, --- 1455,1461 ---- /* Call the class file reader/write demo code */ ((JavaCrwDemo)(gdata->java_crw_demo_function))( cnum, ! classname, class_data, class_data_len, system_class, *************** *** 1461,1473 **** if ( new_length > 0 ) { unsigned char *jvmti_space; ! LOG2("cbClassFileLoadHook DID inject this class", name); jvmti_space = (unsigned char *)jvmtiAllocate((jint)new_length); (void)memcpy((void*)jvmti_space, (void*)new_image, (int)new_length); *new_class_data_len = (jint)new_length; *new_class_data = jvmti_space; /* VM will deallocate */ } else { ! LOG2("cbClassFileLoadHook DID NOT inject this class", name); *new_class_data_len = 0; *new_class_data = NULL; } --- 1477,1489 ---- if ( new_length > 0 ) { unsigned char *jvmti_space; ! LOG2("cbClassFileLoadHook DID inject this class", classname); jvmti_space = (unsigned char *)jvmtiAllocate((jint)new_length); (void)memcpy((void*)jvmti_space, (void*)new_image, (int)new_length); *new_class_data_len = (jint)new_length; *new_class_data = jvmti_space; /* VM will deallocate */ } else { ! LOG2("cbClassFileLoadHook DID NOT inject this class", classname); *new_class_data_len = 0; *new_class_data = NULL; } *************** *** 1475,1480 **** --- 1491,1497 ---- (void)free((void*)new_image); /* Free malloc() space with free() */ } } + (void)free((void*)classname); } rawMonitorExit(gdata->data_access_lock); } END_CALLBACK(); } *************** *** 1781,1786 **** --- 1798,1855 ---- } + /* Dynamic library loading */ + static void * + load_library(char *name) + { + char lname[FILENAME_MAX+1]; + char err_buf[256+FILENAME_MAX+1]; + char *boot_path; + void *handle; + + handle = NULL; + + /* The library may be located in different ways, try both, but + * if it comes from outside the SDK/jre it isn't ours. + */ + getSystemProperty("sun.boot.library.path", &boot_path); + md_build_library_name(lname, FILENAME_MAX, boot_path, name); + handle = md_load_library(lname, err_buf, (int)sizeof(err_buf)); + if ( handle == NULL ) { + /* This may be necessary on Windows. */ + md_build_library_name(lname, FILENAME_MAX, "", name); + handle = md_load_library(lname, err_buf, (int)sizeof(err_buf)); + if ( handle == NULL ) { + HPROF_ERROR(JNI_TRUE, err_buf); + } + } + return handle; + } + + /* Lookup dynamic function pointer in shared library */ + static void * + lookup_library_symbol(void *library, char **symbols, int nsymbols) + { + void *addr; + int i; + + addr = NULL; + for( i = 0 ; i < nsymbols; i++ ) { + addr = md_find_library_entry(library, symbols[i]); + if ( addr != NULL ) { + break; + } + } + if ( addr == NULL ) { + char errmsg[256]; + + (void)md_snprintf(errmsg, sizeof(errmsg), + "Cannot find library symbol '%s'", symbols[0]); + HPROF_ERROR(JNI_TRUE, errmsg); + } + return addr; + } + /* ------------------------------------------------------------------- */ /* The OnLoad interface */ *************** *** 1859,1900 **** /* Load java_crw_demo library and find function "java_crw_demo" */ if ( gdata->bci ) { - static char *symbols[] = JAVA_CRW_DEMO_SYMBOLS; /* "java_crw_demo" */ - char lname[FILENAME_MAX+1]; - char err_buf[256+FILENAME_MAX+1]; - char *boot_path; - int i; ! /* The library may be located in different ways, try both, but ! * if it comes from outside the SDK/jre it isn't ours. ! * But with a name like java_crw_demo, what are the odds? ! */ ! getSystemProperty("sun.boot.library.path", &boot_path); ! md_build_library_name(lname, FILENAME_MAX, boot_path, "java_crw_demo"); ! gdata->java_crw_demo_library = ! md_load_library(lname, err_buf, (int)sizeof(err_buf)); ! if ( gdata->java_crw_demo_library == NULL ) { ! /* This may be necessary on Windows. */ ! md_build_library_name(lname, FILENAME_MAX, "", "java_crw_demo"); ! gdata->java_crw_demo_library = ! md_load_library(lname, err_buf, (int)sizeof(err_buf)); ! if ( gdata->java_crw_demo_library == NULL ) { ! HPROF_ERROR(JNI_TRUE, err_buf); ! } ! } ! /* The function may have different names, we find the first one */ ! for( i = 0 ; i < (int)(sizeof(symbols)/sizeof(char*)) ; i++ ) { gdata->java_crw_demo_function = ! md_find_library_entry(gdata->java_crw_demo_library, symbols[i]); ! if ( gdata->java_crw_demo_function != NULL ) { ! break; } } - if ( gdata->java_crw_demo_function == NULL ) { - HPROF_ERROR(JNI_TRUE, "Cannot find java_crw_demo function."); } - } return JNI_OK; } --- 1928,1950 ---- /* Load java_crw_demo library and find function "java_crw_demo" */ if ( gdata->bci ) { ! /* Load the library or get the handle to it */ ! gdata->java_crw_demo_library = load_library("java_crw_demo"); ! { /* "java_crw_demo" */ ! static char *symbols[] = JAVA_CRW_DEMO_SYMBOLS; gdata->java_crw_demo_function = ! lookup_library_symbol(gdata->java_crw_demo_library, ! symbols, (int)(sizeof(symbols)/sizeof(char*))); } + { /* "java_crw_demo_classname" */ + static char *symbols[] = JAVA_CRW_DEMO_CLASSNAME_SYMBOLS; + gdata->java_crw_demo_classname_function = + lookup_library_symbol(gdata->java_crw_demo_library, + symbols, (int)(sizeof(symbols)/sizeof(char*))); } } return JNI_OK; } ---------------------------------------------------------------------------- java_crw_demo: ------- java_crw_demo.c ------- *** /tmp/sccs.pHaaJv Thu Sep 23 16:01:24 2004 --- java_crw_demo.c Fri Sep 17 09:24:09 2004 *************** *** 1,5 **** /* ! * @(#)java_crw_demo.c 1.20 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)java_crw_demo.c 1.23 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 285,300 **** void * ptr; ptr = malloc(nbytes); return ptr; } static void * allocate_clean(CrwClassImage *ci, int nbytes) { void * ptr; ! ptr = allocate(ci, nbytes); ! (void)memset(ptr, 0, nbytes); return ptr; } --- 282,314 ---- void * ptr; ptr = malloc(nbytes); + if ( ptr == NULL ) { + CRW_FATAL(ci, "Ran out of malloc memory"); + } return ptr; } static void * + reallocate(CrwClassImage *ci, void *optr, int nbytes) + { + void * ptr; + + ptr = realloc(optr, nbytes); + if ( ptr == NULL ) { + CRW_FATAL(ci, "Ran out of malloc memory"); + } + return ptr; + } + + static void * allocate_clean(CrwClassImage *ci, int nbytes) { void * ptr; ! ptr = calloc(nbytes, 1); ! if ( ptr == NULL ) { ! CRW_FATAL(ci, "Ran out of malloc memory"); ! } return ptr; } *************** *** 356,363 **** --- 370,379 ---- writeU1(CrwClassImage *ci, unsigned val) /* Only writes out lower 8 bits */ { CRW_ASSERT_CI(ci); + if ( ci->output != NULL ) { ci->output[ci->output_position++] = val & 0xFF; } + } static void writeU2(CrwClassImage *ci, unsigned val) *************** *** 666,673 **** --- 682,691 ---- } } +
28-09-2004

PUBLIC COMMENTS Also see bug 5096167. If the JVMTI ClassFileLoadHook interface does return NULL for the class name, the demos hprof, mtrace, and heapTracker could cause a SEGV and trigger a VM crash.
28-09-2004

SUGGESTED FIX if ( ci->tclass_name != NULL ) { ci->tracker_class_index = add_new_class_cpool_entry(ci, ci->tclass_name); + } if (ci->obj_init_name != NULL) { ci->object_init_tracker_index = add_new_method_cpool_entry(ci, ci->tracker_class_index, *************** *** 2121,2127 **** /* Do the injection */ max_length = file_len*2 + 512; /* Twice as big + 512 */ ! new_image = malloc((int)max_length); new_length = inject_class(&ci, system_class, tclass_name, --- 2152,2158 ---- /* Do the injection */ max_length = file_len*2 + 512; /* Twice as big + 512 */ ! new_image = allocate(&ci, (int)max_length); new_length = inject_class(&ci, system_class, tclass_name, *************** *** 2139,2148 **** /* Dispose or shrink the space to be returned. */ if ( new_length == 0 ) { ! (void)free(new_image); new_image = NULL; } else { ! new_image = (void*)realloc((void*)new_image, (int)new_length); } /* Return the new class image */ --- 2170,2179 ---- /* Dispose or shrink the space to be returned. */ if ( new_length == 0 ) { ! deallocate(&ci, (void*)new_image); new_image = NULL; } else { ! new_image = (void*)reallocate(&ci, (void*)new_image, (int)new_length); } /* Return the new class image */ *************** *** 2153,2155 **** --- 2184,2240 ---- cleanup(&ci); } + /* Return the classname for this class which is inside the classfile image. */ + JNIEXPORT char * JNICALL + java_crw_demo_classname(const unsigned char *file_image, long file_len, + FatalErrorHandler fatal_error_handler) + { + CrwClassImage ci; + CrwConstantPoolEntry cs; + CrwCpoolIndex this_class; + unsigned magic; + char * name; + + name = NULL; + + if ( file_len==0 || file_image==NULL ) { + return name; + } + + /* The only fields we need filled in are the image pointer and the error + * handler. + * By not adding an output buffer pointer, no output is created. + */ + (void)memset(&ci, 0, (int)sizeof(CrwClassImage)); + ci.input = file_image; + ci.input_len = file_len; + ci.fatal_error_handler = fatal_error_handler; + + /* Read out the bytes from the classfile image */ + + magic = readU4(&ci); /* magic number */ + CRW_ASSERT(&ci, magic==0xCAFEBABE); + if ( magic != 0xCAFEBABE ) { + return name; + } + (void)readU2(&ci); /* minor version number */ + (void)readU2(&ci); /* major version number */ + + /* Read in constant pool. Since no output setup, writes are NOP's */ + cpool_setup(&ci); + + (void)readU2(&ci); /* access flags */ + this_class = readU2(&ci); /* 'this' class */ + + /* Get 'this' constant pool entry */ + cs = cpool_entry(&ci, (CrwCpoolIndex)(cpool_entry(&ci, this_class).index1)); + + /* Duplicate the name */ + name = (char *)duplicate(&ci, cs.ptr, cs.len); + + /* Cleanup before we leave. */ + cleanup(&ci); + + /* Return malloc space */ + return name; + } ------- java_crw_demo.h ------- *** /tmp/sccs.CMaiKv Thu Sep 23 16:01:25 2004 --- java_crw_demo.h Fri Sep 17 09:20:05 2004 *************** *** 1,5 **** /* ! * @(#)java_crw_demo.h 1.13 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)java_crw_demo.h 1.15 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 159,163 **** --- 159,185 ---- ); + + /* External to read the class name out of a class file . + * + * WARNING: If You change the typedef, you MUST change + * multiple things in this file, including this name. + */ + + #define JAVA_CRW_DEMO_CLASSNAME_SYMBOLS + { "java_crw_demo_classname", "_java_crw_demo_classname@12" } + + /* Typedef needed for type casting in dynamic access situations. */ + + typedef char * (JNICALL *JavaCrwDemoClassname)( + const unsigned char *file_image, + long file_len, + FatalErrorHandler fatal_error_handler); + + JNIEXPORT char * JNICALL java_crw_demo_classname( + const unsigned char *file_image, + long file_len, + FatalErrorHandler fatal_error_handler); + #endif ---------------------------------------------------------------------------- mtrace demo: ------- mtrace.c ------- *** /tmp/sccs.nPaqLv Thu Sep 23 16:01:25 2004 --- mtrace.c Fri Sep 17 09:21:10 2004 *************** *** 1,5 **** /* ! * @(#)mtrace.c 1.23 04/07/27 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * --- 1,5 ---- /* ! * @(#)mtrace.c 1.26 04/09/17 * * Copyright (c) 2004 Sun Microsystems, Inc. All Rights Reserved. * *************** *** 269,277 **** --- 269,283 ---- mp = cp->methods + mnum; mp->name = (const char *)strdup(names[mnum]); + if ( mp->name == NULL ) { + fatal_error("ERROR: Out of malloc memory\n"); + } mp->signature = (const char *)strdup(sigs[mnum]); + if ( mp->signature == NULL ) { + fatal_error("ERROR: Out of malloc memory\n"); } } + } /* Java Native Method for entry */ static void *************** *** 523,534 **** /* It's possible we get here right after VmDeath event, be careful */ if ( !gdata->vm_is_dead ) { *new_class_data_len = 0; *new_class_data = NULL; /* The tracker class itself? */ ! if ( interested((char*)name, "", gdata->include, gdata->exclude) ! && strcmp(name, STRING(MTRACE_class)) != 0 ) { jint cnum; int system_class; unsigned char *new_image; --- 529,556 ---- /* It's possible we get here right after VmDeath event, be careful */ if ( !gdata->vm_is_dead ) { + const char *classname; + + /* Name could be NULL */ + if ( name == NULL ) { + classname = java_crw_demo_classname(class_data, class_data_len, + NULL); + if ( classname == NULL ) { + fatal_error("ERROR: No classname inside classfile\n"); + } + } else { + classname = strdup(name); + if ( classname == NULL ) { + fatal_error("ERROR: Out of malloc memory\n"); + } + } + *new_class_data_len = 0; *new_class_data = NULL; /* The tracker class itself? */ ! if ( interested((char*)classname, "", gdata->include, gdata->exclude) ! && strcmp(classname, STRING(MTRACE_class)) != 0 ) { jint cnum; int system_class; unsigned char *new_image; *************** *** 551,557 **** fatal_error("ERROR: Out of malloc memory\n"); } cp = gdata->classes + cnum; ! cp->name = (const char *)strdup(name); cp->calls = 0; cp->mcount = 0; cp->methods = NULL; --- 573,582 ---- fatal_error("ERROR: Out of malloc memory\n"); } cp = gdata->classes + cnum; ! cp->name = (const char *)strdup(classname); ! if ( cp->name == NULL ) { ! fatal_error("ERROR: Out of malloc memory\n"); ! } cp->calls = 0; cp->mcount = 0; cp->methods = NULL; *************** *** 570,576 **** /* Call the class file reader/write demo code */ java_crw_demo(cnum, ! name, class_data, class_data_len, system_class, --- 595,601 ---- /* Call the class file reader/write demo code */ java_crw_demo(cnum, ! classname, class_data, class_data_len, system_class, *************** *** 601,606 **** --- 626,632 ---- (void)free((void*)new_image); /* Free malloc() space with free() */ } } + (void)free((void*)classname); } } exit_critical_section(jvmti); } ###@###.### 2004-09-23
23-09-2004

EVALUATION Just needs a null pointer check in the classfile load hook. ###@###.### 2004-09-03 hprof may have a bit of a snag here. It wants to create an entry for a class in it's class table, and that has used it's name. It needed to do this before the classfile image is passed on to the java_crw_demo for BCI because it needed a unique ID for the injection. Without the classname, it would need to dig into the classfile and get it, a pain. I think hprof will still work ok, but without extensive testing, I'm not sure what it will mean to have an empty class name as it's search criteria. I wish this classname==NULL was illegal, it certainly isn't something I would want to allow forever. Regardless, I think when JVMTI is fixed and hprof starts getting NULl classnames that this bug will be fixed. Still needs testing. ###@###.### 2004-09-05
05-09-2004