RFR: 8296024: Usage of DirectBuffer::address should be guarded [v18]

Per Minborg pminborg at openjdk.org
Tue Dec 6 10:06:28 UTC 2022


> This PR proposes the introduction of **guarding** of the use of `DirectBuffer::address` within the JDK. With the introduction of the Foreign Function and Memory access, it is possible to derive Buffer instances that are backed by native memory that, in turn, can be closed asynchronously by the user (and not only via a `Cleaner` when there is no other reference to the `Buffer` object). If another thread is invoking `MemorySession::close` while a thread is doing work using raw addresses, the outcome is undefined. This means the JVM might crash or even worse, silent modification of unrelated memory might occur. 
> 
> Design choices in this PR:
> 
> There is already a method `MemorySession::whileAlive` that takes a runnable and that will perform the provided action while acquiring the underlying` MemorySession` (if any). However, using this method entailed relatively large changes while converting larger/nested code segments into lambdas. This would also risk introducing lambda capturing. So, instead, a ~~try-with-resources~~ *try-finally* friendly access method was added. This made is more easy to add guarding and did not risk lambda capturing. Also, introducing lambdas in certain fundamental JDK classes might incur bootstrap problems.
> 
> The aforementioned ~~TwR~~ TF is using a ~~"session acquisition" that is not used explicitly in the try block itself~~ session used in the *finally* block. ~~This raises a warning that is suppressed using `@SuppressWarnings("try")`. In the future, we might be able to remove these suppressions by using the reserved variable name `_`.~~
> 
> In some cases, where is is guaranteed that the backing memory session is non-closeable, we do not have to guard the use of `DirectBuffer::address`. ~~These cases have been documented in the code.~~
> 
> On some occasions, a plurality of acquisitions are made. This would never lead to deadlocks as acquisitions are fundamentally concurrent counters and not resources that only one thread can "own".
> 
> I have added comments (and not javadocs) before the declaration of the non-public-api `DirectBuffer::address` method, that the use of the returned address needs to be guarded. It can be discussed if this is preferable or not.
> 
> This PR spawns several areas of responsibility and so, I expect more than one reviewer before promoting the PR.

Per Minborg has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 19 commits:

 - Merge master
 - Remove session exposure
 - Re-introduce yet another address vairable
 - Re-introduce address variables
 - Reformat and fix comment
 - Remove redundant method
 - Cleanup
 - Refactor to try-finally handling
 - Remove redundant guard and fix comment permanently
 - Rework Acquisition
 - ... and 9 more: https://git.openjdk.org/jdk/compare/a6139985...cd95bc86

-------------

Changes: https://git.openjdk.org/jdk/pull/11260/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=11260&range=17
  Stats: 809 lines in 24 files changed: 377 ins; 172 del; 260 mod
  Patch: https://git.openjdk.org/jdk/pull/11260.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/11260/head:pull/11260

PR: https://git.openjdk.org/jdk/pull/11260



More information about the security-dev mailing list