On Tue, 4 Aug 2026 10:25:41 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).
>
> David CARLIER has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Apply feedback: fix stale method names in the test comment, drop the unused 
> JNINativeMethod array

Thank you for the update. Looks good to me.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/32180#pullrequestreview-4863448350

Reply via email to