Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]

2024-05-22 Thread Kevin Rushforth
On Fri, 17 May 2024 13:38:25 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 comments

I tested this with JavaFX and everything is working as I would expect. Without 
any options, I get the expected warnings, one time per modules for the three 
`javafx.*` modules that use JNI. If I pass the `--enable-native-access` options 
at runtime, listing those three modules, there is no warning. Further, I 
confirm that if I pass that option to jlink or jpackage when creating a custom 
runtime, there is no warning.

-

Marked as reviewed by kcr (Author).

PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2072430338


Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v3]

2024-05-22 Thread Larry Cable
On Tue, 21 May 2024 17:10:15 GMT, Sebastian Lövdahl  wrote:

>> 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid 
>> (Kubernetes debug container)
>
> Sebastian Lövdahl has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Remove unused `SELF_PID_NS`
>  - Rewrite in line with suggestion from Larry Cable

On 5/22/24 11:58 AM, Sebastian Lövdahl wrote:
>
> I haven't but I will BTW which linux capabilities should be
> enabled in order to prevent a /proc/... style attach due to lack
> of permissions to access target's /proc fs? Rgds - Larry
>
> I know for sure that |CAP_NET_BIND_SERVICE| prevents access to 
> |/proc//root| at least. I don't know if there's any distinction 
> between the different privileges a process can have to be honest, but 
> I somehow got the impression that having /any/ privilege restricts 
> access to |/proc//root| (among others). But right now I cannot 
> recall what gave me that impression. There's a long list of 
> capabilities though: 
> https://man7.org/linux/man-pages/man7/capabilities.7.html 
> 
>
> it lives ...it lives!!!
>
> I love it when a patch comes together!
>
> :)
>
> thx for testing this before my 1dt cup of coffee!
>
> Great feeling indeed! Ah, the best cup of the day, have a good one :)
>

likewise Slainte Mhath!

- Larry

> —
> Reply to this email directly, view it on GitHub 
> ,
>  
> or unsubscribe 
> .
> You are receiving this because you were mentioned.Message ID: 
> ***@***.***>
>

--Rdb42IWaMAGxS5O004yPY6ws
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit



  
  


On 5/22/24 11:58 AM, Sebastian Lövdahl
  wrote:


  
  
I haven't but I will BTW which linux capabilities
  should be enabled in order to prevent a /proc/... style attach
  due to lack of permissions to access target's /proc fs? Rgds -
  Larry
  
  I know for sure that CAP_NET_BIND_SERVICE
prevents access to /proc/pid/root
at least. I don't know if there's any distinction between the
different privileges a process can have to be honest, but I
somehow got the impression that having any privilege
restricts access to /proc/pid/root
(among others). But right now I cannot recall what gave me that
impression. There's a long list of capabilities though: https://urldefense.com/v3/__https://man7.org/linux/man-pages/man7/capabilities.7.html__;!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0JV3zd5SA$;
 rel="nofollow" 
moz-do-not-send="true">https://man7.org/linux/man-pages/man7/capabilities.7.html
  
it lives ...it lives!!!
I love it when a patch comes together!
:)
thx for testing this before my 1dt cup of coffee!
  
  Great feeling indeed! Ah, the best cup of the day,
have a good one :)


likewise Slainte Mhath!

- Larry


  —
Reply to this email directly, https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2125541556__;Iw!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0JG0EA7Zg$;
 moz-do-not-send="true">view it on GitHub, or https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67VJZL3MIT2HANZ3BLDZDTTG7AVCNFSM6ABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRVGU2DCNJVGY__;!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0IYrO2-pA$;
 moz-do-not-send="true">unsubscribe.
You are receiving this because you were mentioned.https://github.com/notifications/beacon/ANTA67VXC2SXHYIOCXNVH3DZDTTG7A5CNFSM6ABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6WEYLI.gif;
 alt="" moz-do-not-send="true" width="1" height="1">Message
  ID: 
openjdk/jdk/pull/19055/c2125541556@github.com
  [
{
***@***.***": "http://schema.org";,
***@***.***": "EmailMessage",
"potentialAction": {
***@***.***": "ViewAction",
"target": "

Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v3]

2024-05-22 Thread Sebastian Lövdahl
On Wed, 22 May 2024 18:40:00 GMT, Larry Cable  wrote:

> I haven't but I will BTW which linux capabilities should be enabled in order 
> to prevent a /proc/... style attach due to lack of permissions to access 
> target's /proc fs? Rgds - Larry

I know for sure that `CAP_NET_BIND_SERVICE` prevents access to 
`/proc//root` at least. I don't know if there's any distinction between 
the different privileges a process can have to be honest, but I somehow got the 
impression that having _any_ privilege restricts access to `/proc//root` 
(among others). But right now I cannot recall what gave me that impression. 
There's a long list of capabilities though: 
https://man7.org/linux/man-pages/man7/capabilities.7.html

> it lives ...it lives!!!
>
> I love it when a patch comes together!
> 
> :)
> 
> thx for testing this before my 1dt cup of coffee!

Great feeling indeed! Ah, the best cup of the day, have a good one :)

-

PR Comment: https://git.openjdk.org/jdk/pull/19055#issuecomment-2125541556


Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v3]

2024-05-22 Thread Larry Cable
On Tue, 21 May 2024 17:10:15 GMT, Sebastian Lövdahl  wrote:

>> 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid 
>> (Kubernetes debug container)
>
> Sebastian Lövdahl has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Remove unused `SELF_PID_NS`
>  - Rewrite in line with suggestion from Larry Cable

I haven't but I will BTW which linux capabilities should be enabled in 
order to prevent a /proc/... style attach due to lack of permissions to 
access target's /proc fs?

Rgds

- Larry

On 5/22/24 2:37 AM, Sebastian Lövdahl wrote:
>
> Thanks for the explanation @larry-cable 
> ,
>  
> that makes sense. By chance, did you already test the Docker |--user| 
> case with the suggested patch? I don't have access to an environment 
> with rootless Docker at hand, but I may be able to set it up in a VM 
> if you didn't already test it.
>
> —
> Reply to this email directly, view it on GitHub 
> ,
>  
> or unsubscribe 
> .
> You are receiving this because you were mentioned.Message ID: 
> ***@***.***>
>

--p6E1JhKjfAr6K0U0BUrS5J3x
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit



  
  
I haven't but I will BTW which linux capabilities should be enabled
in order to prevent a /proc/... style attach due to lack of
permissions to access target's /proc fs?

Rgds

- Larry

On 5/22/24 2:37 AM, Sebastian Lövdahl
  wrote:


  
  Thanks for the explanation https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufP1OzXcw$;
 ***@***.***, that makes sense. By
chance, did you already test the Docker --user case with the suggested
patch? I don't have access to an environment with rootless
Docker at hand, but I may be able to set it up in a VM if you
didn't already test it.
  —
Reply to this email directly, https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2124324590__;Iw!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufIAfBLoA$;
 moz-do-not-send="true">view it on GitHub, or https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67WR5PKR6GOLQ5YR4YTZDRRNVAVCNFSM6ABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRUGMZDINJZGA__;!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufsOv7sDg$;
 moz-do-not-send="true">unsubscribe.
You are receiving this because you were mentioned.https://github.com/notifications/beacon/ANTA67SYZIQHARL7TOTRLHLZDRRNVA5CNFSM6ABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6T2PO4.gif;
 alt="" moz-do-not-send="true" width="1" height="1">Message
  ID: 
openjdk/jdk/pull/19055/c2124324590@github.com
  [
{
***@***.***": "http://schema.org";,
***@***.***": "EmailMessage",
"potentialAction": {
***@***.***": "ViewAction",
"target": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2124324590";,
"url": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2124324590";,
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
***@***.***": "Organization",
"name": "GitHub",
"url": "https://github.com";
}
}
]


  


--p6E1JhKjfAr6K0U0BUrS5J3x--

it lives ...it lives!!!

I love it when a patch comes together!

:)

thx for testing this before my 1dt cup of coffee!

Rgds

- Larry

On 5/22/24 4:21 AM, Sebastian Lövdahl wrote:
>
> I set up rootless Docker in a VM by following 
> https://docs.docker.com/engine/security/rootless 
> .
>
> ***@***.***:~$ systemctl status 

Re: RFR: 8328866: Add raw monitor rank support to the debug agent. [v7]

2024-05-22 Thread Chris Plummer
On Wed, 22 May 2024 17:43:53 GMT, Chris Plummer  wrote:

>>> Is that "then popFrameEventLock second"
>>> 
>> Yes. I'll fix.
>> 
>>> Drawing these out in two columns I can't see a deadlock either 8-)
>> 
>> Ironically right now I'm looking at a very rare deadlock that involves this 
>> code. It doesn't seem to happen when I disabled ranked locking. It might be 
>> instigated by the dbgRawMonitor that ranked locking uses.
>
>> Ironically right now I'm looking at a very rare deadlock that involves this 
>> code. It doesn't seem to happen when I disabled ranked locking. It might be 
>> instigated by the dbgRawMonitor that ranked locking uses.
> 
> @kevinjwalls I tracked down this deadlock and filed 
> [JDK-8332738](https://bugs.openjdk.org/browse/JDK-8332738). It's really a 
> pre-existing issue, but we get lucky with the current implementation because 
> RawMonitorExit does not self suspend until after releasing the monitor, thus 
> avoiding the deadlock. The ranked monitors implementation adds a self suspend 
> opportunity when we release a raw monitor, which is it hits this bug. This is 
> a very ugly issue that involves two threads getting events at the same time, 
> and the debugger doing a StackFrame.PopFrames. Fortunately the "pop frames" 
> locks are not involved.

I should also add that this issue was only turning up when I had virtual 
threads enabled, because that caused one of the threads to trigger a class load 
of some CarrierThread inner class the first time it parked. This generated a 
ClassPrepare event with unfortunately timing (it would be on Thread 2 of my 
description in [JDK-8332738](https://bugs.openjdk.org/browse/JDK-8332738)). 
However, I now have a much more direct test that doesn't require virtual 
threads and more readily reproduces the deadlock.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1610413014


Re: RFR: 8328866: Add raw monitor rank support to the debug agent. [v7]

2024-05-22 Thread Chris Plummer
On Thu, 16 May 2024 20:02:22 GMT, Chris Plummer  wrote:

> Ironically right now I'm looking at a very rare deadlock that involves this 
> code. It doesn't seem to happen when I disabled ranked locking. It might be 
> instigated by the dbgRawMonitor that ranked locking uses.

@kevinjwalls I tracked down this deadlock and filed 
[JDK-8332738](https://bugs.openjdk.org/browse/JDK-8332738). It's really a 
pre-existing issue, but we get lucky with the current implementation because 
RawMonitorExit does not self suspend until after releasing the monitor, thus 
avoiding the deadlock. The ranked monitors implementation adds a self suspend 
opportunity when we release a raw monitor, which is it hits this bug. This is a 
very ugly issue that involves two threads getting events at the same time, and 
the debugger doing a StackFrame.PopFrames. Fortunately the "pop frames" locks 
are not involved.

-

PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1610404920


Re: RFR: 8332641: Update nsk.share.jpda.Jdb to don't use finalization

2024-05-22 Thread Chris Plummer
On Tue, 21 May 2024 21:49:51 GMT, Leonid Mesnik  wrote:

> The nsk.share.jdb.Jdb has finalize() nethods that close jdb connection and 
> output streams.
> 
> The fix renames the method to close() and calls it explicitly after the test 
> finishes. I verified that close() called for each nsk share jdb test.
> 
> The jdb is also LocalProcess with it's own cleanup(). This part still remains 
> the same so far.

Looks good.

-

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19336#pullrequestreview-2071834856


Re: RFR: 8332631: Update nsk.share.jpda.BindServer to don't use finalization

2024-05-22 Thread Chris Plummer
On Tue, 21 May 2024 19:55:01 GMT, Leonid Mesnik  wrote:

> The BindServer starts several threads and opens streams.
> 
> It registered them for cleanup using "Finalizer" from nsk.share.framework. 
> Currently, it cleanup resources during shutdown hook.
> 
> This fix changes BindServer to explicitly close streams and finish threads 
> after test is completed. The exceptions are just printed like it was done 
> previously. I haven't caught any exception during in close method during 
> testing.

Looks good. Just one minor comment suggestion.

test/hotspot/jtreg/vmTestbase/nsk/share/jpda/BindServer.java line 634:

> 632: /**
> 633:  * Close thread by closing all connections and waiting
> 634:  * for thread finished.

Suggestion:

 * for thread to finish.

-

Marked as reviewed by cjplummer (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19335#pullrequestreview-2071799883
PR Review Comment: https://git.openjdk.org/jdk/pull/19335#discussion_r1610313535


Integrated: 8332071: Convert package.html files in `java.management.rmi` to package-info.java

2024-05-22 Thread Nizar Benalla
On Thu, 16 May 2024 10:39:43 GMT, Nizar Benalla  wrote:

> Please review this change. I converted the `package.html` file to 
> `package-info.java`, because `javac` cannot recognize `package.html`.
> I already brought this up [in the mailing 
> list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html).
> The conversion was done in-place, only renaming it in git.
> 
> I also added a couple of `@since` tags, with only 2 changes I don't want to 
> split these two fixes into separate PRs.
> `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in 
> https://bugs.openjdk.org/browse/JDK-8187556

This pull request has now been integrated.

Changeset: a0c5714d
Author:Nizar Benalla 
Committer: Kevin Walls 
URL:   
https://git.openjdk.org/jdk/commit/a0c5714dbc8a60d905f9deea153e7f31fbd64d06
Stats: 675 lines in 3 files changed: 330 ins; 329 del; 16 mod

8332071: Convert package.html files in `java.management.rmi` to 
package-info.java
8332376: Add `@since` tags to `java.management.rmi`

Reviewed-by: kevinw, rriggs

-

PR: https://git.openjdk.org/jdk/pull/19263


Re: RFR: 8332071: Convert package.html files in `java.management.rmi` to package-info.java [v5]

2024-05-22 Thread Roger Riggs
On Mon, 20 May 2024 20:55:16 GMT, Nizar Benalla  wrote:

>> Please review this change. I converted the `package.html` file to 
>> `package-info.java`, because `javac` cannot recognize `package.html`.
>> I already brought this up [in the mailing 
>> list](https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055650.html).
>> The conversion was done in-place, only renaming it in git.
>> 
>> I also added a couple of `@since` tags, with only 2 changes I don't want to 
>> split these two fixes into separate PRs.
>> `CREDENTIALS_FILTER_PATTERN` and `SERIAL_FILTER_PATTERN` were first added in 
>> https://bugs.openjdk.org/browse/JDK-8187556
>
> Nizar Benalla has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fix indentation

LGTM

-

Marked as reviewed by rriggs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19263#pullrequestreview-2071636919


Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v3]

2024-05-22 Thread Sebastian Lövdahl
On Tue, 21 May 2024 17:10:15 GMT, Sebastian Lövdahl  wrote:

>> 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid 
>> (Kubernetes debug container)
>
> Sebastian Lövdahl has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - Remove unused `SELF_PID_NS`
>  - Rewrite in line with suggestion from Larry Cable

I set up rootless Docker in a VM by following 
https://docs.docker.com/engine/security/rootless.


slovdahl@slovdahl-virtual-machine:~$ systemctl status --user docker.service 
● docker.service - Docker Application Container Engine (Rootless)
 Loaded: loaded (/home/slovdahl/.config/systemd/user/docker.service; 
enabled; vendor preset: enabled)
 Active: active (running) since Wed 2024-05-22 13:55:06 EEST; 5min ago
   Docs: https://docs.docker.com/go/rootless/
   Main PID: 3314 (rootlesskit)
  Tasks: 58
 Memory: 596.4M
CPU: 16.821s
 CGroup: 
/user.slice/user-1000.slice/user@1000.service/app.slice/docker.service
 ├─3314 rootlesskit --state-dir=/run/user/1000/dockerd-rootless 
--net=slirp4netns --mtu=65520 --slirp4netns-sandbox=auto 
--slirp4netns-seccomp=auto --disable-host-loopback --port-driver=builtin 
--copy-up=/etc --copy-up=/run --propagation=rslave /usr/bin/dockerd>
 ├─3325 /proc/self/exe --state-dir=/run/user/1000/dockerd-rootless 
--net=slirp4netns --mtu=65520 --slirp4netns-sandbox=auto 
--slirp4netns-seccomp=auto --disable-host-loopback --port-driver=builtin 
--copy-up=/etc --copy-up=/run --propagation=rslave /usr/bin/dock>
 ├─3343 slirp4netns --mtu 65520 -r 3 --disable-host-loopback 
--enable-sandbox --enable-seccomp 3325 tap0
 ├─3350 dockerd
 ├─3373 containerd --config 
/run/user/1000/docker/containerd/containerd.toml
 └─4116 /usr/bin/containerd-shim-runc-v2 -namespace moby -id 
3a84c6c9f7b8ee6220b8953b65ff56639dd51335999cb37580292f4944ee0e65 -address 
/run/user/1000/docker/containerd/containerd.sock


Started a container running as my user:

slovdahl@slovdahl-virtual-machine:~$ docker run --name reproducer --rm -v 
.:/app -w /app eclipse-temurin:17 java Reproducer.java
Hello, World!
Bound to port 81


Using the Ubuntu OpenJDK 17 package:

slovdahl@slovdahl-virtual-machine:~$ java -version
openjdk version "17.0.10" 2024-01-16
OpenJDK Runtime Environment (build 17.0.10+7-Ubuntu-122.04.1)
OpenJDK 64-Bit Server VM (build 17.0.10+7-Ubuntu-122.04.1, mixed mode, sharing)

slovdahl@slovdahl-virtual-machine:~$ jcmd
4139 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
5965 jdk.jcmd/sun.tools.jcmd.JCmd

slovdahl@slovdahl-virtual-machine:~$ jcmd 4139 VM.version
4139:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11


Using mainline JDK without the changes in this PR:

slovdahl@slovdahl-virtual-machine:~$ /jdk/bin/jcmd 4139 VM.version
4139:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11


Using JDK built from this PR:

slovdahl@slovdahl-virtual-machine:~$ /jdk/bin/jcmd 4139 VM.version
4139:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11


Using a sidecar container mounted into the same PID namespace with Eclipse 
Temurin 17:

slovdahl@slovdahl-virtual-machine:~$ docker run --interactive --tty --rm 
--pid=container:reproducer eclipse-temurin:17.0.11_9-jdk-jammy /bin/bash
root@b746aeae40d2:/# jcmd
44 jdk.jcmd/sun.tools.jcmd.JCmd
root@b746aeae40d2:/# jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11


Using a sidecar container mounted into the same PID namespace with mainline JDK 
(expected to fail):

slovdahl@slovdahl-virtual-machine:~$ docker run --interactive --tty --rm 
--pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash
root@7b0c9dc87175:/# /jdk/bin/jcmd
1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
234 jdk.jcmd/sun.tools.jcmd.JCmd
root@7b0c9dc87175:/# /jdk/bin/jcmd 1 VM.version
1:
com.sun.tools.attach.AttachNotSupportedException: Unable to open socket file 
/tmp/.java_pid1: target process 1 doesn't respond within 10500ms or HotSpot VM 
not loaded
at 
jdk.attach/sun.tools.attach.VirtualMachineImpl.(VirtualMachineImpl.java:99)
at 
jdk.attach/sun.tools.attach.AttachProviderImpl.attachVirtualMachine(AttachProviderImpl.java:58)
at 
jdk.attach/com.sun.tools.attach.VirtualMachine.attach(VirtualMachine.java:207)
at jdk.jcmd/sun.tools.jcmd.JCmd.executeCommandForPid(JCmd.java:113)
at jdk.jcmd/sun.tools.jcmd.JCmd.main(JCmd.java:97)


Using a sidecar container mounted into the same PID namespace with JDK built 
from this PR:

slovdahl@slovdahl-virtual-machine:~$ docker run --interactive --tty --rm 
--pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash
root@1ed0633e74eb:/# /jdk/bin/jcmd
1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
154 jdk.jcmd/sun.tools.jcmd.JCmd
root@1ed0633e74eb:/# /jdk/bin/jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11



Re: RFR: 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) [v3]

2024-05-22 Thread Sebastian Lövdahl
On Tue, 21 May 2024 21:06:22 GMT, Larry Cable  wrote:

>> Sebastian Lövdahl has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - Remove unused `SELF_PID_NS`
>>  - Rewrite in line with suggestion from Larry Cable
>
> Hi Sebastian!
> 
> On 5/21/24 9:50 AM, Sebastian Lövdahl wrote:
>>
>> In these cases, is it not a requirement that jcmd is run as root?
>> So even if the target process is run with elevated privileges,
>> attaching would always work.
>>
> 
> the constraint (as I understand it) is based upon the filesystem access 
> to /proc//root/tmp, where the createAttachFile fails... if the 
> "attacher" (jcmd) has access, if it has the
> appropriate +og r/w access then it will be successful.
> 
> the "root" requirement comes from the default behavior of the container 
> mgmt (docker) running containers as 'root'.
> 
> if you employ the --user option to 'force' the container to adopt a 
> non-root identity jcmd will succeed if issued from the same 
> user because it has r/w access to the /proc//root/tmp
> 
> note: if the container is in a distinct uid ns (from the attacher) I 
> think the current checks performed by 
> *os::Posix::matches_effective_uid_and_gid_or_root* will complete since 
> the comparison is based on the uid values returned by the O.S 
> (independent of the fact that the uid's may exist in distinct uid ns'es!)
> 
>> Or is there some way to attach from host to container with a
>> non-root user that I'm missing?
>>
>> Or could it work in case the container is also run as a non-|root| user?
>>
> 
> the use case I was addressing with my proposal is when the jcmd 
> "container" (as a sidecar) is in the same pid ns as the target container 
> but in a different mnt ns (I believe this is the regression use case) in 
> that case falling back
> to /tmp will not work and since the attacher and the attachee do not 
> share a fs...
> 
> if the target JVM has elevated privs a (sidecar) attacher cannot use the 
> target's /proc//root/... hence my experiment to recurse "up" 
> the attachee's pid ns to look for a an un-privileged ancestor, which does
> share the same mnt ns as the attachee, so the attacher can use the 
> /proc//root/tmp to attach to the target... all things being 
> equal...
> 
> Rgds
> 
> - Larry
> 
>> —
>> Reply to this email directly, view it on GitHub 
>> ,
>>  
>> or unsubscribe 
>> 

Re: RFR: 8331671: Implement JEP 472: Prepare to Restrict the Use of JNI [v8]

2024-05-22 Thread Magnus Ihse Bursie
On Fri, 17 May 2024 13:38:25 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 comments

Build changes look good. Thanks for trimming down NATIVE_ACCESS_MODULES.

-

Marked as reviewed by ihse (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19213#pullrequestreview-2070573791