RFR: JDK-8289633: Forbid raw C-heap allocation functions in hotspot and fix findings [v3]

[Kim Barrett]

On Mon, 4 Jul 2022 13:31:43 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> [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

Marked as reviewed by kbarrett (Reviewer).

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

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