RFR: 8284877: Check type compatibility before looking up method from receiver's vtable

[Yi Yang]

On Wed, 20 Apr 2022 23:57:30 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Hi, can I have a review for this enhancement? This patch adds type compatibility check before method lookup for robustness. In some internal cases, serialization framework may improperly generate an object of wrong type, which leads JVM randomly crashes during method resolution.
>> 
>> For example:
>> 
>> invokevirtual selected method: receiver-class:java.util.ArrayList, resolved-class:com.taobao.forest.domain.util.LongMapSupportArrayList, resolved_method:com.taobao.forest.domain.util.LongMapSupportArrayList.toMap()Ljava/util/Map;, selected_method:0x458, vtable_index:56#
>> 
>> The real type of receiver is ArrayList, while the resolved method is LongMapSupportArrayList.toMap. VM attempts to select method as if looking up from receiver's vtable via vtable index of resolved method(i.e. attempts to lookup `toMap()` from 
>>  ArrayList), an invalid method or incorrect method would be selected, thus causing some strange crashes.
>> 
>> I think it's reasonable to add a type compatibility check before method lookup. If such an incompatible call is found, JVM could throw an exception instead.
>
> If Unsafe is being used to circumvent the type system, then we should not be adding additional checks in the VM to catch that later. If you use Unsafe directly and you do it wrong then you can crash the VM - that is not a VM problem to fix.

> @dholmes-ora From my understanding, the bytecode is correct, but the third‑party serialisation library ends up incorrectly deserialising the `LongMapSupportArrayList` instance as an `ArrayList`, and then uses `Unsafe.setObject` to store it in the `longValues` field, which is typed as `LongMapSupportArrayList`.
> 
> Then, when the `getValuesMap()` method calls `this.longValues.toMap()`, the JVM assumes that since `longValues` is typed as `LongMapSupportArrayList`, the `LongMapSupportArrayList.toMap:()Ljava/util/Map;` method will always exist, but since the `longValues` field now contains a `java.util.ArrayList` (which doesn’t have the `toMap()` method), the `vtable` index is invalid, which leads to a VM crash.

Exactly right.

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

PR: https://git.openjdk.java.net/jdk/pull/8241