[jdk8u-dev] RFR: 8362208: [8u] Buffer overflow in g1GCPhaseTimes.cpp::LineBuffer::_buffer

[Paul Hohensee]

On Tue, 15 Jul 2025 06:47:42 GMT, SendaoYan <syan at openjdk.org> wrote:

> Hi all,
> 
> When running hotspot/test/gc/g1/TestG1TraceEagerReclaimHumongousObjects.java on a CPU with more than 200 physical threads, the jvm will crashes. The reason is that the testcase turn on the gc log, which prints the statistics of each gc thread. If the machine has more cores, more gc threads will be turned on (143 gc threads on a machine with 224 physical threads). In the G1GCParPhasePrinter::print_time_values function (hotspot/src/share/vm/gc_implementation/g1/g1GCPhaseTimes.cpp), the relevant statistics of all gc threads are concatenated into one line, and the string concatenation content is saved in the array defined by g1GCPhaseTimes.cpp::LineBuffer::_buffer. Therefore, on machines with a large number of physical threads, it is easy for the GC log output line length to exceed the predefined buffer size. When the buffer size is exceeded, an error occurs when calling the os::vsnprintf function.
> In JDK9, JDK-8150068 refactors the relevant GC log output, so buffer overflow will no longer occur. However, JDK-8150068 is a new feature, and JDK-8150068 cannot be directly backported to jdk8u. In addition, the amount of JDK-8150068 code is large, and the risk of backporting to jdk8u is also very high. Therefore, this PR changes the buffer length to 1024*3 to ensure that there will be no problems with GC log output in some scenarios, and leave a certain margin.
> 
> In addition, this PR adds a guarantee statement to ensure that an error is reported before calling os::vsnprintf when the buffer overflows, which is conducive to the rapid location of the problem
> 
> Change has been verified locally, risk is low.
> 
> Additional testing:
> 
> - [ ] jtreg tests include tier1/2/3 etc.. on linux-x64 with release build

Afaik a guarantee is supposed to check for JVM-ending situations, so it seems to strong to me just for losing some logging info. Even an assert is imo too strong. I'd turn the guarantee into a debug-only warning with something like "previous LineBuffer overflow, request ignored" and have it execute just once. I'd also gate the whole method on (_cur < BUFFER_LEN) so vnsprintf isn't called unnecessarily.

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

PR Review: https://git.openjdk.org/jdk8u-dev/pull/668#pullrequestreview-3025310666