JDK-8258077 : Using -Xcheck:jni can lead to a double-free after JDK-8193234
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: openjdk8u272,11,16
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2020-12-11
  • Updated: 2022-05-06
  • Resolved: 2021-01-08
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 15 JDK 16 JDK 17
11.0.11-oracleFixed 15.0.8Fixed 16.0.1Fixed 17 b05Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Description
On 11/12/2020 4:01 am, Mauro Lacy wrote:

    Hello,

    I'm a contributor of the [jni-rs](https://github.com/jni-rs/jni-rs/) project. Rust bindings to the JNI.

    When running tests against OpenJDK with the flag "-Xcheck:jni" enabled, we detected the following: the behaviour of the `JNI_COMMIT` mode of `ReleasePrimitiveArrayCritical` seems to be inconsistent, when running with `-Xcheck:jni` enabled, vs. without `-Xcheck:jni`.

    If using "-Xcheck:jni" we're hitting a double free using `ReleasePrimitiveArrayCritical` with `JNI_COMMIT`, when we attempt to release the buffer at the end. It is our understanding that the behaviour of `ReleasePrimitiveArrayCritical` must be consistent, independently of whether the check JNI mode is enabled or not.

    You can check [https://github.com/jni-rs/jni-rs/issues/283](https://github.com/jni-rs/jni-rs/issues/283) for details / comments on this.

    We've tracked down the issue to this February 2019 commit: [openjdk commit 3e904a4](https://github.com/openjdk/jdk/commit/3e904a4801b2bf2e988ba096e5cb64a17fd5fce7). The related discussion is here: [openjdk bug JDK-8193234](https://bugs.openjdk.java.net/browse/JDK-8193234).

    As there are no associated tests or functionality in the OpenJDK code base, I've simply reverted the commit. Just in case, I've also built and tested the JDK, without issues.

    Not sure what tests are the ones mentioned in the discussions that led to the introduction of this behaviour. It is our understanding that tests of `JNI_COMMIT` functionality must still make a call to `ReleasePrimitiveArrayCritical` with a proper mode (`0` or `JNI_ABORT`) at the end, to release the buffer. And this, independently of `-Xcheck:jni` being enabled or not.

    If there are any issues related to this that we're not aware of, please let us know.

Comments
Fix request (15u) I'd like to backport this useful fix to 15u, too. tier-tested on several platforms.
06-05-2022

A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk15u-dev/pull/206 Date: 2022-05-06 09:43:22 +0000
06-05-2022

Fix Request (16u) Backporting this low-risk fix prevents this bug from occurring in JDK-16u. The original bug fix patch applied cleanly and included regression tests. After applying the patch to a JDK-16u repo, the fix was regression tested by running Mach5 tiers 1 and 2 on Linux, Windows, and Mac OS, and running tiers 3-5 on Linux x64.
24-02-2021

Fix Request Should get backported for parity with 11.0.11-oracle. Doesn't apply cleanly. Review thread: http://mail.openjdk.java.net/pipermail/jdk-updates-dev/2021-January/004741.html
26-01-2021

Changeset: 712014c5 Author: David Holmes <dholmes@openjdk.org> Date: 2021-01-08 04:11:22 +0000 URL: https://git.openjdk.java.net/jdk/commit/712014c5
08-01-2021

Actually the above failure mode is worse than would be expected. It seems that in trying to print more information about the guard failure we hit the SEGV and in turn that somehow leads to the malloc corruption error. If I get rid of the additional printing attempt then we see a more civilised: ReleasePrimitiveArrayCritical: release array failed bounds check, incorrect pointer returned ? array: 0x00007ff63009b850 carray: 0x00007ff628311960 FATAL ERROR in native method: ReleasePrimitiveArrayCritical: failed bounds check at TestCheckedReleaseArrayElements.fill(Native Method) at TestCheckedReleaseArrayElements.test(TestCheckedReleaseArrayElements.java:59) at TestCheckedReleaseArrayElements.main(TestCheckedReleaseArrayElements.java:105)
17-12-2020

I create a regression test for the non-Critical use of JNI_COMMIT. With the current code I see the reported "double-free" error: *** Error in `/scratch/users/daholme/jdk-dev2.git/build/linux-x64-debug-8258077/images/jdk/bin/java': malloc(): memory corruption: 0x00007f2c803118a0 *** ======= Backtrace: ========= ... ReleasePrimitiveArrayCritical: release array failed bounds check, incorrect pointer returned ? array: 0x00007f2c88378850 carray: 0x00007f2c80311890 GuardedMemory(0x00007f2c88378700) base_addr=0x00007f2c80311870 tag=0xabababababababab user_size=139829223518565 user_data=0x00007f2c80311890 Header guard @0x00007f2c80311870 is BROKEN # # A fatal error has been detected by the Java Runtime Environment: # # SIGSEGV (0xb) at pc=0x00007f2c8640726b, pid=15524, tid=15525 --- With the change for JDK-8193234 backed out the test passes. With a variant of the change for JDK-8193234 re-applied only to affect the Release*Critical API the test also passes.
17-12-2020

The "critical" array functions make it clear that if no copy is made then the mode is ignored. That implies that only a singular use of the Release function is permitted irrespective of the mode. JDK-8193234 was incorrect to affect both the regular Release<PrimitiveType>ArrayElements function and the ReleasePrimitiveArrayCritical function - it should only have affected the latter.
17-12-2020

ILW = MMM = P3
15-12-2020

Reading back through JDK-8193234 and the related discussions I think this comes down to the question of whether passing COMMIT mode ends the critical section for accessing the "critical primitive" array or not. If you believe it does and the commit mode is always ignored because hotspot never makes copy, and you just use COMMIT mode, then you will see the memory leak as reported in JDK-8193234. If you think it doesn't and so you need to either use mode 0, or else you call once with COMMIT mode (to do the write back) and again with another mode (to free the copy if it exists), then you will hit the double-free problem if you do the two calls. But the legality of two calls was itself questioned in that discussion because the spec states in relation to the Get<PrimitiveType>ArrayElements routines that: "The result is valid until the corresponding Release<PrimitiveType>ArrayElements() function is called." which doesn't account for calling release in COMMIT mode, doing something else, and then calling release with mode 0 or ABORT. Going back through things, and not withstanding the above quote from the spec, I'm more inclined today to think that the spec should have made it clear that multiple release calls are needed if you only COMMIT first. But we can't revert JDK-8193234 without reintroducing the leak for anyone using only COMMIT mode - which we've implicitly endorsed now. Trying to also allow the two release calls would likely require some not insignificant work to track if we had already done the free, but I will examine that in more detail.
11-12-2020