From 2ed9d3525247a49ac9157e2826707a893b84f579 Mon Sep 17 00:00:00 2001 From: tajila Date: Mon, 4 Nov 2024 11:19:17 -0500 Subject: [PATCH] JFR: Prevent chunk buffer overflows Detect chunk buffer overflow and prevent subsequent writes to the buffer. Also, update chunk buffer size calculations. Skip frames with incomplete data in stacktraces. Add missing memory frees. Signed-off-by: tajila --- runtime/vm/BufferWriter.hpp | 165 ++++++++++++++++++---------- runtime/vm/JFRChunkWriter.hpp | 60 +++++----- runtime/vm/JFRConstantPoolTypes.cpp | 26 +++++ runtime/vm/JFRConstantPoolTypes.hpp | 16 ++- 4 files changed, 176 insertions(+), 91 deletions(-) diff --git a/runtime/vm/BufferWriter.hpp b/runtime/vm/BufferWriter.hpp index 990774985c5..76c79c6ec89 100644 --- a/runtime/vm/BufferWriter.hpp +++ b/runtime/vm/BufferWriter.hpp @@ -34,8 +34,9 @@ class VM_BufferWriter { U_8 *_buffer; U_8 *_cursor; UDATA _size; - + U_8 *_bufferEnd; U_8 *_maxCursor; + bool _overflow; #if defined(J9VM_ENV_LITTLE_ENDIAN) static const bool _isLE = true; @@ -94,8 +95,26 @@ class VM_BufferWriter { : _buffer(buffer) , _cursor(buffer) , _size(size) + , _bufferEnd(buffer + size) , _maxCursor(NULL) + , _overflow(false) + { + } + + bool + checkBounds(UDATA size) + { + if ((_cursor + size) >= _bufferEnd) { + _overflow = true; + } + + return !_overflow; + } + + bool + overflowOccurred() { + return _overflow; } U_64 @@ -123,50 +142,66 @@ class VM_BufferWriter { } void - writeU8(U_8 val) + writeU8NoCheck(U_8 val) { *_cursor = val; _cursor += sizeof(U_8); } + void + writeU8(U_8 val) + { + if (checkBounds(sizeof(U_8))) { + writeU8NoCheck(val); + } + } + void writeU16(U_16 val) { - U_16 newVal = val; - if (_isLE) { - newVal = byteSwap(val); + if (checkBounds(sizeof(U_16))) { + U_16 newVal = val; + if (_isLE) { + newVal = byteSwap(val); + } + *(U_16 *)_cursor = newVal; + _cursor += sizeof(U_16); } - *(U_16 *)_cursor = newVal; - _cursor += sizeof(U_16); } void writeU32(U_32 val) { - U_32 newVal = val; - if (_isLE) { - newVal = byteSwap(val); + if (checkBounds(sizeof(U_32))) { + U_32 newVal = val; + if (_isLE) { + newVal = byteSwap(val); + } + *(U_32 *)_cursor = newVal; + _cursor += sizeof(U_32); } - *(U_32 *)_cursor = newVal; - _cursor += sizeof(U_32); } void writeU64(U_64 val) { - U_64 newVal = val; - if (_isLE) { - newVal = byteSwap(val); + if (checkBounds(sizeof(U_64))) { + U_64 newVal = val; + if (_isLE) { + newVal = byteSwap(val); + } + *(U_64 *)_cursor = newVal; + _cursor += sizeof(U_64); } - *(U_64 *)_cursor = newVal; - _cursor += sizeof(U_64); } void writeData(U_8 *data, UDATA size) { - memcpy(_cursor, data, size); - _cursor += size; + if (checkBounds(size)) { + memcpy(_cursor, data, size); + _cursor += size; + } } U_8 * @@ -203,19 +238,21 @@ class VM_BufferWriter { void writeLEB128(U_64 val) { - U_64 newVal = val; - if (!_isLE) { - newVal = byteSwap(val); - } - do { - U_8 byte = newVal & 0x7F; - newVal >>= 7; - - if (newVal > 0) { - byte |= 0x80; + if (checkBounds(9)) { + U_64 newVal = val; + if (!_isLE) { + newVal = byteSwap(val); } - writeU8(byte); - } while (newVal > 0); + do { + U_8 byte = newVal & 0x7F; + newVal >>= 7; + + if (newVal > 0) { + byte |= 0x80; + } + writeU8NoCheck(byte); + } while (newVal > 0); + } } void @@ -230,19 +267,21 @@ class VM_BufferWriter { void writeLEB128PaddedU72(U_64 val) { - U_64 newVal = val; - if (!_isLE) { - newVal = byteSwap(val); + if (checkBounds(9)) { + U_64 newVal = val; + if (!_isLE) { + newVal = byteSwap(val); + } + writeU8NoCheck((newVal & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 7) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 14) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 21) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 28) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 35) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 42) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 49) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 56) & 0x7F)); } - writeU8((newVal & 0x7F) | 0x80); - writeU8(((newVal >> 7) & 0x7F) | 0x80); - writeU8(((newVal >> 14) & 0x7F) | 0x80); - writeU8(((newVal >> 21) & 0x7F) | 0x80); - writeU8(((newVal >> 28) & 0x7F) | 0x80); - writeU8(((newVal >> 35) & 0x7F) | 0x80); - writeU8(((newVal >> 42) & 0x7F) | 0x80); - writeU8(((newVal >> 49) & 0x7F) | 0x80); - writeU8(((newVal >> 56) & 0x7F)); } void @@ -257,18 +296,20 @@ class VM_BufferWriter { void writeLEB128PaddedU64(U_64 val) { - U_64 newVal = val; - if (!_isLE) { - newVal = byteSwap(val); + if (checkBounds(sizeof(U_64))) { + U_64 newVal = val; + if (!_isLE) { + newVal = byteSwap(val); + } + writeU8NoCheck((newVal & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 7) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 14) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 21) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 28) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 35) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 42) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 49) & 0x7F)); } - writeU8((newVal & 0x7F) | 0x80); - writeU8(((newVal >> 7) & 0x7F) | 0x80); - writeU8(((newVal >> 14) & 0x7F) | 0x80); - writeU8(((newVal >> 21) & 0x7F) | 0x80); - writeU8(((newVal >> 28) & 0x7F) | 0x80); - writeU8(((newVal >> 35) & 0x7F) | 0x80); - writeU8(((newVal >> 42) & 0x7F) | 0x80); - writeU8(((newVal >> 49) & 0x7F)); } void @@ -283,14 +324,16 @@ class VM_BufferWriter { void writeLEB128PaddedU32(U_32 val) { - U_64 newVal = val; - if (!_isLE) { - newVal = byteSwap(val); + if (checkBounds(sizeof(U_32))) { + U_64 newVal = val; + if (!_isLE) { + newVal = byteSwap(val); + } + writeU8NoCheck((newVal & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 7) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 14) & 0x7F) | 0x80); + writeU8NoCheck(((newVal >> 21) & 0x7F)); } - writeU8((newVal & 0x7F) | 0x80); - writeU8(((newVal >> 7) & 0x7F) | 0x80); - writeU8(((newVal >> 14) & 0x7F) | 0x80); - writeU8(((newVal >> 21) & 0x7F)); } static U_32 diff --git a/runtime/vm/JFRChunkWriter.hpp b/runtime/vm/JFRChunkWriter.hpp index e5e73b19af4..d968a07ec54 100644 --- a/runtime/vm/JFRChunkWriter.hpp +++ b/runtime/vm/JFRChunkWriter.hpp @@ -135,6 +135,7 @@ class VM_JFRChunkWriter { /* conservative sizing for JFR chunk */ static constexpr int STRING_HEADER_LENGTH = sizeof(U_64); static constexpr int CHECKPOINT_EVENT_HEADER_AND_FOOTER = 68; + static constexpr int STRING_CONSTANT_SIZE = 128; static constexpr int THREADSTATE_ENTRY_LENGTH = CHECKPOINT_EVENT_HEADER_AND_FOOTER + sizeof(threadStateNames) + (THREADSTATE_COUNT * STRING_HEADER_LENGTH); static constexpr int CLASS_ENTRY_ENTRY_SIZE = (5 * sizeof(U_64)) + sizeof(U_8); static constexpr int CLASSLOADER_ENTRY_SIZE = 3 * sizeof(U_64); @@ -150,6 +151,7 @@ class VM_JFRChunkWriter { static constexpr int THREAD_START_EVENT_SIZE = (6 * sizeof(U_64)) + sizeof(U_32); static constexpr int THREAD_END_EVENT_SIZE = (4 * sizeof(U_64)) + sizeof(U_32); static constexpr int THREAD_SLEEP_EVENT_SIZE = (7 * sizeof(U_64)) + sizeof(U_32); + static constexpr int MONITOR_WAIT_EVENT_SIZE = (9 * sizeof(U_64)) + sizeof(U_32); static constexpr int JVM_INFORMATION_EVENT_SIZE = 3000; static constexpr int PHYSICAL_MEMORY_EVENT_SIZE = (4 * sizeof(U_64)) + sizeof(U_32); static constexpr int VIRTUALIZATION_INFORMATION_EVENT_SIZE = 50; @@ -343,6 +345,10 @@ class VM_JFRChunkWriter { writeJFRHeader(); + if (_bufferWriter->overflowOccurred()) { + _buildResult = OutOfMemory; + } + if (isResultNotOKay()) { Trc_VM_jfr_ErrorWritingChunk(_currentThread, _buildResult); goto freeBuffer; @@ -592,53 +598,57 @@ class VM_JFRChunkWriter { UDATA calculateRequiredBufferSize() { - UDATA requireBufferSize = _constantPoolTypes.getRequiredBufferSize(); - requireBufferSize += JFR_CHUNK_HEADER_SIZE; + UDATA requiredBufferSize = _constantPoolTypes.getRequiredBufferSize(); + requiredBufferSize += JFR_CHUNK_HEADER_SIZE; + + requiredBufferSize += METADATA_HEADER_SIZE; + + requiredBufferSize += _vm->jfrState.metaDataBlobFileSize; - requireBufferSize += METADATA_HEADER_SIZE; + requiredBufferSize += THREADSTATE_ENTRY_LENGTH; - requireBufferSize += _vm->jfrState.metaDataBlobFileSize; + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getClassCount() * CLASS_ENTRY_ENTRY_SIZE)); - requireBufferSize += THREADSTATE_ENTRY_LENGTH; + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getClassloaderCount() * CLASSLOADER_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getClassCount() * CLASS_ENTRY_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + ((_constantPoolTypes.getPackageCount() + _constantPoolTypes.getStringUTF8Count()) * STRING_CONSTANT_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getClassloaderCount() * CLASSLOADER_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getPackageCount() * PACKAGE_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getPackageCount() * PACKAGE_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getMethodCount() * METHOD_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getMethodCount() * METHOD_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getThreadCount() * THREAD_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getThreadCount() * THREAD_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getThreadGroupCount() * THREADGROUP_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getThreadGroupCount() * THREADGROUP_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getModuleCount() * MODULE_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getModuleCount() * MODULE_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getStackTraceCount() * STACKTRACE_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getStackTraceCount() * STACKTRACE_ENTRY_SIZE); + requiredBufferSize += (CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getStackFrameCount() * STACKFRAME_ENTRY_SIZE)); - requireBufferSize += CHECKPOINT_EVENT_HEADER_AND_FOOTER + (_constantPoolTypes.getStackFrameCount() * STACKFRAME_ENTRY_SIZE); + requiredBufferSize += (_constantPoolTypes.getExecutionSampleCount() * EXECUTION_SAMPLE_EVENT_SIZE); - requireBufferSize += _constantPoolTypes.getExecutionSampleCount() * EXECUTION_SAMPLE_EVENT_SIZE; + requiredBufferSize += (_constantPoolTypes.getThreadStartCount() * THREAD_START_EVENT_SIZE); - requireBufferSize += _constantPoolTypes.getThreadStartCount() * THREAD_START_EVENT_SIZE; + requiredBufferSize += (_constantPoolTypes.getThreadEndCount() * THREAD_END_EVENT_SIZE); - requireBufferSize += _constantPoolTypes.getThreadEndCount() * THREAD_END_EVENT_SIZE; + requiredBufferSize += (_constantPoolTypes.getThreadSleepCount() * THREAD_SLEEP_EVENT_SIZE); - requireBufferSize += _constantPoolTypes.getThreadSleepCount() * THREAD_SLEEP_EVENT_SIZE; + requiredBufferSize += (_constantPoolTypes.getMonitorWaitCount() * MONITOR_WAIT_EVENT_SIZE); - requireBufferSize += JVM_INFORMATION_EVENT_SIZE; + requiredBufferSize += JVM_INFORMATION_EVENT_SIZE; - requireBufferSize += OS_INFORMATION_EVENT_SIZE; + requiredBufferSize += OS_INFORMATION_EVENT_SIZE; - requireBufferSize += PHYSICAL_MEMORY_EVENT_SIZE; + requiredBufferSize += PHYSICAL_MEMORY_EVENT_SIZE; - requireBufferSize += VIRTUALIZATION_INFORMATION_EVENT_SIZE; + requiredBufferSize += VIRTUALIZATION_INFORMATION_EVENT_SIZE; - requireBufferSize += CPU_INFORMATION_EVENT_SIZE; + requiredBufferSize += CPU_INFORMATION_EVENT_SIZE; - requireBufferSize += INITIAL_SYSTEM_PROPERTY_EVENT_SIZE; - return requireBufferSize; + requiredBufferSize += INITIAL_SYSTEM_PROPERTY_EVENT_SIZE; + return requiredBufferSize; } ~VM_JFRChunkWriter() diff --git a/runtime/vm/JFRConstantPoolTypes.cpp b/runtime/vm/JFRConstantPoolTypes.cpp index a1ecb03af47..a0a3ec28c34 100644 --- a/runtime/vm/JFRConstantPoolTypes.cpp +++ b/runtime/vm/JFRConstantPoolTypes.cpp @@ -1152,5 +1152,31 @@ VM_JFRConstantPoolTypes::freeStackStraceEntries(void *entry, void *userData) return FALSE; } +UDATA +VM_JFRConstantPoolTypes::freeThreadNameEntries(void *entry, void *userData) +{ + ThreadEntry *tableEntry = (ThreadEntry *) entry; + J9VMThread *currentThread = (J9VMThread *)userData; + PORT_ACCESS_FROM_VMC(currentThread); + + j9mem_free_memory(tableEntry->javaThreadName); + tableEntry->javaThreadName = NULL; + + return FALSE; +} + +UDATA +VM_JFRConstantPoolTypes::freeThreadGroupNameEntries(void *entry, void *userData) +{ + ThreadGroupEntry *tableEntry = (ThreadGroupEntry *) entry; + J9VMThread *currentThread = (J9VMThread *)userData; + PORT_ACCESS_FROM_VMC(currentThread); + + j9mem_free_memory(tableEntry->name); + tableEntry->name = NULL; + + return FALSE; +} + #endif /* defined(J9VM_OPT_JFR) */ diff --git a/runtime/vm/JFRConstantPoolTypes.hpp b/runtime/vm/JFRConstantPoolTypes.hpp index 11c13b18d2b..8a043eb0211 100644 --- a/runtime/vm/JFRConstantPoolTypes.hpp +++ b/runtime/vm/JFRConstantPoolTypes.hpp @@ -406,6 +406,10 @@ class VM_JFRConstantPoolTypes { static UDATA freeStackStraceEntries(void *entry, void *userData); + static UDATA freeThreadNameEntries(void *entry, void *userData); + + static UDATA freeThreadGroupNameEntries(void *entry, void *userData); + U_32 getMethodEntry(J9ROMMethod *romMethod, J9Class *ramClass); U_32 getClassEntry(J9Class *clazz); @@ -451,12 +455,8 @@ class VM_JFRConstantPoolTypes { VM_JFRConstantPoolTypes *cp = (VM_JFRConstantPoolTypes*) userData; StackFrame *frame = &cp->_currentStackFrameBuffer[cp->_currentFrameCount]; - cp->_currentFrameCount++; - if ((NULL == ramClass) || (NULL == romMethod)) { - /* unknown native method */ - frame->methodIndex = 0; - frame->frameType = Native; + goto skipFrame; } else { frame->methodIndex = cp->getMethodEntry(romMethod, ramClass); frame->frameType = Interpreted; /* TODO need a way to know if its JIT'ed and inlined */ @@ -474,6 +474,9 @@ class VM_JFRConstantPoolTypes { frame->lineNumber = lineNumber; } + cp->_currentFrameCount++; + +skipFrame: return J9_STACKWALK_KEEP_ITERATING; } @@ -1197,6 +1200,8 @@ class VM_JFRConstantPoolTypes { { hashTableForEachDo(_stringUTF8Table, &freeUTF8Strings, _currentThread); hashTableForEachDo(_stackTraceTable, &freeStackStraceEntries, _currentThread); + hashTableForEachDo(_threadTable, &freeThreadNameEntries, _currentThread); + hashTableForEachDo(_threadGroupTable, &freeThreadGroupNameEntries, _currentThread); hashTableFree(_classTable); hashTableFree(_packageTable); hashTableFree(_moduleTable); @@ -1210,6 +1215,7 @@ class VM_JFRConstantPoolTypes { pool_kill(_threadStartTable); pool_kill(_threadEndTable); pool_kill(_threadSleepTable); + pool_kill(_monitorWaitTable); j9mem_free_memory(_globalStringTable); }