On Mon, 3 Aug 2026 15:32:39 GMT, David CARLIER <[email protected]> wrote:
> In `vframeStreamCommon::skip_prefixed_method_and_wrappers()`, `prefix_len` is > a `size_t` computed as `prefixed_name_len - name_len`, so the `prefix_len <= > 0` guard only ever catches `prefix_len == 0`. When the next method on the > stack has a longer name than the prefixed native method, the subtraction > underflows, the guard passes, and `strcmp` reads at `prefixed_name + > prefix_len`, which has wrapped to a pointer before the string. The walk is > only reached when a JVMTI agent has registered a native method prefix with > `SetNativeMethodPrefix`, which several profilers do. > > The fix checks the lengths before subtracting. `name_len >= > prefixed_name_len` covers the `prefix_len == 0` case the old guard was meant > to catch, so nothing changes for inputs that were already handled correctly, > and `prefix_len` is now in `[1, prefixed_name_len)` for the comparisons that > follow. > > The test registers `wrapped_` as a prefix and declares a native > `wrapped_go()` next to a non-native `go()` with the same signature, so > resolving `wrapped_go()` strips the prefix and binds it to `go()`'s entry > point as a prefixed native. The native code then calls `RegisterNatives` with > no methods on a boot loader class, since that asks the VM for the calling > class at depth 1 and walks over the native frame, its wrapper and the > longer-named caller below. > > Note that the test does not fail on an unfixed VM: `strcmp` stops at the > first mismatching byte, so the over-read is a single byte inside the > resource-area chunk the string came from, which no sanitizer will flag. It > covers a path nothing else covers and locks in the guard. I confirmed the > underflow is reached by temporarily printing the lengths from the VM. > > Testing: > > - [x] `test/hotspot/jtreg:tier1`, linux-x86_64 fastdebug: 3583 passed, 7 > failed. The failures are `gc/TestTransparentHugePagesHeap.java` and > `runtime/os/TestTracePageSizes.java`, which fail on this machine because of > its transparent huge page settings and are unrelated to the change. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). Thank you for fixing this. The fix looks correct, there is a small nit related to naming convention of constant in the test. Seems that unlikely VM can crash reliable because of this issue eve with sanitizer. Most likely we are getting not matched string. The only one way to make reliable test that I see is to add logging of skipped wrappers and verify that they match expected methods. Something like ```log_info(class, load, cause)("Skipping native wrapper %s ," prefixed_name + prefix_len);``` Not sure that it is worth though. test/hotspot/jtreg/serviceability/jvmti/SetNativeMethodPrefix/libPrefixedNativeStackWalk.cpp line 30: > 28: extern "C" { > 29: > 30: static const char* const NativeMethodPrefix = "wrapped_"; Please use `NATIVE_METHOD_PREFIX` name which the standard choice for constants. ------------- Changes requested by lmesnik (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/32180#pullrequestreview-4848103100 PR Review Comment: https://git.openjdk.org/jdk/pull/32180#discussion_r3707300872
