[fault-injection] Unprotected access to JVM class loader data mutex and method list#678
Draft
zhengyu123 wants to merge 1 commit into
Draft
[fault-injection] Unprotected access to JVM class loader data mutex and method list#678zhengyu123 wants to merge 1 commit into
zhengyu123 wants to merge 1 commit into
Conversation
Contributor
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()intovmStructs.inline.hand introducedVMStructs::load_at_offset()to supportSafeAccess-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 forVMClassLoaderData::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.
Comment on lines
+631
to
+635
| 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); |
Comment on lines
+1169
to
+1179
| VMClassLoaderDataMutexLocker::VMClassLoaderDataMutexLocker(void* mutex) : _mutex(mutex) { | ||
| if (_mutex != nullptr) { | ||
| _lock_func(_mutex); | ||
| } | ||
| } | ||
|
|
||
| VMClassLoaderDataMutexLocker::~VMClassLoaderDataMutexLocker() { | ||
| if (_mutex != nullptr) { | ||
| _unlock_func(_mutex); | ||
| } | ||
| } |
Comment on lines
+1267
to
1270
| 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()); | ||
| } |
Comment on lines
+214
to
+221
| // 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); | ||
| } |
Contributor
CI Test ResultsRun: #29842352361 | Commit:
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. Summary: Total: 32 | Passed: 28 | Failed: 4 Updated: 2026-07-21 15:32:01 UTC |
|
🔄 Datadog auto-retried 3 jobs - 0 passed on retry 🔗 Commit SHA: 6b61672 | Docs | Datadog PR Page | Give us feedback! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.Unsure? Have a question? Request a review!