RFR: JDK-8289633: Forbid raw C-heap allocation functions in hotspot and fix findings [v3]
Thomas Stuefe
stuefe at openjdk.org
Mon Jul 4 13:31:43 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
Thomas Stuefe has updated the pull request incrementally with one additional commit since the last revision:
Use our realpath() wrapper in os_perf_linux.cpp
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/9356/files
- new: https://git.openjdk.org/jdk/pull/9356/files/b93ca75f..f304868d
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=9356&range=02
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=9356&range=01-02
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 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