7103570: AtomicIntegerFieldUpdater does not work when SecurityManager is installed
Summary: Perform class.getField inside a doPrivileged block
Reviewed-by: chegar, psandoz
--- a/jdk/src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java Mon May 07 13:34:19 2012 +0100
+++ b/jdk/src/share/classes/java/util/concurrent/atomic/AtomicIntegerFieldUpdater.java Tue May 08 02:59:10 2012 -0400
@@ -35,7 +35,11 @@
package java.util.concurrent.atomic;
import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
/**
* A reflection-based utility that enables atomic updates to
@@ -67,7 +71,9 @@
* @throws IllegalArgumentException if the field is not a
* volatile integer type
* @throws RuntimeException with a nested reflection-based
- * exception if the class does not hold field or is the wrong type
+ * exception if the class does not hold field or is the wrong type,
+ * or the field is inaccessible to the caller according to Java language
+ * access control
*/
public static <U> AtomicIntegerFieldUpdater<U> newUpdater(Class<U> tclass, String fieldName) {
return new AtomicIntegerFieldUpdaterImpl<U>(tclass, fieldName);
@@ -267,17 +273,29 @@
private final Class<T> tclass;
private final Class<?> cclass;
- AtomicIntegerFieldUpdaterImpl(Class<T> tclass, String fieldName) {
+ AtomicIntegerFieldUpdaterImpl(final Class<T> tclass, final String fieldName) {
Field field = null;
Class<?> caller = null;
int modifiers = 0;
try {
- field = tclass.getDeclaredField(fieldName);
+ field = AccessController.doPrivileged(
+ new PrivilegedExceptionAction<Field>() {
+ public Field run() throws NoSuchFieldException {
+ return tclass.getDeclaredField(fieldName);
+ }
+ });
caller = sun.reflect.Reflection.getCallerClass(3);
modifiers = field.getModifiers();
sun.reflect.misc.ReflectUtil.ensureMemberAccess(
caller, tclass, null, modifiers);
- sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ ClassLoader cl = tclass.getClassLoader();
+ ClassLoader ccl = caller.getClassLoader();
+ if ((ccl != null) && (ccl != cl) &&
+ ((cl == null) || !isAncestor(cl, ccl))) {
+ sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ }
+ } catch (PrivilegedActionException pae) {
+ throw new RuntimeException(pae.getException());
} catch (Exception ex) {
throw new RuntimeException(ex);
}
@@ -295,6 +313,22 @@
offset = unsafe.objectFieldOffset(field);
}
+ /**
+ * Returns true if the second classloader can be found in the first
+ * classloader's delegation chain.
+ * Equivalent to the inaccessible: first.isAncestor(second).
+ */
+ private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+ ClassLoader acl = first;
+ do {
+ acl = acl.getParent();
+ if (second == acl) {
+ return true;
+ }
+ } while (acl != null);
+ return false;
+ }
+
private void fullCheck(T obj) {
if (!tclass.isInstance(obj))
throw new ClassCastException();
--- a/jdk/src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java Mon May 07 13:34:19 2012 +0100
+++ b/jdk/src/share/classes/java/util/concurrent/atomic/AtomicLongFieldUpdater.java Tue May 08 02:59:10 2012 -0400
@@ -35,7 +35,11 @@
package java.util.concurrent.atomic;
import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
/**
* A reflection-based utility that enables atomic updates to
@@ -67,7 +71,9 @@
* @throws IllegalArgumentException if the field is not a
* volatile long type.
* @throws RuntimeException with a nested reflection-based
- * exception if the class does not hold field or is the wrong type.
+ * exception if the class does not hold field or is the wrong type,
+ * or the field is inaccessible to the caller according to Java language
+ * access control
*/
public static <U> AtomicLongFieldUpdater<U> newUpdater(Class<U> tclass, String fieldName) {
if (AtomicLong.VM_SUPPORTS_LONG_CAS)
@@ -267,17 +273,29 @@
private final Class<T> tclass;
private final Class<?> cclass;
- CASUpdater(Class<T> tclass, String fieldName) {
+ CASUpdater(final Class<T> tclass, final String fieldName) {
Field field = null;
Class<?> caller = null;
int modifiers = 0;
try {
- field = tclass.getDeclaredField(fieldName);
+ field = AccessController.doPrivileged(
+ new PrivilegedExceptionAction<Field>() {
+ public Field run() throws NoSuchFieldException {
+ return tclass.getDeclaredField(fieldName);
+ }
+ });
caller = sun.reflect.Reflection.getCallerClass(3);
modifiers = field.getModifiers();
sun.reflect.misc.ReflectUtil.ensureMemberAccess(
caller, tclass, null, modifiers);
- sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ ClassLoader cl = tclass.getClassLoader();
+ ClassLoader ccl = caller.getClassLoader();
+ if ((ccl != null) && (ccl != cl) &&
+ ((cl == null) || !isAncestor(cl, ccl))) {
+ sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ }
+ } catch (PrivilegedActionException pae) {
+ throw new RuntimeException(pae.getException());
} catch (Exception ex) {
throw new RuntimeException(ex);
}
@@ -350,17 +368,29 @@
private final Class<T> tclass;
private final Class<?> cclass;
- LockedUpdater(Class<T> tclass, String fieldName) {
+ LockedUpdater(final Class<T> tclass, final String fieldName) {
Field field = null;
Class<?> caller = null;
int modifiers = 0;
try {
- field = tclass.getDeclaredField(fieldName);
+ field = AccessController.doPrivileged(
+ new PrivilegedExceptionAction<Field>() {
+ public Field run() throws NoSuchFieldException {
+ return tclass.getDeclaredField(fieldName);
+ }
+ });
caller = sun.reflect.Reflection.getCallerClass(3);
modifiers = field.getModifiers();
sun.reflect.misc.ReflectUtil.ensureMemberAccess(
caller, tclass, null, modifiers);
- sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ ClassLoader cl = tclass.getClassLoader();
+ ClassLoader ccl = caller.getClassLoader();
+ if ((ccl != null) && (ccl != cl) &&
+ ((cl == null) || !isAncestor(cl, ccl))) {
+ sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ }
+ } catch (PrivilegedActionException pae) {
+ throw new RuntimeException(pae.getException());
} catch (Exception ex) {
throw new RuntimeException(ex);
}
@@ -433,4 +463,20 @@
);
}
}
+
+ /**
+ * Returns true if the second classloader can be found in the first
+ * classloader's delegation chain.
+ * Equivalent to the inaccessible: first.isAncestor(second).
+ */
+ private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+ ClassLoader acl = first;
+ do {
+ acl = acl.getParent();
+ if (second == acl) {
+ return true;
+ }
+ } while (acl != null);
+ return false;
+ }
}
--- a/jdk/src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java Mon May 07 13:34:19 2012 +0100
+++ b/jdk/src/share/classes/java/util/concurrent/atomic/AtomicReferenceFieldUpdater.java Tue May 08 02:59:10 2012 -0400
@@ -35,7 +35,11 @@
package java.util.concurrent.atomic;
import sun.misc.Unsafe;
-import java.lang.reflect.*;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.security.AccessController;
+import java.security.PrivilegedExceptionAction;
+import java.security.PrivilegedActionException;
/**
* A reflection-based utility that enables atomic updates to
@@ -86,7 +90,9 @@
* @return the updater
* @throws IllegalArgumentException if the field is not a volatile reference type.
* @throws RuntimeException with a nested reflection-based
- * exception if the class does not hold field or is the wrong type.
+ * exception if the class does not hold field or is the wrong type,
+ * or the field is inaccessible to the caller according to Java language
+ * access control
*/
public static <U, W> AtomicReferenceFieldUpdater<U,W> newUpdater(Class<U> tclass, Class<W> vclass, String fieldName) {
return new AtomicReferenceFieldUpdaterImpl<U,W>(tclass,
@@ -197,21 +203,33 @@
* screenings fail.
*/
- AtomicReferenceFieldUpdaterImpl(Class<T> tclass,
+ AtomicReferenceFieldUpdaterImpl(final Class<T> tclass,
Class<V> vclass,
- String fieldName) {
+ final String fieldName) {
Field field = null;
Class<?> fieldClass = null;
Class<?> caller = null;
int modifiers = 0;
try {
- field = tclass.getDeclaredField(fieldName);
+ field = AccessController.doPrivileged(
+ new PrivilegedExceptionAction<Field>() {
+ public Field run() throws NoSuchFieldException {
+ return tclass.getDeclaredField(fieldName);
+ }
+ });
caller = sun.reflect.Reflection.getCallerClass(3);
modifiers = field.getModifiers();
sun.reflect.misc.ReflectUtil.ensureMemberAccess(
- caller, tclass, null, modifiers);
- sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ caller, tclass, null, modifiers);
+ ClassLoader cl = tclass.getClassLoader();
+ ClassLoader ccl = caller.getClassLoader();
+ if ((ccl != null) && (ccl != cl) &&
+ ((cl == null) || !isAncestor(cl, ccl))) {
+ sun.reflect.misc.ReflectUtil.checkPackageAccess(tclass);
+ }
fieldClass = field.getType();
+ } catch (PrivilegedActionException pae) {
+ throw new RuntimeException(pae.getException());
} catch (Exception ex) {
throw new RuntimeException(ex);
}
@@ -232,6 +250,22 @@
offset = unsafe.objectFieldOffset(field);
}
+ /**
+ * Returns true if the second classloader can be found in the first
+ * classloader's delegation chain.
+ * Equivalent to the inaccessible: first.isAncestor(second).
+ */
+ private static boolean isAncestor(ClassLoader first, ClassLoader second) {
+ ClassLoader acl = first;
+ do {
+ acl = acl.getParent();
+ if (second == acl) {
+ return true;
+ }
+ } while (acl != null);
+ return false;
+ }
+
void targetCheck(T obj) {
if (!tclass.isInstance(obj))
throw new ClassCastException();
@@ -281,7 +315,7 @@
}
@SuppressWarnings("unchecked")
- public V get(T obj) {
+ public V get(T obj) {
if (obj == null || obj.getClass() != tclass || cclass != null)
targetCheck(obj);
return (V)unsafe.getObjectVolatile(obj, offset);
@@ -292,14 +326,14 @@
return;
}
throw new RuntimeException(
- new IllegalAccessException("Class " +
- cclass.getName() +
- " can not access a protected member of class " +
- tclass.getName() +
- " using an instance of " +
- obj.getClass().getName()
- )
- );
+ new IllegalAccessException("Class " +
+ cclass.getName() +
+ " can not access a protected member of class " +
+ tclass.getName() +
+ " using an instance of " +
+ obj.getClass().getName()
+ )
+ );
}
}
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/java/util/concurrent/atomic/AtomicUpdaters.java Tue May 08 02:59:10 2012 -0400
@@ -0,0 +1,189 @@
+/*
+ * Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 7103570
+ * @author David Holmes
+ * @run main/othervm AtomicUpdaters
+ * @run main/othervm AtomicUpdaters UseSM
+ * @summary Checks the (in)ability to create field updaters for differently
+ * accessible fields in different locations with/without a security
+ * manager
+ */
+import java.util.concurrent.atomic.*;
+import java.lang.reflect.*;
+import java.security.AccessControlException;
+
+public class AtomicUpdaters {
+ enum TYPE { INT, LONG, REF }
+
+ static class Config {
+ final Class<?> clazz;
+ final String field;
+ final String access;
+ final boolean reflectOk;
+ final boolean updaterOk;
+ final String desc;
+ final TYPE type;
+
+ Config(Class<?> clazz, String field, String access,
+ boolean reflectOk, boolean updaterOk, String desc, TYPE type) {
+ this.clazz = clazz;
+ this.field = field;
+ this.access = access;
+ this.reflectOk = reflectOk;
+ this.updaterOk = updaterOk;
+ this.desc = desc;
+ this.type =type;
+ }
+
+ public String toString() {
+ return desc + ": " + access + " " + clazz.getName() + "." + field;
+ }
+ }
+
+ static Config[] tests;
+
+ static void initTests(boolean hasSM) {
+ tests = new Config[] {
+ new Config(AtomicUpdaters.class, "pub_int", "public", true, true, "public int field of current class", TYPE.INT),
+ new Config(AtomicUpdaters.class, "priv_int", "private", true, true, "private int field of current class", TYPE.INT),
+ new Config(AtomicUpdaters.class, "pub_long", "public", true, true, "public long field of current class", TYPE.LONG),
+ new Config(AtomicUpdaters.class, "priv_long", "private", true, true, "private long field of current class", TYPE.LONG),
+ new Config(AtomicUpdaters.class, "pub_ref", "public", true, true, "public ref field of current class", TYPE.REF),
+ new Config(AtomicUpdaters.class, "priv_ref", "private", true, true, "private ref field of current class", TYPE.REF),
+
+ // Would like to test a public volatile in a class in another
+ // package - but of course there aren't any
+ new Config(java.util.concurrent.atomic.AtomicInteger.class, "value", "private", hasSM ? false : true, false, "private int field of class in different package", TYPE.INT),
+ new Config(java.util.concurrent.atomic.AtomicLong.class, "value", "private", hasSM ? false : true, false, "private long field of class in different package", TYPE.LONG),
+ new Config(java.util.concurrent.atomic.AtomicReference.class, "value", "private", hasSM ? false : true, false, "private reference field of class in different package", TYPE.REF),
+ };
+ }
+
+ public volatile int pub_int;
+ private volatile int priv_int;
+ public volatile long pub_long;
+ private volatile long priv_long;
+ public volatile Object pub_ref;
+ private volatile Object priv_ref;
+
+
+ // This should be set dynamically at runtime using a System property, but
+ // ironically we get a SecurityException if we try to do that with a
+ // SecurityManager installed
+ static boolean verbose;
+
+ public static void main(String[] args) throws Throwable {
+ boolean hasSM = false;
+ for (String arg : args) {
+ if ("-v".equals(arg)) {
+ verbose = true;
+ }
+ else if ("UseSM".equals(arg)) {
+ SecurityManager m = System.getSecurityManager();
+ if (m != null)
+ throw new RuntimeException("No security manager should initially be installed");
+ System.setSecurityManager(new java.lang.SecurityManager());
+ hasSM = true;
+ }
+ else {
+ throw new IllegalArgumentException("Unexpected option: " + arg);
+ }
+ }
+ initTests(hasSM);
+
+ int failures = 0;
+
+ System.out.printf("Testing with%s a SecurityManager present\n", hasSM ? "" : "out");
+ for (Config c : tests) {
+ System.out.println("Testing: " + c);
+ Error reflectionFailure = null;
+ Error updaterFailure = null;
+ Class<?> clazz = c.clazz;
+ // See if we can reflectively access the field
+ System.out.println(" - testing getDeclaredField");
+ try {
+ Field f = clazz.getDeclaredField(c.field);
+ if (!c.reflectOk)
+ reflectionFailure = new Error("Unexpected reflective access: " + c);
+ }
+ catch (AccessControlException e) {
+ if (c.reflectOk)
+ reflectionFailure = new Error("Unexpected reflective access failure: " + c, e);
+ else if (verbose) {
+ System.out.println("Got expected reflection exception: " + e);
+ e.printStackTrace(System.out);
+ }
+ }
+
+ if (reflectionFailure != null) {
+ reflectionFailure.printStackTrace(System.out);
+ }
+
+ // see if we can create an atomic updater for the field
+ Object u = null;
+ try {
+ switch (c.type) {
+ case INT:
+ System.out.println(" - testing AtomicIntegerFieldUpdater");
+ u = AtomicIntegerFieldUpdater.newUpdater(clazz, c.field);
+ break;
+ case LONG:
+ System.out.println(" - testing AtomicLongFieldUpdater");
+ u = AtomicLongFieldUpdater.newUpdater(clazz, c.field);
+ break;
+ case REF:
+ System.out.println(" - testing AtomicReferenceFieldUpdater");
+ u = AtomicReferenceFieldUpdater.newUpdater(clazz, Object.class, c.field);
+ break;
+ }
+
+ if (!c.updaterOk)
+ updaterFailure = new Error("Unexpected updater access: " + c);
+ }
+ catch (Exception e) {
+ if (c.updaterOk)
+ updaterFailure = new Error("Unexpected updater access failure: " + c, e);
+ else if (verbose) {
+ System.out.println("Got expected updater exception: " + e);
+ e.printStackTrace(System.out);
+ }
+ }
+
+ if (updaterFailure != null) {
+ updaterFailure.printStackTrace(System.out);
+ }
+
+ if (updaterFailure != null || reflectionFailure != null) {
+ failures++;
+
+ }
+ }
+
+ if (failures > 0) {
+ throw new Error("Some tests failed - see previous stacktraces");
+ }
+ }
+}