Hello Phil,
Thank you for review of this fix. I also do not know what happens, if
NULL is passed as "jclass clazz" argument to "IsInstanceOf" function,
but it looks that nothing good, because the documentation, the URL to
which you provided, explicitly prohibits passing NULL as "clazz" argument.
Relatively to the fix, the line "1594 !env->IsInstanceOf(jOpposite,
windowCls)) {" will not be reached, if "windowCls" is NULL, because I
deliberately guard against this case by the line
1580 CHECK_NULL(windowCls);
Where the macro "CHECK_NULL" defined in the file
"src/java.base/share/native/libjava/jni_util.h" will just execute
"return;", if "windowCls" is NULL. This macro is already used several
times before the fix in the method "AwtWindow::SendWindowEvent(jint,
HWND, jint, jint)", therefore the possibility of returning from this
method is not new.
Thank you,
Anton
On 13/04/2020 20:22, Philip Race wrote:
1594 !env->IsInstanceOf(jOpposite, windowCls)) {
I am not entirely sure what this does if windowCls is NULL but the
docs don't mention any exceptions :-
https://docs.oracle.com/en/java/javase/14/docs/specs/jni/functions.html#isinstanceof
So I think it will just return false - and if windowCls is NULL we
have big problems anyay.
So +1
-phil.
On 4/10/20, 1:32 PM, Anton Litvinov wrote:
Hello,
Could you please review the following fix for the bug.
Bug: https://bugs.openjdk.java.net/browse/JDK-8242498
Webrev: http://cr.openjdk.java.net/~alitvinov/8242498/jdk15/webrev.00
The bug is the JVM crash, which occurs because a not existing method
is called on a Java object which is not an instance of the expected
Java class that has such a method. Such discrepancy of the expected
type and the type in runtime is possible, because the Java object,
whose field value is set to the instance of the not expected Java
class, is instantiated by AWT native code through JNI invocation.
Since JNI does not validate arguments passed to Java class
constructor and since AWT native code does not validate arguments
prior to invoking Java class constructor through JNI, such invalid
object is created.
REASON OF THE CRASH:
The fact that in the method
"java.awt.DefaultKeyboardFocusManager.dispatchEvent(AWTEvent)" in the
case "WindowEvent.WINDOW_LOST_FOCUS" of switch operator the variable
defined by the expression "Window oppositeWindow =
we.getOppositeWindow();" in runtime is instance of
"java.awt.Component" class instead of "java.awt.Window" class. The
crash occurs during attempt to call the method
"java.awt.Window.getTemporaryLostComponent()" on the object
"oppositeWindow" which in runtime is "Component" instead of the
expected "Window" object, and since the method
"getTemporaryLostComponent()" does not exist in "java.awt.Component"
class JVM cannot find this method and initiates the crash.
ROOT CAUSE OF THE BUG:
Transfer of the object of the incompatible type "java.awt.Component"
instead of an object of "java.awt.Window" type as "opposite" argument
to the constructor "TimedWindowEvent(Window source, int id, Window
opposite, int oldState, int newState, long time)" of the class
"sun.awt.TimedWindowEvent" through JNI invocation. This JNI
invocation occurs in the C++ class method
"AwtWindow::SendWindowEvent(jint, HWND, jint, jint)" in the file
"src/java.desktop/windows/native/libawt/windows/awt_Window.cpp". The
exact expression creating the instance of Java class
"TimedWindowEvent" with the invalid value of the field "opposite" is
following:
jobject event = env->NewObject(wClassEvent, wEventInitMID, target, id,
jOpposite, oldState, newState, ::JVM_CurrentTimeMillis(NULL,
0));
THE FIX:
The fix changes "AwtWindow::SendWindowEvent(jint, HWND, jint, jint)"
method in the file "awt_Window.cpp" to introduce the code which
verifies that the Java object "jOpposite" is really instance of the
class "java.awt.Window", and if it is not then the fix tries to get
Java object corresponding to parent window of the original "opposite"
HWND. And if this parent window object also is not instance of
"java.awt.Window" class, then NULL value is passed to the constructor
of "TimedWindowEvent" class.
Thank you,
Anton