On Mon, 13 May 2024 10:42:26 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
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.

Javadoc: 
https://cr.openjdk.org/~mcimadamore/jdk/8331671/v1/javadoc/api/index.html
Specdiff: 
https://cr.openjdk.org/~mcimadamore/jdk/8331671/v1/specdiff_out/overview-summary.html

make/conf/module-loader-map.conf line 105:

> 103:     java.smartcardio \
> 104:     jdk.accessibility \
> 105:     jdk.attach \

The list of allowed modules has been rewritten from scratch, by looking at the 
set of modules containing at least one `native` method declaration.

src/hotspot/share/prims/nativeLookup.cpp line 277:

> 275: 
> 276:   Klass*   klass = vmClasses::ClassLoader_klass();
> 277:   Handle jni_class(THREAD, method->method_holder()->java_mirror());

This is the biggest change in this PR. That is, we need to pass enough 
arguments to `ClassLoader::findNative` so that the method can start a 
restricted check accordingly.

src/java.base/share/classes/java/lang/Module.java line 311:

> 309:         Module target = moduleForNativeAccess();
> 310:         ModuleBootstrap.IllegalNativeAccess illegalNativeAccess = 
> ModuleBootstrap.illegalNativeAccess();
> 311:         if (illegalNativeAccess != 
> ModuleBootstrap.IllegalNativeAccess.ALLOW &&

There are some changes in this code:
* this code is no-op if `--illegal-native-access` is set to `allow`
* we also attach the location of the problematic class to the warning message, 
using `CodeSource`
* we use the "initial error stream" to emit the warning, similarly to what is 
done for other runtime warnings

src/java.base/share/classes/jdk/internal/reflect/Reflection.java line 115:

> 113:     @ForceInline
> 114:     public static void ensureNativeAccess(Class<?> currentClass, 
> Class<?> owner, String methodName) {
> 115:         if (VM.isModuleSystemInited()) {

If we call this code too early, we can see cases where `module` is `null`.

src/java.desktop/macosx/classes/com/apple/eio/FileManager.java line 61:

> 59:     }
> 60: 
> 61:     @SuppressWarnings({"removal", "restricted"})

There are several of these changes. One option might have been to just disable 
restricted warnings when building. But on a deeper look, I realized that in all 
these places we already disabled deprecation warnings for the use of security 
manager, so I also added a new suppression instead.

test/jdk/java/foreign/enablenativeaccess/panama_jni_load_module/module-info.java
 line 24:

> 22:  */
> 23: 
> 24: module panama_jni_load_module {

This module setup is a bit convoluted, but I wanted to make sure that we got 
separate warnings for `System.loadLibrary` and binding of the `native` method, 
and that warning on the _use_ of the native method was not generated 
(typically, all three operations occur in the same module).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19213#issuecomment-2107272261
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598269825
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598271285
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598274987
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598276455
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598277853
PR Review Comment: https://git.openjdk.org/jdk/pull/19213#discussion_r1598279827

Reply via email to