[fault-injection] Unprotected access to JVM class loader data mutex and method list - #678
[fault-injection] Unprotected access to JVM class loader data mutex and method list#678zhengyu123 wants to merge 1 commit into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
This PR hardens HotSpot ClassLoaderData interactions for fault-injection scenarios by moving offset-based reads behind centralized helpers, adding an RAII mutex locker, and switching selected field reads to SafeAccess to avoid crashes outside longjmp-protected regions.
Changes:
- Refactored VMStructs::at() into vmStructs.inline.h and introduced VMStructs::load_at_offset() to support SafeAccess-based pointer loads.
- Added VMClassLoaderDataMutexLocker (RAII) and updated the JDK-8062116 workaround path to use it instead of manual lock/unlock.
- Added SafeAccess-protected accessors for VMClassLoaderData::mutex() / methodList() (but current API/usage is inconsistent and needs follow-up).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| ddprof-lib/src/main/cpp/hotspot/vmStructs.inline.h | Defines VMStructs::at()/load_at_offset() and adds SafeAccess-based ClassLoaderData field loads. |
| ddprof-lib/src/main/cpp/hotspot/vmStructs.h | Declares the new access helpers and introduces VMClassLoaderDataMutexLocker + updated ClassLoaderData accessor signatures. |
| ddprof-lib/src/main/cpp/hotspot/vmStructs.cpp | Implements VMClassLoaderDataMutexLocker constructor/destructor. |
| ddprof-lib/src/main/cpp/hotspot/hotspotSupport.cpp | Switches ClassLoaderData patching to RAII locking and updated method-list handling (currently does not type-check with new accessor signature). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
| class VMClassLoaderDataMutexLocker : protected VMStructs { | ||
| private: | ||
| void* mutex() { | ||
| return *(void**) at(sizeof(uintptr_t) * 3); | ||
| } | ||
|
|
||
| void* _mutex; | ||
| public: | ||
| void lock() { | ||
| _lock_func(mutex()); | ||
| } | ||
|
|
||
| void unlock() { | ||
| _unlock_func(mutex()); | ||
| } | ||
| VMClassLoaderDataMutexLocker(void* mutex); |
| VMClassLoaderDataMutexLocker::VMClassLoaderDataMutexLocker(void* mutex) : _mutex(mutex) { | ||
| if (_mutex != nullptr) { | ||
| _lock_func(_mutex); | ||
| } | ||
| } | ||
|
|
||
| VMClassLoaderDataMutexLocker::~VMClassLoaderDataMutexLocker() { | ||
| if (_mutex != nullptr) { | ||
| _unlock_func(_mutex); | ||
| } | ||
| } |
| VMClassLoaderDataMutexLocker locker(cld->mutex()); | ||
| for (int i = 0; i < method_count; i += MethodList::SIZE) { | ||
| *cld->methodList() = new MethodList(*cld->methodList()); | ||
| *cld->methodList() = new MethodList(cld->methodList()); | ||
| } |
| // ClassLoaderData accesses are unprotected | ||
| void* VMClassLoaderData::mutex() { | ||
| return load_at_offset<void*, true /*safe*/>(sizeof(uintptr_t) * 3); | ||
| } | ||
|
|
||
| MethodList* VMClassLoaderData::methodList() { | ||
| return load_at_offset<MethodList*, true /*safe*/>(sizeof(uintptr_t) * 6 + 8); | ||
| } |
CI Test ResultsRun: #29842352361 | Commit: 2610a3f | Duration: 14m 33s (longest job)
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Failed Testsmusl-amd64/debug / 8-librcaJob: View logs No detailed failure information available. Check the job logs. musl-aarch64/debug / 8-librcaJob: View logs No detailed failure information available. Check the job logs. glibc-amd64/debug / 8-orclJob: View logs No detailed failure information available. Check the job logs. glibc-amd64/debug / 8Job: View logs No detailed failure information available. Check the job logs. Summary: Total: 32 | Passed: 28 | Failed: 4 Updated: 2026-07-21 15:32:01 UTC |
Sorry, something went wrong.
|
🔄 Datadog auto-retried 3 jobs - 0 passed on retry 🔗 Commit SHA: 6b61672 | Docs | Datadog PR Page | Give us feedback! |
Sorry, something went wrong.
What does this PR do?:
Motivation:
Additional Notes:
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a security review (run the dd:platform-security-review
skill, or file a request via the PSEC review form).
bewaire also runs automatically on every PR.
Unsure? Have a question? Request a review!