diff --git a/ddprof-lib/src/main/cpp/flightRecorder.cpp b/ddprof-lib/src/main/cpp/flightRecorder.cpp index eeaa3f32d..6bae34f59 100644 --- a/ddprof-lib/src/main/cpp/flightRecorder.cpp +++ b/ddprof-lib/src/main/cpp/flightRecorder.cpp @@ -176,14 +176,18 @@ void Lookup::fillJavaMethodInfo(MethodInfo *mi, jmethodID method, bool entry = false; if (VMMethod::check_jmethodID(method) && jvmti->GetMethodDeclaringClass(method, &method_class) == 0 && + // GetMethodDeclaringClass may return a jclass wrapping a stale/garbage oop when the class was + // unloaded between sample capture and dump (TOCTOU race with class unloading). Guard against + // null handles before calling GetClassSignature. + method_class != NULL && // On some older versions of J9, the JVMTI call to GetMethodDeclaringClass will return OK = 0, but when a // classloader is unloaded they free all JNIIDs. This means that anyone holding on to a jmethodID is // pointing to corrupt data and the behaviour is undefined. // The behaviour is adjusted so that when asgct() is used or if `-XX:+KeepJNIIDs` is specified, // when a classloader is unloaded, the jmethodIDs are not freed, but instead marked as -1. - // The nested check below is to mitigate these crashes. - // In more recent versions, the condition above will short-circuit safely. - ((!VM::isOpenJ9() || method_class != reinterpret_cast(-1)) && jvmti->GetClassSignature(method_class, &class_name, NULL) == 0) && + // The check below mitigates these crashes on J9. + (!VM::isOpenJ9() || method_class != reinterpret_cast(-1)) && + jvmti->GetClassSignature(method_class, &class_name, NULL) == 0 && jvmti->GetMethodName(method, &method_name, &method_sig, NULL) == 0) { if (first_time) { diff --git a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp index b3d047b27..52a91ca21 100644 --- a/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp +++ b/ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp @@ -969,6 +969,12 @@ bool VMMethod::check_jmethodID_hotspot(jmethodID id) { } } if (VMStructs::class_loader_data_offset() >= 0) { + // Verify the Klass at cpool_holder is readable over the full range we're about to access, + // catching freed/reclaimed Klass memory between check_jmethodID and the JVMTI calls (PROF-14003). + if (!SafeAccess::isReadableRange(cpool_holder, + (size_t)VMStructs::class_loader_data_offset() + sizeof(void*))) { + return false; + } cl_data = (const char *)SafeAccess::load( (void **)(cpool_holder + VMStructs::class_loader_data_offset())); if (cl_data == NULL) {