On 9/15/16 12:10 PM, harold seigel wrote:
(Adding hotspot-runtime)
Hi Dan,
Thanks for looking at this.
I could pass NULL instead of clazz to ToReflectMethod() to ensure that
the method object isn't being obtained from clazz.
I don't think that would be a JNI spec compliant use of the
JNI ToReflectedMethod() function. That would be relying on
the fact that HotSpot doesn't use the clazz parameter to
convert {clazz,jmethodID} => method_object.
Sorry... again...
Dan
Harold
On 9/15/2016 1:09 PM, Daniel D. Daugherty wrote:
On 9/15/16 9:31 AM, harold seigel wrote:
Hi,
Please review this small fix for JDK-8160987. The JDWP
InvokeStatic() method was depending on the JNI function that it
called to enforce the requirement that the specified method must be
a member of the specified class or one of its super classes. But,
JNI does not enforce this requirement. This fix adds code to JDWP
to do its own check that the specified method is a member of the
specified class or one of its super classes.
JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8160987
Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8160987/
src/jdk.jdwp.agent/share/native/libjdwp/invoker.c
Sorry I didn't think of this comment during the pre-review...
The only "strange" part of this fix is:
L374: /* Get the method object from the method's jmethodID. */
L375: method_object = JNI_FUNC_PTR(env,ToReflectedMethod)(env,
L376: clazz,
L377: method,
L378: JNI_TRUE /* isStatic */);
L379: if (method_object == NULL) {
L380: return JVMTI_ERROR_NONE; /* Bad jmethodID ? This
will be handled elsewhere */
L381: }
Here we are using parameter 'clazz' to find the method_object for
parameter 'method' so that we can validate that 'clazz' refers to
method's class or superclass.
When a bogus 'clazz' value is passed in by a JCK test, the only
reason that JNI ToReflectedMethod() can still find the right
method_object is that our (HotSpot) implementation of JNI
ToReflectedMethod() doesn't really require the 'clazz' parameter
to find the right method_object. So the 'method_object' that we
return is the real one which has a 'clazz' field that doesn't
match the 'clazz' parameter.
Wow does that twist your head around or what?
So we're trusting JNI ToReflectedMethod() to return the right
method_object when we give it a potentially bad 'clazz' value.
So should we use JNI FromReflectedMethod() to convert the
method_object back into a jmethodID and verify that jmethodID
matches the one that we passed to check_methodClass()?
I might be too paranoid here so feel free to say that enough is
enough with this fix.
Thumbs up!
Dan
The fix was tested with the two failing JCK vm/jdwp tests listed in
the bug, the JCK Lang, VM, and API tests, the hotspot JTReg tests,
the java/lang, java/util and other JTReg tests, the co-located and
non-colocated NSK tests, and with the RBT Tier2 tests.
Thanks, Harold