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

[Paul Hohensee]

On Thu, 17 Jul 2025 07:30:33 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
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix bugs

Looks like the test on line 44 should be

_cur >= BUFFER_LEN

because _cur is set to BUFFER_LEN at line 52 as the sentinal.

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

Changes requested by phh (Reviewer).

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