Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove System.getSecurityManager calls from java.base and criu #20939

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+60 −5
Open

Remove System.getSecurityManager calls from java.base and criu #20939

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
821f41c
Remove System.getSecurityManager calls from java.base and CRIU
theresa-m Jan 14, 2025
fb0ffb9
Update end compiler directive comments
theresa-m Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions jcl/src/java.base/share/classes/java/lang/Class.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@
import sun.reflect.annotation.AnnotationType;
import java.util.Arrays;
import com.ibm.oti.vm.VM;
/*[IF JAVA_SPEC_VERSION >= 11]*/
/*[IF (JAVA_SPEC_VERSION >= 11) & (JAVA_SPEC_VERSION < 24)]*/
theresa-m marked this conversation as resolved.
Show resolved Hide resolved
import static com.ibm.oti.util.Util.doesClassLoaderDescendFrom;
/*[ENDIF] JAVA_SPEC_VERSION >= 11*/
/*[ENDIF] (JAVA_SPEC_VERSION >= 11) && (JAVA_SPEC_VERSION < 24) */
theresa-m marked this conversation as resolved.
Show resolved Hide resolved

/*[IF JAVA_SPEC_VERSION >= 9]
import jdk.internal.misc.Unsafe;
Expand Down Expand Up @@ -5547,9 +5547,11 @@ static byte[] getExecutableTypeAnnotationBytes(Executable exec) {
/**
* Answers the host class of the receiver's nest.
*
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if nestHost is not same as the current class, a security manager
* is present, the classloader of the caller is not the same or an ancestor of nestHost
* class, and checkPackageAccess() denies access
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @return the host class of the receiver.
*/
@CallerSensitive
Expand All @@ -5561,6 +5563,7 @@ public Class<?> getNestHost()
if (nestHost == null) {
nestHost = getNestHostImpl();
}
/*[IF JAVA_SPEC_VERSION < 24]*/
/* The specification requires that if:
* - the returned class is not the current class
* - a security manager is present
Expand All @@ -5582,6 +5585,7 @@ public Class<?> getNestHost()
}
}
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */
return nestHost;
}

Expand Down
20 changes: 20 additions & 0 deletions jcl/src/java.base/share/classes/java/lang/ClassLoader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,14 +324,21 @@ static final void initializeClassLoaders() {
* Constructs a new instance of this class with the system
* class loader as its parent.
*
/*[IF JAVA_SPEC_VERSION < 24]
* @exception SecurityException
* if a security manager exists and it does not
* allow the creation of new ClassLoaders.
/*[ENDIF] JAVA_SPEC_VERSION < 24
*/
protected ClassLoader() {
/*[IF JAVA_SPEC_VERSION >= 24]*/
this(null, null, applicationClassLoader);
/*[ELSE] JAVA_SPEC_VERSION >= 24 */
this(checkSecurityPermission(), null, applicationClassLoader);
/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
}

/*[IF JAVA_SPEC_VERSION < 24]*/
/**
* This is a static helper method to perform security check earlier such that current ClassLoader object
* can't be resurrected when there is a SecurityException thrown.
Expand All @@ -346,6 +353,7 @@ private static Void checkSecurityPermission() {
}
return null;
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */

/**
* Constructs a new instance of this class with the given
Expand All @@ -354,12 +362,18 @@ private static Void checkSecurityPermission() {
* @param parentLoader ClassLoader
* the ClassLoader to use as the new class
* loaders parent.
/*[IF JAVA_SPEC_VERSION < 24]
* @exception SecurityException
* if a security manager exists and it does not
* allow the creation of new ClassLoaders.
/*[ENDIF] JAVA_SPEC_VERSION < 24
*/
protected ClassLoader(ClassLoader parentLoader) {
/*[IF JAVA_SPEC_VERSION >= 24]*/
this(null, null, parentLoader);
/*[ELSE] JAVA_SPEC_VERSION >= 24 */
this(checkSecurityPermission(), null, parentLoader);
/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
}

/*[IF JAVA_SPEC_VERSION >= 9]*/
Expand All @@ -374,13 +388,19 @@ protected ClassLoader(ClassLoader parentLoader) {
* loaders parent.
* @exception IllegalArgumentException
* if the name of this class loader is empty.
/*[IF JAVA_SPEC_VERSION < 24]
* @exception SecurityException
* if a security manager exists and it does not
* allow the creation of new ClassLoaders.
/*[ENDIF] JAVA_SPEC_VERSION < 24
*@since 9
*/
protected ClassLoader(String classLoaderName, ClassLoader parentLoader) {
/*[IF JAVA_SPEC_VERSION >= 24]*/
this(null, classLoaderName, parentLoader);
/*[ELSE] JAVA_SPEC_VERSION >= 24 */
this(checkSecurityPermission(), classLoaderName, parentLoader);
/*[ENDIF] JAVA_SPEC_VERSION >= 24 */
}
/*[ENDIF] JAVA_SPEC_VERSION >= 9 */

Expand Down
12 changes: 12 additions & 0 deletions jcl/src/java.base/share/classes/java/lang/StackWalker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
/*[ENDIF] JAVA_SPEC_VERSION >= 10 */
import java.lang.module.ModuleDescriptor;
import java.lang.module.ModuleDescriptor.Version;
/*[IF JAVA_SPEC_VERSION < 24]*/
import java.security.Permission;
/*[ENDIF] JAVA_SPEC_VERSION < 24 */
import java.util.Collections;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -125,11 +127,13 @@ private StackWalker(
}

if ((flags & J9_RETAIN_CLASS_REFERENCE) != 0) {
/*[IF JAVA_SPEC_VERSION < 24]*/
@SuppressWarnings("removal")
SecurityManager securityMgr = System.getSecurityManager();
if (null != securityMgr) {
securityMgr.checkPermission(PermissionSingleton.perm);
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */
/*[IF JAVA_SPEC_VERSION >= 19]*/
this.retainClassRef = true;
} else {
Expand Down Expand Up @@ -166,9 +170,11 @@ public static StackWalker getInstance() {
* @param option
* select the type of information to include
* @return StackWalker instance configured with the value of option
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException
* if option is RETAIN_CLASS_REFERENCE and the security manager
* check fails
/*[ENDIF] JAVA_SPEC_VERSION < 24
*/
public static StackWalker getInstance(Option option) {
Objects.requireNonNull(option);
Expand All @@ -181,9 +187,11 @@ public static StackWalker getInstance(Option option) {
* @param options
* select the types of information to include
* @return StackWalker instance configured with the given options
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException
* if RETAIN_CLASS_REFERENCE is requested and not permitted by
* the security manager
/*[ENDIF] JAVA_SPEC_VERSION < 24
*/
public static StackWalker getInstance(Set<Option> options) {
return getInstance(options, DEFAULT_BUFFER_SIZE);
Expand All @@ -198,9 +206,11 @@ public static StackWalker getInstance(Set<Option> options) {
* hint for the size of buffer to use. Must be 1 or greater
* @return StackWalker instance with the given options specifying the stack
* frame information it can access
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException
* if RETAIN_CLASS_REFERENCE is requested and not permitted by
* the security manager
/*[ENDIF] JAVA_SPEC_VERSION < 24
*/
public static StackWalker getInstance(Set<Option> options, int estimatedDepth) {
Objects.requireNonNull(options);
Expand Down Expand Up @@ -585,8 +595,10 @@ Object[] getMonitors() {
/*[ENDIF] JAVA_SPEC_VERSION >= 21 */
}

/*[IF JAVA_SPEC_VERSION < 24]*/
static final class PermissionSingleton {
static final Permission perm =
new RuntimePermission("getStackWalkerWithClassReference"); //$NON-NLS-1$
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */
}
2 changes: 1 addition & 1 deletion jcl/src/java.base/share/classes/openj9/internal/criu/CRIUDumpPermission.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*[INCLUDE-IF CRIU_SUPPORT]*/
/*[INCLUDE-IF CRIU_SUPPORT & (JAVA_SPEC_VERSION < 24)]*/
/*
* Copyright IBM Corp. and others 2023
*
Expand Down
15 changes: 14 additions & 1 deletion jcl/src/java.base/share/classes/openj9/internal/criu/InternalCRIUSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,9 @@ public static void runRunnableInNotCheckpointSafeMode(Runnable run) {

@SuppressWarnings("restriction")
private static Unsafe unsafe;
/*[IF JAVA_SPEC_VERSION < 24]*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains other security manager-related API usages, such as AccessController.doPrivileged(). Are they to be addressed in another PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I am planning to do those separately since there are also many uses.

private static final CRIUDumpPermission CRIU_DUMP_PERMISSION = new CRIUDumpPermission();
/*[ENDIF] JAVA_SPEC_VERSION < 24 */
private static boolean nativeLoaded;
private static boolean initComplete;
private static String errorMsg;
Expand Down Expand Up @@ -307,16 +309,19 @@ private static void init() {
* @param imageDir the directory that will hold the dump files as a
* java.nio.file.Path
* @throws NullPointerException if imageDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access imageDir or no
* CRIU_DUMP_PERMISSION
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if imageDir is not a valid directory
*/
public InternalCRIUSupport(Path imageDir) {
/*[IF JAVA_SPEC_VERSION < 24]*/
SecurityManager manager = System.getSecurityManager();
if (manager != null) {
manager.checkPermission(CRIU_DUMP_PERMISSION);
}

/*[ENDIF] JAVA_SPEC_VERSION < 24 */
setImageDir(imageDir);
}

Expand Down Expand Up @@ -431,7 +436,9 @@ public InternalCRIUSupport setGhostFileLimit(long limit) {
* @param imageDir the directory as a java.nio.file.Path
* @return this
* @throws NullPointerException if imageDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access imageDir
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if imageDir is not a valid directory
*/
public InternalCRIUSupport setImageDir(Path imageDir) {
Expand All @@ -441,10 +448,12 @@ public InternalCRIUSupport setImageDir(Path imageDir) {
}
String dir = imageDir.toAbsolutePath().toString();

/*[IF JAVA_SPEC_VERSION < 24]*/
SecurityManager manager = System.getSecurityManager();
if (manager != null) {
manager.checkWrite(dir);
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */

this.imageDir = dir;
return this;
Expand Down Expand Up @@ -592,7 +601,9 @@ public InternalCRIUSupport setTrackMemory(boolean trackMemory) {
* @param workDir the directory as a java.nio.file.Path
* @return this
* @throws NullPointerException if workDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access workDir
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if workDir is not a valid directory
*/
public InternalCRIUSupport setWorkDir(Path workDir) {
Expand All @@ -602,10 +613,12 @@ public InternalCRIUSupport setWorkDir(Path workDir) {
}
String dir = workDir.toAbsolutePath().toString();

/*[IF JAVA_SPEC_VERSION < 24]*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems

openj9/jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/CRIUSupport.java

Lines 38 to 40 in 48e4702

/*[IF JAVA_SPEC_VERSION >= 17]*/
@SuppressWarnings({ "deprecation", "removal" })
/*[ENDIF] JAVA_SPEC_VERSION >= 17 */

should be moved to this file and changed to

/*[IF (17 <= JAVA_SPEC_VERSION) & (JAVA_SPEC_VERSION < 24)]*/
@SuppressWarnings({ "deprecation", "removal" })
/*[ENDIF] (17 <= JAVA_SPEC_VERSION) & (JAVA_SPEC_VERSION < 24) */

because the deprecated System.getSecurityManager() usages are being removed.

SecurityManager manager = System.getSecurityManager();
if (manager != null) {
manager.checkWrite(dir);
}
/*[ENDIF] JAVA_SPEC_VERSION < 24 */

this.workDir = dir;
return this;
Expand Down
2 changes: 1 addition & 1 deletion jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/CRIUDumpPermission.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*[INCLUDE-IF CRIU_SUPPORT]*/
/*[INCLUDE-IF CRIU_SUPPORT & (JAVA_SPEC_VERSION < 24)]*/
/*
* Copyright IBM Corp. and others 2021
*
Expand Down
6 changes: 6 additions & 0 deletions jcl/src/openj9.criu/share/classes/org/eclipse/openj9/criu/CRIUSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,10 @@ public static enum HookMode {
* @param imageDir the directory that will hold the dump files as a
* java.nio.file.Path
* @throws NullPointerException if imageDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access imageDir or no
* CRIU_DUMP_PERMISSION
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if imageDir is not a valid directory
*/
public CRIUSupport(Path imageDir) {
Expand Down Expand Up @@ -141,7 +143,9 @@ public static String getErrorMessage() {
* @param imageDir the directory as a java.nio.file.Path
* @return this
* @throws NullPointerException if imageDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access imageDir
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if imageDir is not a valid directory
*/
public CRIUSupport setImageDir(Path imageDir) {
Expand Down Expand Up @@ -283,7 +287,9 @@ public CRIUSupport setTrackMemory(boolean trackMemory) {
* @param workDir the directory as a java.nio.file.Path
* @return this
* @throws NullPointerException if workDir is null
/*[IF JAVA_SPEC_VERSION < 24]
* @throws SecurityException if no permission to access workDir
/*[ENDIF] JAVA_SPEC_VERSION < 24
* @throws IllegalArgumentException if workDir is not a valid directory
*/
public CRIUSupport setWorkDir(Path workDir) {
Expand Down