On Mon, 4 Nov 2024 15:29:49 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> David Holmes has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Restore previous behaviour for zero length strings > > src/hotspot/share/utilities/exceptions.cpp line 264: > >> 262: // will be within reasonable limits - specifically we will never hit >> the INT_MAX limit >> 263: // of os::vsnprintf when it tries to report how big a buffer is needed. >> Even so we >> 264: // further limit the formatted output to 1024 characters. > > If we're chopping off the message at 1024 characters, why do we have to audit > the callers? Is it because vsnprintf will overflow before truncating the > message? Seems like it could be easy to add a caller that breaks this. > There's no way to check here? Thanks for looking at this @coleenp . Yes it is because vsnprintf will try to tell you how big a buffer you would actually need when you pass a smaller size buffer. There is no way to know how big the formatted message would be without actually expanding it - which of course is only done by vsnprintf. So this comes down to doing a basic audit like this and then remembering to pay attention with new code. The alternative would be to define some global max string size, like the 1024 used here, and then have every caller of `fthrow` use `%*s` passing that max. But that is extremely intrusive if you look at how we end up calling `fthrow`. We have code in the classfile parser/error code where one function creates the message with the `%s` and a lower function actually supplies the argument. It is doable, but it is a lot of code churn to put up a safety rail that this code can't run into anyway. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21867#discussion_r1830400893
