Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:55:35 GMT, Jaikiran Pai wrote: >> We already do, see >> https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 > > This is slightly different from what we do in the other PR for unsafe memory > access where we specify the default in the launcher's help text too > https://github.com/openjdk/jdk/pull/19174/files#diff-799093930b698e97b23ead98c6496261af1e2e33ec7aa9261584870cbee8a5eaR219. > > I don't have a strong opinion on this, I think either one is fine. Ah, apologies, I was looking in the wrong place. I agree that we should specify default in the launcher, as well as in the man pages. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603233038
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:47:13 GMT, Maurizio Cimadamore wrote: >> src/java.base/share/classes/sun/launcher/resources/launcher.properties line >> 72: >> >>> 70: \ by code in modules for which native access is not >>> explicitly enabled.\n\ >>> 71: \ is one of "deny", "warn" or "allow".\n\ >>> 72: \ This option will be removed in a future release.\n\ >> >> Should this specify the current default value for this option if it isn't >> set? > > We already do, see > https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 This is slightly different from what we do in the other PR for unsafe memory access where we specify the default in the launcher's help text too https://github.com/openjdk/jdk/pull/19174/files#diff-799093930b698e97b23ead98c6496261af1e2e33ec7aa9261584870cbee8a5eaR219. I don't have a strong opinion on this, I think either one is fine. - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603205279
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Thu, 16 May 2024 11:42:48 GMT, Jaikiran Pai wrote: > Hello Maurizio, in the current mainline, we have code in `LauncherHelper` > https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/launcher/LauncherHelper.java#L636 > where we enable native access to all unnamed modules if an executable jar > with `Enable-Native-Access: ALL-UNNAMED` manifest is being launched. For such > executable jars, what is the expected semantics when the launch also > explicitly has a `--enable-native-access=M1,M2` option. Something like: > > ``` > java --enable-native-access=M1,M2 -jar foo.jar > ``` > > where `foo.jar` has `Enable-Native-Access: ALL-UNNAMED` in its manifest. The options are additive - e.g. the enable-native-access in the manifest will add up to the enable-native-access in the command line, so effectively it will be as if you were running with --enable-native-access=M1,M2,ALL-UNNAMED > src/java.base/share/classes/sun/launcher/resources/launcher.properties line > 72: > >> 70: \ by code in modules for which native access is not >> explicitly enabled.\n\ >> 71: \ is one of "deny", "warn" or "allow".\n\ >> 72: \ This option will be removed in a future release.\n\ > > Should this specify the current default value for this option if it isn't set? We already do, see https://github.com/openjdk/jdk/pull/19213/files/1c45e5d56c429205ab8185481bc1044a86ab3bc6#diff-d05029afe6aed86f860a10901114402f1f6af4fe1e4b46d883141ab1d2a527b8R582 - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2115012361 PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603195671
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Wed, 15 May 2024 16:08:17 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment Hello Maurizio, in the current mainline, we have code in `LauncherHelper` https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/launcher/LauncherHelper.java#L636 where we enable native access to all unnamed modules if an executable jar with `Enable-Native-Access: ALL-UNNAMED` manifest is being launched. For such executable jars, what is the expected semantics when the launch also explicitly has a `--enable-native-access=M1,M2` option. Something like: java --enable-native-access=M1,M2 -jar foo.jar where `foo.jar` has `Enable-Native-Access: ALL-UNNAMED` in its manifest. - PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2115005638
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
On Wed, 15 May 2024 16:08:17 GMT, Maurizio Cimadamore wrote: >> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting >> the use of JNI in the following ways: >> >> * `System::load` and `System::loadLibrary` are now restricted methods >> * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods >> * binding a JNI `native` method declaration to a native implementation is >> now considered a restricted operation >> >> This PR slightly changes the way in which the JDK deals with restricted >> methods, even for FFM API calls. In Java 22, the single >> `--enable-native-access` was used both to specify a set of modules for which >> native access should be allowed *and* to specify whether illegal native >> access (that is, native access occurring from a module not specified by >> `--enable-native-access`) should be treated as an error or a warning. More >> specifically, an error is only issued if the `--enable-native-access flag` >> is used at least once. >> >> Here, a new flag is introduced, namely >> `illegal-native-access=allow/warn/deny`, which is used to specify what >> should happen when access to a restricted method and/or functionality is >> found outside the set of modules specified with `--enable-native-access`. >> The default policy is `warn`, but users can select `allow` to suppress the >> warnings, or `deny` to cause `IllegalCallerException` to be thrown. This >> aligns the treatment of restricted methods with other mechanisms, such as >> `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. >> >> Some changes were required in the package-info javadoc for >> `java.lang.foreign`, to reflect the changes in the command line flags >> described above. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Address review comment src/java.base/share/classes/sun/launcher/resources/launcher.properties line 72: > 70: \ by code in modules for which native access is not > explicitly enabled.\n\ > 71: \ is one of "deny", "warn" or "allow".\n\ > 72: \ This option will be removed in a future release.\n\ Should this specify the current default value for this option if it isn't set? - PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1603157916
Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v6]
> This PR implements [JEP 472](https://openjdk.org/jeps/472), by restricting > the use of JNI in the following ways: > > * `System::load` and `System::loadLibrary` are now restricted methods > * `Runtime::load` and `Runtime::loadLibrary` are now restricted methods > * binding a JNI `native` method declaration to a native implementation is now > considered a restricted operation > > This PR slightly changes the way in which the JDK deals with restricted > methods, even for FFM API calls. In Java 22, the single > `--enable-native-access` was used both to specify a set of modules for which > native access should be allowed *and* to specify whether illegal native > access (that is, native access occurring from a module not specified by > `--enable-native-access`) should be treated as an error or a warning. More > specifically, an error is only issued if the `--enable-native-access flag` is > used at least once. > > Here, a new flag is introduced, namely > `illegal-native-access=allow/warn/deny`, which is used to specify what should > happen when access to a restricted method and/or functionality is found > outside the set of modules specified with `--enable-native-access`. The > default policy is `warn`, but users can select `allow` to suppress the > warnings, or `deny` to cause `IllegalCallerException` to be thrown. This > aligns the treatment of restricted methods with other mechanisms, such as > `--illegal-access` and the more recent `--sun-misc-unsafe-memory-access`. > > Some changes were required in the package-info javadoc for > `java.lang.foreign`, to reflect the changes in the command line flags > described above. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Address review comment - Changes: - all: https://git.openjdk.org/jdk/pull/19213/files - new: https://git.openjdk.org/jdk/pull/19213/files/daf729f4..1c45e5d5 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19213=05 - incr: https://webrevs.openjdk.org/?repo=jdk=19213=04-05 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19213.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19213/head:pull/19213 PR: https://git.openjdk.org/jdk/pull/19213
