[GEODE-10521] Security: Eliminate reflection-based access to java.nio.Buffer internals #7956
+125
−99
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.
Summary
This PR eliminates reflection-based access to
java.nio.DirectByteBufferprivate APIs inAddressableMemoryManager, removing the requirement for the--add-opens=java.base/java.nio=ALL-UNNAMEDJVM flag. The implementation uses Unsafe field offset access instead of method reflection, achieving full Java module system compliance while maintaining zero-copy semantics and improving performance.Problem Statement
Current Implementation
The
AddressableMemoryManagerclass currently uses reflection to access private internal APIs ofjava.nio.DirectByteBuffer:DirectByteBuffer.address()method - to retrieve the native memory address from a DirectByteBufferDirectByteBuffer(long, int)constructor - to create a DirectByteBuffer wrapping a native memory addressThis reflection approach requires calling
setAccessible(true), which violates Java's module encapsulation rules introduced in Java 9 (JEP 260, 261).Required JVM Flag
To allow this reflection at runtime, Apache Geode currently requires:
This flag is hardcoded in
MemberJvmOptions.JAVA_11_OPTIONSand automatically added to all Geode member JVMs running on Java 11+.Why This Matters
Security and Compliance:
--add-opensusage as potential vulnerabilitiesForward Compatibility:
--add-opensmay become deprecated or restricted in future releasesOperational Impact:
Solution Overview
Key Insight
Field offset access via Unsafe does NOT require
--add-opensflags, while method reflection withsetAccessible()does. This fundamental difference enables the solution.Approach
Instead of using reflection to call private methods/constructors, we:
Unsafe.getLong()/putLong()using pre-computed field offsetsThis approach:
setAccessible(true)calls--add-opensflagsChanges
1. Enhanced Unsafe Wrapper (
geode-unsafe/src/main/java/.../Unsafe.java)Added cached field offsets (static initializer):
Added buffer field access methods:
Why this works without
--add-opens:Unsafe.objectFieldOffset()on a Field object doesn't require module accessUnsafe.getLong()/putLong()with an offset are direct memory operationssetAccessible(true)calls are involved2. Refactored AddressableMemoryManager (
geode-core/src/main/java/.../AddressableMemoryManager.java)Removed (99 lines deleted):
Constructor,Method,InvocationTargetException,MakeNotStaticdbbClass,dbbCtor,dbbAddressMethod,dbbCreateFailed,dbbAddressFailedBefore (reflection-based):
After (field offset-based):
Before (constructor reflection):
After (field manipulation):
Key improvements:
3. Removed JAVA_NIO_OPEN Flag (
geode-gfsh/src/main/java/.../MemberJvmOptions.java)Removed:
Updated JAVA_11_OPTIONS:
Removed unused import:
Testing
Unit Tests
All existing unit tests pass without modification:
OffHeapStoredObjectJUnitTest (69 tests)
Key test validations:
getDirectByteBufferShouldCreateAByteBuffer- Validates ByteBuffer creation and address mappingMemoryAllocatorJUnitTest
Validates:
Manual Testing
Validated the implementation maintains expected behavior:
getDirectByteBufferAddress()returns correct native memory addressescreateDirectByteBuffer()creates buffers wrapping specified memory addressesBenefits
Security and Compliance
--add-opensflag from security scan resultsOperational Excellence
Java Platform Compatibility
Developer Experience
Performance Improvements
invoke()overhead on every callModule System Compliance Initiative
This PR is part of a broader initiative to eliminate all JVM module system violations in Apache Geode:
Related Work
--add-opens=java.base/java.lang=ALL-UNNAMED(UnsafeThreadLocal) - COMPLETED--add-exports=java.base/sun.nio.ch=ALL-UNNAMED(DirectBuffer) - COMPLETED--add-opens=java.base/java.nio=ALL-UNNAMED(AddressableMemoryManager)Remaining Work
--add-opens=java.base/com.sun.management.internal=ALL-UNNAMED(VMStats50)Strategic Goal
Run Apache Geode on Java 17, and 21 without requiring any
--add-opensor--add-exportsflags, ensuring:Technical Details
Why Field Offset Access Doesn't Require --add-opens
The critical difference between the old and new approaches:
Method Reflection (requires
--add-opens):Field Offset Access (does NOT require
--add-opens):The module system restricts
setAccessible(true)on methods/constructors across modules, butUnsafe.objectFieldOffset()andUnsafe.getLong()/putLong()are native operations that bypass these checks.Zero-Copy Semantics Validation
The implementation maintains zero-copy behavior critical for off-heap memory performance:
Test Validation:
This confirms:
Off-Heap Memory Architecture Context
AddressableMemoryManageris a cornerstone of Geode's off-heap memory management:Purpose:
Performance Critical:
This refactoring maintains all performance characteristics while improving security and compatibility.
Files Changed
References
Checklist
--add-opensflags requiredBackward Compatibility
This change is fully backward compatible:
getDirectByteBufferAddress()andcreateDirectByteBuffer()signatures identical)Existing code using
AddressableMemoryManagerrequires no modifications.For all changes, please confirm:
develop)?gradlew buildrun cleanly?