RFR: JDK-8289633: Forbid raw C-heap allocation functions in hotspot and fix findings
Thomas Stuefe
stuefe at openjdk.org
Sun Jul 3 10:45:24 UTC 2022
[JDK-8214976](https://bugs.openjdk.org/browse/JDK-8214976) introduced a way to forbid functions from being called outside of explicitly allowed contexts. Kim [1] proposed to use that functionality to forbid raw malloc and friends. That would have prevented [JDK-8289477](https://bugs.openjdk.org/browse/JDK-8289477), which sneaked in raw malloc and free via C-runtime defined macros.
We forbid now all functions that return C-heap, even if that is only optional, like with `realpath`. Note that there may be more functions, but these are all I know from the top of my head. We forbid them even if they are exotic to prevent devs from using them in the future and also from creep-in via system macros.
I found a number of places where raw allocation functions were used, mostly strdup. I either changes those places to use os::xxx where I was confident that works, or where I saw we really must use the raw functions I marked them with ALLOW_C_FUNCTION.
Places that allow raw C functions:
- decoder on Linux, since the C++ demangler returns raw C heap
- realpath, in conjunction with allowing real free for the returned buffer
- ZGC uses posix_memalign for a static global buffer that never is deleted. Keeping to use posix_memalign is probably ok, but we should add an os::posix_memalign at some point
- UL, LogTagSet, since UL may also be used for logging inside NMT and we don't want circularities
- obviously os::malloc and friends
- NMT pre-initialization code because circularities
- In gtest main function - I think gtest should work always, even if os::malloc is broken.
Places I fixed:
- ZGC, mountpoint string handling
- In CompilerEvent we hold a global lookup table with phase names. The names in there leak, but this table never gets cleared, so I think that's okay
- gcLogPrecious, string is fed to VMError::report_and_die, so it probably does not matter
- there were several places in JVMCI, one where we ::strdup a string which we give to a new code blob as blob name. These strings actually leak. I opened https://bugs.openjdk.org/browse/JDK-8289632 to track this
- A couple of places in gtests.
Note, wherever I introduced os::xxx and had to add os.hpp, I commented the include with "//malloc" to earmark those in case we ever want to move os::malloc and friends into its own header.
----
Tests: I build and ran gtests manually on x64 fastdebug, release, arm fastdebug, aarch64 fastdebug, x86 fastdebug (all Linux). I also tested build on Alpine x64.
GHAs are in work.
[1] https://mail.openjdk.org/pipermail/hotspot-dev/2022-July/061602.html
-------------
Commit messages:
- explicitly add globalDefinitions to os_linux.cpp
- Fix ppcle
- JDK-8289633-Forbid-raw-C-heap-allocation-functions-in-hotspot-and-fix-findings
Changes: https://git.openjdk.org/jdk/pull/9356/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9356&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8289633
Stats: 81 lines in 20 files changed: 38 ins; 1 del; 42 mod
Patch: https://git.openjdk.org/jdk/pull/9356.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/9356/head:pull/9356
PR: https://git.openjdk.org/jdk/pull/9356
More information about the hotspot-dev
mailing list