8152335: Improve MethodHandle consistency
Reviewed-by: acorn, ahgross, jrose
Contributed-by: vladimir.x.ivanov@oracle.com, michael.haupt@oracle.com
--- a/jdk/src/java.base/share/classes/java/lang/ClassLoader.java Fri Jan 22 13:27:09 2016 +0100
+++ b/jdk/src/java.base/share/classes/java/lang/ClassLoader.java Fri Mar 18 18:07:55 2016 -0700
@@ -817,6 +817,9 @@
if (!checkName(name))
throw new NoClassDefFoundError("IllegalName: " + name);
+ // Note: Checking logic in java.lang.invoke.MemberName.checkForTypeAlias
+ // relies on the fact that spoofing is impossible if a class has a name
+ // of the form "java.*"
if ((name != null) && name.startsWith("java.")
&& this != getBuiltinPlatformClassLoader()) {
throw new SecurityException
--- a/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java Fri Jan 22 13:27:09 2016 +0100
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/MemberName.java Fri Mar 18 18:07:55 2016 -0700
@@ -827,7 +827,7 @@
assert(isResolved() == isResolved);
}
- void checkForTypeAlias() {
+ void checkForTypeAlias(Class<?> refc) {
if (isInvocable()) {
MethodType type;
if (this.type instanceof MethodType)
@@ -835,16 +835,16 @@
else
this.type = type = getMethodType();
if (type.erase() == type) return;
- if (VerifyAccess.isTypeVisible(type, clazz)) return;
- throw new LinkageError("bad method type alias: "+type+" not visible from "+clazz);
+ if (VerifyAccess.isTypeVisible(type, refc)) return;
+ throw new LinkageError("bad method type alias: "+type+" not visible from "+refc);
} else {
Class<?> type;
if (this.type instanceof Class<?>)
type = (Class<?>) this.type;
else
this.type = type = getFieldType();
- if (VerifyAccess.isTypeVisible(type, clazz)) return;
- throw new LinkageError("bad field type alias: "+type+" not visible from "+clazz);
+ if (VerifyAccess.isTypeVisible(type, refc)) return;
+ throw new LinkageError("bad field type alias: "+type+" not visible from "+refc);
}
}
@@ -1016,10 +1016,25 @@
MemberName m = ref.clone(); // JVM will side-effect the ref
assert(refKind == m.getReferenceKind());
try {
+ // There are 4 entities in play here:
+ // * LC: lookupClass
+ // * REFC: symbolic reference class (MN.clazz before resolution);
+ // * DEFC: resolved method holder (MN.clazz after resolution);
+ // * PTYPES: parameter types (MN.type)
+ //
+ // What we care about when resolving a MemberName is consistency between DEFC and PTYPES.
+ // We do type alias (TA) checks on DEFC to ensure that. DEFC is not known until the JVM
+ // finishes the resolution, so do TA checks right after MHN.resolve() is over.
+ //
+ // All parameters passed by a caller are checked against MH type (PTYPES) on every invocation,
+ // so it is safe to call a MH from any context.
+ //
+ // REFC view on PTYPES doesn't matter, since it is used only as a starting point for resolution and doesn't
+ // participate in method selection.
m = MethodHandleNatives.resolve(m, lookupClass);
- m.checkForTypeAlias();
+ m.checkForTypeAlias(m.getDeclaringClass());
m.resolution = null;
- } catch (LinkageError ex) {
+ } catch (ClassNotFoundException | LinkageError ex) {
// JVM reports that the "bytecode behavior" would get an error
assert(!m.isResolved());
m.resolution = ex;
--- a/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java Fri Jan 22 13:27:09 2016 +0100
+++ b/jdk/src/java.base/share/classes/java/lang/invoke/MethodHandleNatives.java Fri Mar 18 18:07:55 2016 -0700
@@ -49,7 +49,7 @@
static native void init(MemberName self, Object ref);
static native void expand(MemberName self);
- static native MemberName resolve(MemberName self, Class<?> caller) throws LinkageError;
+ static native MemberName resolve(MemberName self, Class<?> caller) throws LinkageError, ClassNotFoundException;
static native int getMembers(Class<?> defc, String matchName, String matchSig,
int matchFlags, Class<?> caller, int skip, MemberName[] results);
--- a/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java Fri Jan 22 13:27:09 2016 +0100
+++ b/jdk/src/java.base/share/classes/sun/invoke/util/VerifyAccess.java Fri Mar 18 18:07:55 2016 -0700
@@ -231,22 +231,66 @@
* @param refc the class attempting to make the reference
*/
public static boolean isTypeVisible(Class<?> type, Class<?> refc) {
- if (type == refc) return true; // easy check
+ if (type == refc) {
+ return true; // easy check
+ }
while (type.isArray()) type = type.getComponentType();
- if (type.isPrimitive() || type == Object.class) return true;
- ClassLoader parent = type.getClassLoader();
- if (parent == null) return true;
- ClassLoader child = refc.getClassLoader();
- if (child == null) return false;
- if (parent == child || loadersAreRelated(parent, child, true))
+ if (type.isPrimitive() || type == Object.class) {
return true;
- // Do it the hard way: Look up the type name from the refc loader.
- try {
- Class<?> res = child.loadClass(type.getName());
- return (type == res);
- } catch (ClassNotFoundException ex) {
+ }
+ ClassLoader typeLoader = type.getClassLoader();
+ ClassLoader refcLoader = refc.getClassLoader();
+ if (typeLoader == refcLoader) {
+ return true;
+ }
+ if (refcLoader == null && typeLoader != null) {
return false;
}
+ if (typeLoader == null && type.getName().startsWith("java.")) {
+ // Note: The API for actually loading classes, ClassLoader.defineClass,
+ // guarantees that classes with names beginning "java." cannot be aliased,
+ // because class loaders cannot load them directly.
+ return true;
+ }
+
+ // Do it the hard way: Look up the type name from the refc loader.
+ //
+ // Force the refc loader to report and commit to a particular binding for this type name (type.getName()).
+ //
+ // In principle, this query might force the loader to load some unrelated class,
+ // which would cause this query to fail (and the original caller to give up).
+ // This would be wasted effort, but it is expected to be very rare, occurring
+ // only when an attacker is attempting to create a type alias.
+ // In the normal case, one class loader will simply delegate to the other,
+ // and the same type will be visible through both, with no extra loading.
+ //
+ // It is important to go through Class.forName instead of ClassLoader.loadClass
+ // because Class.forName goes through the JVM system dictionary, which records
+ // the class lookup once for all. This means that even if a not-well-behaved class loader
+ // would "change its mind" about the meaning of the name, the Class.forName request
+ // will use the result cached in the JVM system dictionary. Note that the JVM system dictionary
+ // will record the first successful result. Unsuccessful results are not stored.
+ //
+ // We use doPrivileged in order to allow an unprivileged caller to ask an arbitrary
+ // class loader about the binding of the proposed name (type.getName()).
+ // The looked up type ("res") is compared for equality against the proposed
+ // type ("type") and then is discarded. Thus, the worst that can happen to
+ // the "child" class loader is that it is bothered to load and report a class
+ // that differs from "type"; this happens once due to JVM system dictionary
+ // memoization. And the caller never gets to look at the alternate type binding
+ // ("res"), whether it exists or not.
+ final String name = type.getName();
+ Class<?> res = java.security.AccessController.doPrivileged(
+ new java.security.PrivilegedAction<>() {
+ public Class<?> run() {
+ try {
+ return Class.forName(name, false, refcLoader);
+ } catch (ClassNotFoundException | LinkageError e) {
+ return null; // Assume the class is not found
+ }
+ }
+ });
+ return (type == res);
}
/**