RFR - 8190482: InnocuousThread creation should not require the caller to possess enableContextClassLoaderOverride

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR - 8190482: InnocuousThread creation should not require the caller to possess enableContextClassLoaderOverride

Chris Hegarty
Currently JDK code that wants to create innocuous threads is required to do so within a privileged context that has the "enableContextClassLoaderOverride" RuntimePermission ( since the InnocuousThread class overrides setContextClassLoader ). This permissions should not be required, especially if code in de-privileged modules wants to create innocuous threads.

The factory methods for creating innocuous threads should assert privileges before constructing the thread.

diff --git a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
--- a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
+++ b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
@@ -62,10 +62,16 @@
      * set to the system class loader.
      */
     public static Thread newThread(String name, Runnable target) {
-        return new InnocuousThread(INNOCUOUSTHREADGROUP,
-                                   target,
-                                   name,
-                                   ClassLoader.getSystemClassLoader());
+        return AccessController.doPrivileged(
+                new PrivilegedAction<Thread>() {
+                    @Override
+                    public Thread run() {
+                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
+                                                   target,
+                                                   name,
+                                                   ClassLoader.getSystemClassLoader());
+                    }
+                });
     }
 
     /**
@@ -80,8 +86,14 @@
      * Returns a new InnocuousThread with null context class loader.
      */
     public static Thread newSystemThread(String name, Runnable target) {
-        return new InnocuousThread(INNOCUOUSTHREADGROUP,
-                                   target, name, null);
+        return AccessController.doPrivileged(
+                new PrivilegedAction<Thread>() {
+                    @Override
+                    public Thread run() {
+                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
+                                                   target, name, null);
+                    }
+                });
     }
 
-Chris.
Reply | Threaded
Open this post in threaded view
|

Re: RFR - 8190482: InnocuousThread creation should not require the caller to possess enableContextClassLoaderOverride

roger riggs
Hi Chris,

Looks fine

Roger


On 11/5/2017 5:02 AM, Chris Hegarty wrote:

> Currently JDK code that wants to create innocuous threads is required to do so within a privileged context that has the "enableContextClassLoaderOverride" RuntimePermission ( since the InnocuousThread class overrides setContextClassLoader ). This permissions should not be required, especially if code in de-privileged modules wants to create innocuous threads.
>
> The factory methods for creating innocuous threads should assert privileges before constructing the thread.
>
> diff --git a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> --- a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> +++ b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> @@ -62,10 +62,16 @@
>        * set to the system class loader.
>        */
>       public static Thread newThread(String name, Runnable target) {
> -        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> -                                   target,
> -                                   name,
> -                                   ClassLoader.getSystemClassLoader());
> +        return AccessController.doPrivileged(
> +                new PrivilegedAction<Thread>() {
> +                    @Override
> +                    public Thread run() {
> +                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> +                                                   target,
> +                                                   name,
> +                                                   ClassLoader.getSystemClassLoader());
> +                    }
> +                });
>       }
>  
>       /**
> @@ -80,8 +86,14 @@
>        * Returns a new InnocuousThread with null context class loader.
>        */
>       public static Thread newSystemThread(String name, Runnable target) {
> -        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> -                                   target, name, null);
> +        return AccessController.doPrivileged(
> +                new PrivilegedAction<Thread>() {
> +                    @Override
> +                    public Thread run() {
> +                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> +                                                   target, name, null);
> +                    }
> +                });
>       }
>  
> -Chris.

Reply | Threaded
Open this post in threaded view
|

Re: RFR - 8190482: InnocuousThread creation should not require the caller to possess enableContextClassLoaderOverride

Mandy Chung
In reply to this post by Chris Hegarty
Looks fine to me.

Mandy

On 11/5/17 2:02 AM, Chris Hegarty wrote:

> Currently JDK code that wants to create innocuous threads is required to do so within a privileged context that has the "enableContextClassLoaderOverride" RuntimePermission ( since the InnocuousThread class overrides setContextClassLoader ). This permissions should not be required, especially if code in de-privileged modules wants to create innocuous threads.
>
> The factory methods for creating innocuous threads should assert privileges before constructing the thread.
>
> diff --git a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> --- a/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> +++ b/src/java.base/share/classes/jdk/internal/misc/InnocuousThread.java
> @@ -62,10 +62,16 @@
>        * set to the system class loader.
>        */
>       public static Thread newThread(String name, Runnable target) {
> -        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> -                                   target,
> -                                   name,
> -                                   ClassLoader.getSystemClassLoader());
> +        return AccessController.doPrivileged(
> +                new PrivilegedAction<Thread>() {
> +                    @Override
> +                    public Thread run() {
> +                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> +                                                   target,
> +                                                   name,
> +                                                   ClassLoader.getSystemClassLoader());
> +                    }
> +                });
>       }
>  
>       /**
> @@ -80,8 +86,14 @@
>        * Returns a new InnocuousThread with null context class loader.
>        */
>       public static Thread newSystemThread(String name, Runnable target) {
> -        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> -                                   target, name, null);
> +        return AccessController.doPrivileged(
> +                new PrivilegedAction<Thread>() {
> +                    @Override
> +                    public Thread run() {
> +                        return new InnocuousThread(INNOCUOUSTHREADGROUP,
> +                                                   target, name, null);
> +                    }
> +                });
>       }
>  
> -Chris.