From fb96db84c819b78e69a83f0b358f037e65704998 Mon Sep 17 00:00:00 2001 From: mchung Date: Fri, 24 Aug 2012 22:55:49 -0700 Subject: [PATCH] 7193339: Prepare system classes be defined by a non-null module loader Reviewed-by: alanb, dholmes, dsamersoff, sspitsyn, psandoz --- .../sun/jmx/mbeanserver/MXBeanMapping.java | 2 +- .../sun/jmx/remote/internal/IIOPHelper.java | 3 +- src/share/classes/java/lang/Class.java | 8 ++--- src/share/classes/java/lang/ClassLoader.java | 21 ++++++++++-- src/share/classes/java/lang/Thread.java | 3 +- .../lang/management/ManagementFactory.java | 12 +++---- .../lang/management/PlatformComponent.java | 3 +- .../classes/java/util/prefs/Preferences.java | 3 +- .../javax/script/ScriptEngineManager.java | 2 +- .../sun/management/MappedMXBeanType.java | 2 +- src/share/classes/sun/misc/Unsafe.java | 2 +- src/share/classes/sun/misc/VM.java | 8 +++++ .../classes/sun/reflect/misc/ReflectUtil.java | 34 +++++++++++++++++++ 13 files changed, 82 insertions(+), 21 deletions(-) diff --git a/src/share/classes/com/sun/jmx/mbeanserver/MXBeanMapping.java b/src/share/classes/com/sun/jmx/mbeanserver/MXBeanMapping.java index 6fa3058b4..93ca2ea80 100644 --- a/src/share/classes/com/sun/jmx/mbeanserver/MXBeanMapping.java +++ b/src/share/classes/com/sun/jmx/mbeanserver/MXBeanMapping.java @@ -169,7 +169,7 @@ public abstract class MXBeanMapping { return (Class) javaType; try { String className = openType.getClassName(); - return Class.forName(className, false, null); + return Class.forName(className, false, MXBeanMapping.class.getClassLoader()); } catch (ClassNotFoundException e) { throw new RuntimeException(e); // should not happen } diff --git a/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java b/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java index 9ee9152c4..11d4f130c 100644 --- a/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java +++ b/src/share/classes/com/sun/jmx/remote/internal/IIOPHelper.java @@ -52,7 +52,8 @@ public final class IIOPHelper { AccessController.doPrivileged(new PrivilegedAction() { public IIOPProxy run() { try { - Class c = Class.forName(IMPL_CLASS, true, null); + Class c = Class.forName(IMPL_CLASS, true, + IIOPHelper.class.getClassLoader()); return (IIOPProxy)c.newInstance(); } catch (ClassNotFoundException cnf) { return null; diff --git a/src/share/classes/java/lang/Class.java b/src/share/classes/java/lang/Class.java index b636f5ade..267d7d457 100644 --- a/src/share/classes/java/lang/Class.java +++ b/src/share/classes/java/lang/Class.java @@ -228,7 +228,8 @@ public final * ensure it's ok to access the bootstrap class loader. * * @param name fully qualified name of the desired class - * @param initialize whether the class must be initialized + * @param initialize if {@code true} the class will be initialized. + * See Section 12.4 of The Java Language Specification. * @param loader class loader from which the class must be loaded * @return class object representing the desired class * @@ -605,7 +606,7 @@ public final SecurityManager sm = System.getSecurityManager(); if (sm != null) { ClassLoader ccl = ClassLoader.getCallerClassLoader(); - if (ccl != null && ccl != cl && !cl.isAncestor(ccl)) { + if (ClassLoader.needsClassLoaderPermissionCheck(ccl, cl)) { sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION); } } @@ -2170,8 +2171,7 @@ public final if (s != null) { s.checkMemberAccess(this, which); ClassLoader cl = getClassLoader0(); - if ((ccl != null) && (ccl != cl) && - ((cl == null) || !cl.isAncestor(ccl))) { + if (sun.reflect.misc.ReflectUtil.needsPackageAccessCheck(ccl, cl)) { String name = this.getName(); int i = name.lastIndexOf('.'); if (i != -1) { diff --git a/src/share/classes/java/lang/ClassLoader.java b/src/share/classes/java/lang/ClassLoader.java index 8f73b05f2..e62c69a1c 100644 --- a/src/share/classes/java/lang/ClassLoader.java +++ b/src/share/classes/java/lang/ClassLoader.java @@ -1403,7 +1403,7 @@ public abstract class ClassLoader { SecurityManager sm = System.getSecurityManager(); if (sm != null) { ClassLoader ccl = getCallerClassLoader(); - if (ccl != null && !isAncestor(ccl)) { + if (needsClassLoaderPermissionCheck(ccl, this)) { sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION); } } @@ -1473,7 +1473,7 @@ public abstract class ClassLoader { SecurityManager sm = System.getSecurityManager(); if (sm != null) { ClassLoader ccl = getCallerClassLoader(); - if (ccl != null && ccl != scl && !scl.isAncestor(ccl)) { + if (needsClassLoaderPermissionCheck(ccl, scl)) { sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION); } } @@ -1523,6 +1523,23 @@ public abstract class ClassLoader { return false; } + // Tests if class loader access requires "getClassLoader" permission + // check. A class loader 'from' can access class loader 'to' if + // class loader 'from' is same as class loader 'to' or an ancestor + // of 'to'. The class loader in a system domain can access + // any class loader. + static boolean needsClassLoaderPermissionCheck(ClassLoader from, + ClassLoader to) + { + if (from == to) + return false; + + if (from == null) + return false; + + return !to.isAncestor(from); + } + // Returns the invoker's class loader, or null if none. // NOTE: This must always be invoked when there is exactly one intervening // frame from the core libraries on the stack between this method's diff --git a/src/share/classes/java/lang/Thread.java b/src/share/classes/java/lang/Thread.java index 474dbfc73..452f1fdde 100644 --- a/src/share/classes/java/lang/Thread.java +++ b/src/share/classes/java/lang/Thread.java @@ -1449,8 +1449,7 @@ class Thread implements Runnable { SecurityManager sm = System.getSecurityManager(); if (sm != null) { ClassLoader ccl = ClassLoader.getCallerClassLoader(); - if (ccl != null && ccl != contextClassLoader && - !contextClassLoader.isAncestor(ccl)) { + if (ClassLoader.needsClassLoaderPermissionCheck(ccl, contextClassLoader)) { sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION); } } diff --git a/src/share/classes/java/lang/management/ManagementFactory.java b/src/share/classes/java/lang/management/ManagementFactory.java index 20906a1fb..d99333c9e 100644 --- a/src/share/classes/java/lang/management/ManagementFactory.java +++ b/src/share/classes/java/lang/management/ManagementFactory.java @@ -576,16 +576,16 @@ public class ManagementFactory { Class mxbeanInterface) throws java.io.IOException { - final Class interfaceClass = mxbeanInterface; // Only allow MXBean interfaces from rt.jar loaded by the // bootstrap class loader - final ClassLoader loader = + final Class cls = mxbeanInterface; + ClassLoader loader = AccessController.doPrivileged(new PrivilegedAction() { public ClassLoader run() { - return interfaceClass.getClassLoader(); + return cls.getClassLoader(); } }); - if (loader != null) { + if (!sun.misc.VM.isSystemDomainLoader(loader)) { throw new IllegalArgumentException(mxbeanName + " is not a platform MXBean"); } @@ -593,10 +593,10 @@ public class ManagementFactory { try { final ObjectName objName = new ObjectName(mxbeanName); // skip the isInstanceOf check for LoggingMXBean - String intfName = interfaceClass.getName(); + String intfName = mxbeanInterface.getName(); if (!connection.isInstanceOf(objName, intfName)) { throw new IllegalArgumentException(mxbeanName + - " is not an instance of " + interfaceClass); + " is not an instance of " + mxbeanInterface); } final Class[] interfaces; diff --git a/src/share/classes/java/lang/management/PlatformComponent.java b/src/share/classes/java/lang/management/PlatformComponent.java index d9243354b..7e36c74d0 100644 --- a/src/share/classes/java/lang/management/PlatformComponent.java +++ b/src/share/classes/java/lang/management/PlatformComponent.java @@ -363,7 +363,8 @@ enum PlatformComponent { try { // Lazy loading the MXBean interface only when it is needed return (Class) - Class.forName(mxbeanInterfaceName, false, null); + Class.forName(mxbeanInterfaceName, false, + PlatformManagedObject.class.getClassLoader()); } catch (ClassNotFoundException x) { throw new AssertionError(x); } diff --git a/src/share/classes/java/util/prefs/Preferences.java b/src/share/classes/java/util/prefs/Preferences.java index 781b95ae4..1867fca91 100644 --- a/src/share/classes/java/util/prefs/Preferences.java +++ b/src/share/classes/java/util/prefs/Preferences.java @@ -300,7 +300,8 @@ public abstract class Preferences { } try { return (PreferencesFactory) - Class.forName(platformFactory, false, null).newInstance(); + Class.forName(platformFactory, false, + Preferences.class.getClassLoader()).newInstance(); } catch (Exception e) { throw new InternalError( "Can't instantiate platform default Preferences factory " diff --git a/src/share/classes/javax/script/ScriptEngineManager.java b/src/share/classes/javax/script/ScriptEngineManager.java index c7932a8ae..43035125e 100644 --- a/src/share/classes/javax/script/ScriptEngineManager.java +++ b/src/share/classes/javax/script/ScriptEngineManager.java @@ -423,7 +423,7 @@ public class ScriptEngineManager { SecurityManager sm = System.getSecurityManager(); if (sm != null) { ClassLoader callerLoader = getCallerClassLoader(); - if (callerLoader != null) { + if (!sun.misc.VM.isSystemDomainLoader(callerLoader)) { if (loader != callerLoader || !isAncestor(loader, callerLoader)) { try { sm.checkPermission(SecurityConstants.GET_CLASSLOADER_PERMISSION); diff --git a/src/share/classes/sun/management/MappedMXBeanType.java b/src/share/classes/sun/management/MappedMXBeanType.java index db90899b0..aad1f54b2 100644 --- a/src/share/classes/sun/management/MappedMXBeanType.java +++ b/src/share/classes/sun/management/MappedMXBeanType.java @@ -803,7 +803,7 @@ public abstract class MappedMXBeanType { Class c; try { c = Class.forName(t.getClassName(), false, - String.class.getClassLoader()); + MappedMXBeanType.class.getClassLoader()); MappedMXBeanType.newBasicType(c, t); } catch (ClassNotFoundException e) { // the classes that these predefined types declare diff --git a/src/share/classes/sun/misc/Unsafe.java b/src/share/classes/sun/misc/Unsafe.java index da7ec2798..93ce228e0 100644 --- a/src/share/classes/sun/misc/Unsafe.java +++ b/src/share/classes/sun/misc/Unsafe.java @@ -82,7 +82,7 @@ public final class Unsafe { */ public static Unsafe getUnsafe() { Class cc = sun.reflect.Reflection.getCallerClass(2); - if (cc.getClassLoader() != null) + if (!VM.isSystemDomainLoader(cc.getClassLoader())) throw new SecurityException("Unsafe"); return theUnsafe; } diff --git a/src/share/classes/sun/misc/VM.java b/src/share/classes/sun/misc/VM.java index 9928b840e..ed60a5a20 100644 --- a/src/share/classes/sun/misc/VM.java +++ b/src/share/classes/sun/misc/VM.java @@ -217,6 +217,14 @@ public class VM { return allowArraySyntax; } + /** + * Returns true if the given class loader is in the system domain + * in which all permissions are granted. + */ + public static boolean isSystemDomainLoader(ClassLoader loader) { + return loader == null; + } + /** * Returns the system property of the specified key saved at * system initialization time. This method should only be used diff --git a/src/share/classes/sun/reflect/misc/ReflectUtil.java b/src/share/classes/sun/reflect/misc/ReflectUtil.java index 8b1efd565..5a90c28db 100644 --- a/src/share/classes/sun/reflect/misc/ReflectUtil.java +++ b/src/share/classes/sun/reflect/misc/ReflectUtil.java @@ -144,4 +144,38 @@ public final class ReflectUtil { } return true; } + + // Returns true if p is an ancestor of cl i.e. class loader 'p' can + // be found in the cl's delegation chain + private static boolean isAncestor(ClassLoader p, ClassLoader cl) { + ClassLoader acl = cl; + do { + acl = acl.getParent(); + if (p == acl) { + return true; + } + } while (acl != null); + return false; + } + + /** + * Returns true if package access check is needed for reflective + * access from a class loader 'from' to classes or members in + * a class defined by class loader 'to'. This method returns true + * if 'from' is not the same as or an ancestor of 'to'. All code + * in a system domain are granted with all permission and so this + * method returns false if 'from' class loader is a class loader + * loading system classes. On the other hand, if a class loader + * attempts to access system domain classes, it requires package + * access check and this method will return true. + */ + public static boolean needsPackageAccessCheck(ClassLoader from, ClassLoader to) { + if (from == null || from == to) + return false; + + if (to == null) + return true; + + return !isAncestor(from, to); + } } -- GitLab