search

ibmruntimes / openj9-openjdk-jdk11

Extensions for OpenJDK 11 for Eclipse OpenJ9
GNU General Public License v2.0
31 stars 112 forks source link

Fix AccessControlException in MT.fromMethodDescriptorString method #680

Closed dipak-bagadiya closed 1 year ago

dipak-bagadiya commented 1 year ago

If the code invokes the method MethodType.fromMethodDescriptorString(desc, null), it will throw an exception called AccessControlException: Access denied. This exception occurs when access to the system class loader is restricted. The cause of this exception is a SecurityException, which occurs if the security manager is present, the loader parameter is null, and the caller does not possess the RuntimePermission("getClassLoader").

To resolve the aforementioned issue or avoid the SecurityException, it is recommended to make the following changes as suggested in the JDK 14 documentation for MethodType.fromMethodDescriptorString

Update the method BytecodeDescriptor.parseSig to use Class.forName(name, false, loader).

Fixes: https://github.com/eclipse-openj9/openj9/issues/14555

Signed-off-by: Dipak Bagadiya dipak.bagadiya@ibm.com

pshipton commented 1 year ago

Pls use a full link for the issue, #14555 doesn't resolve to anything here.

dipak-bagadiya commented 1 year ago

@pshipton added full link for the issue.

babsingh commented 1 year ago

@dipak-bagadiya The title should not exceed 70 chars: https://github.com/eclipse-openj9/openj9/blob/master/CONTRIBUTING.md#commit-guidelines; the PR description and commit message should be corrected. Also, please post links to the personal builds highlighting that no side-effects are seen for OpenJ9 MHs and OpenJDK MHs.

@pshipton This change is a partial backport of a change which was added in JDK14+: openj9-openjdk-jdk14/commit/a20b0e118; only the BytecodeDescriptor changes are back-ported. Do we append an IBM copyright in this scenario?

dipak-bagadiya commented 1 year ago

@babsingh @pshipton

pshipton commented 1 year ago

Do we append an IBM copyright in this scenario?

We should not apply an IBM copyright when backporting OpenJDK changes, only if IBM added something to the backport.

babsingh commented 1 year ago

JDK11 Javadoc for MT.fromMethodDescriptorString explicitly mentions of using ClassLoader.loadClass. Changing from ClassLoader.loadClass to Class.forName would violate the JDK11 Javadoc.

JDK14 Javadoc for MT.fromMethodDescriptorString removes the restriction to use ClassLoader.loadClass.

So, we shouldn't backport the JDK14 changes to JDK11 unless there is a strong justification to support that using ClassLoader.loadClass is wrong here.

To fix the AccessControlException in https://github.com/eclipse-openj9/openj9/issues/14555, we should verify the workings of the OpenJ9 implementation of AccessController and compare it to the RI. Related to https://github.com/eclipse-openj9/openj9/issues/14555#issuecomment-1632762205. fyi @JasonFengJ9, for high level insights, if any.

dipak-bagadiya commented 1 year ago

@JasonFengJ9 Is there any relation between nonExportedPkgs packages and packages that we provide via --add-exports?

JasonFengJ9 commented 1 year ago

@dipak-bagadiya couple of points for further investigation:

dipak-bagadiya commented 1 year ago

@JasonFengJ9 ,

JasonFengJ9 commented 1 year ago

https://github.com/eclipse-openj9/openj9/issues/14555#issuecomment-1635027344 Run BasePlatformLoggerTest.java against JDK11 w/o MH enabled (default) w/ -Djava.security.debug=access:stack:permission=java/lang/RuntimePermission

access: access allowed ("java.lang.RuntimePermission" "accessClassInPackage.sun.util.logging")

The checkPackageAccess() was successful for sun.util.logging, it seems there are call stacks introduced w/o permission required.

JasonFengJ9 commented 1 year ago

I think this is superseded by

babsingh commented 1 year ago

Closing as per https://github.com/ibmruntimes/openj9-openjdk-jdk11/pull/680#issuecomment-1760297491.