Integrated: JDK-8289633: Forbid raw C-heap allocation functions in hotspot and fix findings

[Thomas Stuefe]

On Sun, 3 Jul 2022 08:04:09 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

This pull request has now been integrated.

Changeset: 688712f7
Author:    Thomas Stuefe <stuefe at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/688712f75cd54caa264494adbe4dfeefc079e1dd
Stats:     79 lines in 20 files changed: 36 ins; 1 del; 42 mod

8289633: Forbid raw C-heap allocation functions in hotspot and fix findings

Reviewed-by: kbarrett, dholmes

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

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