graal icon indicating copy to clipboard operation
graal copied to clipboard

[GR-41270] MethodHandleNatives.resolve should not verify access if caller is null

Open galderz opened this issue 3 years ago • 1 comments

Describe the issue MethodHandles.Lookup.unreflectSetter fails with:

Caused by: java.lang.NoSuchMethodError: jdk.internal.misc.Unsafe.putReference(java.lang.Object, long, java.lang.Object)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.methodhandles.Util_java_lang_invoke_MethodHandleNatives.resolve(Target_java_lang_invoke_MethodHandleNatives.java:347)
	at [email protected]/java.lang.invoke.MethodHandleNatives.resolve(MethodHandleNatives.java:223)
	at [email protected]/java.lang.invoke.MemberName$Factory.resolve(MemberName.java:1085)
	at [email protected]/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1114)
	... 10 more

Method handles logic within GraalVM should be setting Unsafe.putReference for reflection access. Where would be the best place to add that?

However, even if you manually add the method for reflection access, you get:

Exception in thread "main" org.graalvm.compiler.debug.GraalError: java.lang.reflect.InvocationTargetException
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.methodhandles.Util_java_lang_invoke_MethodHandleNatives.verifyAccess(Target_java_lang_invoke_MethodHandleNatives.java:370)
	at [email protected]/java.lang.invoke.MethodHandleNatives.resolve(MethodHandleNatives.java:226)
	at [email protected]/java.lang.invoke.MemberName$Factory.resolve(MemberName.java:1085)
	at [email protected]/java.lang.invoke.MemberName$Factory.resolveOrFail(MemberName.java:1114)
	at [email protected]/java.lang.invoke.DirectMethodHandle.makePreparedFieldLambdaForm(DirectMethodHandle.java:775)
	at [email protected]/java.lang.invoke.DirectMethodHandle.preparedFieldLambdaForm(DirectMethodHandle.java:692)
	at [email protected]/java.lang.invoke.DirectMethodHandle.preparedFieldLambdaForm(DirectMethodHandle.java:680)
	at [email protected]/java.lang.invoke.DirectMethodHandle.make(DirectMethodHandle.java:113)
	at [email protected]/java.lang.invoke.DirectMethodHandle.make(DirectMethodHandle.java:129)
	at [email protected]/java.lang.invoke.MethodHandles$Lookup.getDirectFieldCommon(MethodHandles.java:4049)
	at [email protected]/java.lang.invoke.MethodHandles$Lookup.getDirectFieldNoSecurityManager(MethodHandles.java:4040)
	at [email protected]/java.lang.invoke.MethodHandles$Lookup.unreflectField(MethodHandles.java:3504)
	at [email protected]/java.lang.invoke.MethodHandles$Lookup.unreflectSetter(MethodHandles.java:3487)
	at org.example.methodhandles.MethodHandling.main(MethodHandling.java:17)
Caused by: java.lang.reflect.InvocationTargetException
	at [email protected]/java.lang.reflect.Method.invoke(Method.java:568)
	at org.graalvm.nativeimage.builder/com.oracle.svm.core.methodhandles.Util_java_lang_invoke_MethodHandleNatives.verifyAccess(Target_java_lang_invoke_MethodHandleNatives.java:368)
	... 13 more
Caused by: java.lang.NullPointerException
	at [email protected]/sun.invoke.util.VerifyAccess.isSamePackage(VerifyAccess.java:376)
	at [email protected]/sun.invoke.util.VerifyAccess.isClassAccessible(VerifyAccess.java:195)
	at [email protected]/sun.invoke.util.VerifyAccess.isMemberAc,essible(VerifyAccess.java:104)
	... 15 more

I don't think Util_java_lang_invoke_MethodHandleNatives.resolve is implementing things correctly. Looking at the JDK implementation of that method, it shows that verifying class access is only done when caller class is not null.

// void resolve(MemberName self, Class<?> caller)
JVM_ENTRY(jobject, MHN_resolve_Mem(JNIEnv *env, jobject igcls, jobject mname_jh, jclass caller_jh,
    jint lookup_mode, jboolean speculative_resolve)) {
  if (mname_jh == NULL) { THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), "mname is null"); }
  Handle mname(THREAD, JNIHandles::resolve_non_null(mname_jh));

  // The trusted Java code that calls this method should already have performed
  // access checks on behalf of the given caller.  But, we can verify this.
  // This only verifies from the context of the lookup class.  It does not
  // verify the lookup context for a Lookup object teleported from one module
  // to another. Such Lookup object can only access the intersection of the set
  // of accessible classes from both lookup class and previous lookup class.
  if (VerifyMethodHandles && (lookup_mode & LM_TRUSTED) == LM_TRUSTED && caller_jh != NULL &&
      java_lang_invoke_MemberName::clazz(mname()) != NULL) {
    Klass* reference_klass = java_lang_Class::as_Klass(java_lang_invoke_MemberName::clazz(mname()));
    if (reference_klass != NULL && reference_klass->is_objArray_klass()) {
      reference_klass = ObjArrayKlass::cast(reference_klass)->bottom_klass();
    }

    // Reflection::verify_class_access can only handle instance classes.
    if (reference_klass != NULL && reference_klass->is_instance_klass()) {
      // Emulate LinkResolver::check_klass_accessability.
      Klass* caller = java_lang_Class::as_Klass(JNIHandles::resolve_non_null(caller_jh));
      // access check on behalf of the caller if this is not a public lookup
      // i.e. lookup mode is not UNCONDITIONAL
      if ((lookup_mode & LM_UNCONDITIONAL) == 0
          && Reflection::verify_class_access(caller,
                                             InstanceKlass::cast(reference_klass),
                                             true) != Reflection::ACCESS_OK) {
        ResourceMark rm(THREAD);
        stringStream ss;
        ss.print("caller %s tried to access %s", caller->class_in_module_of_loader(),
                 reference_klass->class_in_module_of_loader());
        THROW_MSG_NULL(vmSymbols::java_lang_InternalError(), ss.as_string());
      }
    }
  }
...

Adding caller != null condition to the if statement before verifyAccess call appears to be enough to fix the issue. I'll be sending a PR shortly.

Steps to reproduce the issue Please include both build steps as well as run steps

  1. git clone --depth 1 https://github.com/galderz/mendrugo
  2. cd methodhandles
  3. make

Describe GraalVM and your environment:

  • GraalVM version 2921da019434
  • JDK major version: 17
  • OS: macOS
  • Architecture: ARM64

galderz avatar Aug 05 '22 10:08 galderz

Thank you for contributing to GraalVM, we'll review your PR shortly

oubidar-Abderrahim avatar Aug 12 '22 10:08 oubidar-Abderrahim