> Found while auditing the JVMTI local-variable accessors.
> 
> `VM_BaseGetOrSetLocal` bounds-checks the requested slot before touching the 
> frame's `StackValueCollection`. For a `long`/`double` the value spans two 
> slots, so the check adds an `extra_slot` of 1: `_index + extra_slot >= 
> method->max_locals()`. When an agent passes `slot == INT_MAX`, `_index + 
> extra_slot` signed-overflows to `INT_MIN`, which is below `max_locals()`, so 
> the guard is bypassed and we go on to index `locals->at(INT_MAX)` — out of 
> bounds. That is an assertion failure in fastdebug and a SIGSEGV or silent 
> corruption in product.
> 
> Doing the arithmetic on the other side (`_index >= method->max_locals() - 
> extra_slot`) avoids the overflow. The same check appears in both 
> `check_slot_type_lvt` and `check_slot_type_no_lvt`, so both are fixed.
> 
> Additional testing:
>  - [x] Linux x86_64 server fastdebug, `serviceability/jvmti/GetLocalVariable`
>  - [ ] Regular testing pipelines
> 
> ---------
> - [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:

  8387718: JVMTI GetLocal/SetLocal: slot bounds check overflows for long/double 
slots
  
  VM_BaseGetOrSetLocal bounds-checks the requested slot before touching the
  frame's StackValueCollection. For a long/double the value spans two slots, so
  the check adds an extra_slot of 1: _index + extra_slot >= 
method->max_locals().
  When an agent passes slot == INT_MAX, _index + extra_slot signed-overflows to
  INT_MIN, which is below max_locals(), so the guard is bypassed and we go on to
  index locals->at(INT_MAX) -- out of bounds. That is an assertion failure in
  fastdebug and a SIGSEGV or silent corruption in product.
  
  Doing the arithmetic on the other side (_index >= method->max_locals() -
  extra_slot) avoids the overflow. The same check appears in both
  check_slot_type_lvt and check_slot_type_no_lvt, so both are fixed.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/31772/files
  - new: https://git.openjdk.org/jdk/pull/31772/files/b0df1bfd..ad50bb00

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=31772&range=01
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=31772&range=00-01

  Stats: 33 lines in 2 files changed: 4 ins; 13 del; 16 mod
  Patch: https://git.openjdk.org/jdk/pull/31772.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/31772/head:pull/31772

PR: https://git.openjdk.org/jdk/pull/31772

Reply via email to