Repository: netty/netty
Issue: CleanerJava9 instantiation fails under SecurityManager
Labels: defect

### Expected behavior

When instantiating `io.netty.util.internal.CleanerJava9`, it should be possible when running under a SecurityManager.

### Actual behavior

`io.netty.util.internal.CleanerJava9` fails with `java.lang.ExceptionInInitializerError`

```
java.lang.ExceptionInInitializerError: null
	at io.netty.util.internal.PlatformDependent.<clinit>(PlatformDependent.java:186) ~[?:?]
	at io.netty.util.ConstantPool.<init>(ConstantPool.java:32) ~[?:?]
	at io.netty.util.AttributeKey$1.<init>(AttributeKey.java:27) ~[?:?]
	at io.netty.util.AttributeKey.<clinit>(AttributeKey.java:27) ~[?:?]
	at org.elasticsearch.transport.netty4.Netty4Transport.<clinit>(Netty4Transport.java:263) ~[?:?]
	at org.elasticsearch.transport.Netty4Plugin.getSettings(Netty4Plugin.java:56) ~[?:?]
	at org.elasticsearch.plugins.PluginsService.lambda$getPluginSettings$0(PluginsService.java:90) ~[elasticsearch-6.3.2.jar:?]
	at java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:271) ~[?:?]
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1492) ~[?:?]
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) ~[?:?]
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) ~[?:?]
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:913) ~[?:?]
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) ~[?:?]
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:578) ~[?:?]
	at org.elasticsearch.plugins.PluginsService.getPluginSettings(PluginsService.java:90) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.node.Node.<init>(Node.java:327) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.node.Node.<init>(Node.java:251) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:215) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Bootstrap.setup(Bootstrap.java:215) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Bootstrap.init(Bootstrap.java:328) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Elasticsearch.init(Elasticsearch.java:136) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Elasticsearch.execute(Elasticsearch.java:127) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:86) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.cli.Command.mainWithoutErrorHandling(Command.java:124) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.cli.Command.main(Command.java:90) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:93) ~[elasticsearch-6.3.2.jar:?]
	at org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:86) ~[elasticsearch-6.3.2.jar:?]
Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessDeclaredMembers")
	at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472) ~[?:?]
	at java.security.AccessController.checkPermission(AccessController.java:895) ~[?:?]
	at java.lang.SecurityManager.checkPermission(SecurityManager.java:335) ~[?:?]
	at java.lang.Class.checkMemberAccess(Class.java:2806) ~[?:?]
	at java.lang.Class.getDeclaredMethod(Class.java:2430) ~[?:?]
	at io.netty.util.internal.CleanerJava9.<clinit>(CleanerJava9.java:41) ~[?:?]
	... 27 more
```

The method call `PlatformDependent0.UNSAFE.getClass().getDeclaredMethod("invokeCleaner", ByteBuffer.class)` and `INVOKE_CLEANER.invoke(PlatformDependent0.UNSAFE, buffer);` should be protected by

```
AccessController.doPrivileged(new PrivilegedExceptionAction() {
         public Object run() throws Exception {
               ....
        }
}
````
so that a security manager can apply a policy to allow "accessDeclaredMembers".

I know that using `PlatformDependent0.UNSAFE` is riding like a dead horse - it appears in Java 10 for the last time. Java 11 will drop `sun.misc.Unsafe`.  A solution (or demonstration) for using native Linux epoll transport under Java 10 (without `sun.misc.Unsafe`) and Java 11+ is most appreciated.

### Steps to reproduce

### Minimal yet complete reproducer code (or URL to code)

### Netty version

4.1.28.Final

### JVM version (e.g. `java -version`)

openjdk version "10.0.2" 2018-07-17
OpenJDK Runtime Environment (build 10.0.2+13)
OpenJDK 64-Bit Server VM (build 10.0.2+13, mixed mode)

### OS version (e.g. `uname -a`)

Linux phoebe 4.17.12-200.fc28.x86_64 #1 SMP Fri Aug 3 15:01:13 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

Issue comments:
@jprante can you provide a PR ?


Method handles works just fine in Java 11, see https://github.com/apache/lucene-solr/blob/master/lucene/core/src/java/org/apache/lucene/store/MMapDirectory.java#L339
@johnou @jprante I think we should look into using handles... Can you open an issue ?
Fixed in https://github.com/netty/netty/pull/8204. @jprante PTAL
@jprante this will allow to use the native transports without unsafe:

https://github.com/netty/netty/pull/8231

Pull request review comments:
idle question: why not use PrivilegedExceptionAction ?
Could just catch a `Throwable`?
@trustin see https://github.com/netty/netty/issues/6096
Also honestly I think it will not make any difference. So maybe just keep it ?
I think there is not much advantage here.. We will need to unwrap then etc. 

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/internal/CleanerJava9.java b/common/src/main/java/io/netty/util/internal/CleanerJava9.java
index d111a7e54a5..3a0c5854c38 100644
--- a/common/src/main/java/io/netty/util/internal/CleanerJava9.java
+++ b/common/src/main/java/io/netty/util/internal/CleanerJava9.java
@@ -21,6 +21,8 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 
 /**
  * Provide a way to clean a ByteBuffer on Java9+.
@@ -34,20 +36,26 @@ final class CleanerJava9 implements Cleaner {
         final Method method;
         final Throwable error;
         if (PlatformDependent0.hasUnsafe()) {
-            ByteBuffer buffer = ByteBuffer.allocateDirect(1);
-            Object maybeInvokeMethod;
-            try {
-                // See https://bugs.openjdk.java.net/browse/JDK-8171377
-                Method m = PlatformDependent0.UNSAFE.getClass().getDeclaredMethod("invokeCleaner", ByteBuffer.class);
-                m.invoke(PlatformDependent0.UNSAFE, buffer);
-                maybeInvokeMethod = m;
-            } catch (NoSuchMethodException e) {
-                maybeInvokeMethod = e;
-            } catch (InvocationTargetException e) {
-                maybeInvokeMethod = e;
-            } catch (IllegalAccessException e) {
-                maybeInvokeMethod = e;
-            }
+            final ByteBuffer buffer = ByteBuffer.allocateDirect(1);
+            Object maybeInvokeMethod = AccessController.doPrivileged(new PrivilegedAction<Object>() {
+                @Override
+                public Object run() {
+                    try {
+                        // See https://bugs.openjdk.java.net/browse/JDK-8171377
+                        Method m = PlatformDependent0.UNSAFE.getClass().getDeclaredMethod(
+                                "invokeCleaner", ByteBuffer.class);
+                        m.invoke(PlatformDependent0.UNSAFE, buffer);
+                        return m;
+                    } catch (NoSuchMethodException e) {
+                        return e;
+                    } catch (InvocationTargetException e) {
+                        return e;
+                    } catch (IllegalAccessException e) {
+                        return e;
+                    }
+                }
+            });
+
             if (maybeInvokeMethod instanceof Throwable) {
                 method = null;
                 error = (Throwable) maybeInvokeMethod;
@@ -73,10 +81,35 @@ static boolean isSupported() {
 
     @Override
     public void freeDirectBuffer(ByteBuffer buffer) {
-        try {
-            INVOKE_CLEANER.invoke(PlatformDependent0.UNSAFE, buffer);
-        } catch (Throwable cause) {
-            PlatformDependent0.throwException(cause);
+        // Try to minimize overhead when there is no SecurityManager present.
+        // See https://bugs.openjdk.java.net/browse/JDK-8191053.
+        if (System.getSecurityManager() == null) {
+            try {
+                INVOKE_CLEANER.invoke(PlatformDependent0.UNSAFE, buffer);
+            } catch (Throwable cause) {
+                PlatformDependent0.throwException(cause);
+            }
+        } else {
+            freeDirectBufferPrivileged(buffer);
+        }
+    }
+
+    private static void freeDirectBufferPrivileged(final ByteBuffer buffer) {
+        Exception error = AccessController.doPrivileged(new PrivilegedAction<Exception>() {
+            @Override
+            public Exception run() {
+                try {
+                    INVOKE_CLEANER.invoke(PlatformDependent0.UNSAFE, buffer);
+                } catch (InvocationTargetException e) {
+                    return e;
+                } catch (IllegalAccessException e) {
+                    return e;
+                }
+                return null;
+            }
+        });
+        if (error != null) {
+            PlatformDependent0.throwException(error);
         }
     }
 }


Test patch: