jdk
jdk copied to clipboard
8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)
Progress
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
- [ ] Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)
Issue
- JDK-8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) (Bug - P4)
Reviewers
- Kevin Walls (@kevinjwalls - Reviewer) ⚠️ Review applies to 8b200fc7
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/19055/head:pull/19055
$ git checkout pull/19055
Update a local copy of the PR:
$ git checkout pull/19055
$ git pull https://git.openjdk.org/jdk.git pull/19055/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 19055
View PR using the GUI difftool:
$ git pr show -t 19055
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/19055.diff
Webrev
:wave: Welcome back slovdahl! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.
@slovdahl This change now passes all automated pre-integration checks.
ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.
After integration, the commit message for the final commit will be:
8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)
Co-authored-by: Larry Cable <[email protected]>
Reviewed-by: kevinw, sgehwolf
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.
At the time when this comment was updated there had been 6 new commits pushed to the master branch:
- 988a531b097ccbd699d233059d73f41cae24dc5b: 8340181: Shenandoah: Cleanup ShenandoahRuntime stubs
- 822a773873c42ea27a6be90da92b2b2c9fb8caee: 8340605: Open source several AWT PopupMenu tests
- 6514aef8403fa5fc09e5c064a783ff0f1fccd0cf: 8340419: ZGC: Create an UseLargePages adaptation of TestAllocateHeapAt.java
- ae4d2f15901bf02efceaac26ee4aa3ae666bf467: 8340621: Open source several AWT List tests
- dd56990962d58e4f482773f67bc43383d7748536: 8340639: Open source few more AWT List tests
- ade17ecb6cb5125d048401a878b557e5afefc08c: 8340560: Open Source several AWT/2D font and rendering tests
Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinjwalls, @jerboaa) but any other Committer may sponsor as well.
➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@slovdahl The following labels will be automatically applied to this pull request:
hotspot-runtimeserviceability
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.
Webrevs
- 06: Full - Incremental (0afbf5d2)
- 05: Full - Incremental (543036bf)
- 04: Full - Incremental (8b200fc7)
- 03: Full - Incremental (c625411b)
- 02: Full - Incremental (c57b4598)
- 01: Full - Incremental (d3e26a0c)
- 00: Full (d37423fc)
This is a first stab at fixing the regression introduced in #17628. There has been a bit of discussion in https://mail.openjdk.org/pipermail/serviceability-dev/2024-April/055317.html and https://mail.openjdk.org/pipermail/serviceability-dev/2024-May/055364.html about exactly how to solve it, but this first attempt requires quite few changes at least.
Ran the following tests locally:
$ make test TEST="jtreg:test/hotspot/jtreg/containers"
...
==============================
Test summary
==============================
TEST TOTAL PASS FAIL ERROR
jtreg:test/hotspot/jtreg/containers 14 14 0 0
==============================
TEST SUCCESS
$ make test TEST="jtreg:test/hotspot/jtreg/serviceability"
...
==============================
Test summary
==============================
TEST TOTAL PASS FAIL ERROR
jtreg:test/hotspot/jtreg/serviceability 374 374 0 0
==============================
TEST SUCCESS
And also manually tested it under various conditions. Basic environment information:
slovdahl@ubuntu2204:~/reproducer$ systemd --version
systemd 249 (249.11-0ubuntu3.12)
+PAM +AUDIT +SELINUX +APPARMOR +IMA +SMACK +SECCOMP +GCRYPT +GNUTLS +OPENSSL +ACL +BLKID +CURL +ELFUTILS +FIDO2 +IDN2 -IDN +IPTC +KMOD +LIBCRYPTSETUP +LIBFDISK +PCRE2 -PWQUALITY -P11KIT -QRENCODE +BZIP2 +LZ4 +XZ +ZLIB +ZSTD -XKBCOMMON +UTMP +SYSVINIT default-hierarchy=unified
slovdahl@ubuntu2204:~/reproducer$ sudo apt-get install openjdk-17-jdk-headless
slovdahl@ubuntu2204:~/reproducer$ /usr/lib/jvm/java-17-openjdk-amd64/bin/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@ubuntu2204:~/reproducer$ /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/bin/java -version
openjdk version "23-internal" 2024-09-17
OpenJDK Runtime Environment (build 23-internal-adhoc.slovdahl.jdk)
OpenJDK 64-Bit Server VM (build 23-internal-adhoc.slovdahl.jdk, mixed mode, sharing)
Reproducer.java used for the target process:
import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.ServerSocket;
public class Reproducer {
public static void main(String[] args) throws InterruptedException, IOException {
int port;
if (args.length > 0) {
port = Integer.parseInt(args[0]);
}
else {
port = 81;
}
System.out.println("Hello, World!");
try (var server = new ServerSocket()) {
server.bind(new InetSocketAddress("localhost", port));
System.out.println("Bound to port " + port);
while (true) {
Thread.sleep(1_000L);
}
}
}
}
systemd unit used for testing the fix for https://bugs.openjdk.org/browse/JDK-8226919:
slovdahl@ubuntu2204:~/reproducer$ cat reproducer.service
[Service]
Type=simple
ExecStart=/usr/lib/jvm/java-17-openjdk-amd64/bin/java /home/slovdahl/reproducer/Reproducer.java
User=slovdahl
Group=slovdahl
ReadWritePaths=/tmp
AmbientCapabilities=CAP_NET_BIND_SERVICE
slovdahl@ubuntu2204:~/reproducer$ sudo cp -a reproducer.service /etc/systemd/system/
slovdahl@ubuntu2204:~/reproducer$ sudo systemctl daemon-reload
slovdahl@ubuntu2204:~/reproducer$ sudo systemctl start reproducer.service
slovdahl@ubuntu2204:~/reproducer$ sudo systemctl status reproducer.service
● reproducer.service
Loaded: loaded (/etc/systemd/system/reproducer.service; static)
Active: active (running) since Wed 2024-05-01 20:22:01 EEST; 4s ago
Main PID: 1576835 (java)
Tasks: 26 (limit: 76968)
Memory: 69.2M
CPU: 832ms
CGroup: /system.slice/reproducer.service
└─1576835 /usr/lib/jvm/java-17-openjdk-amd64/bin/java /home/slovdahl/reproducer/Reproducer.java
maj 01 20:22:01 ubuntu2204 systemd[1]: Started reproducer.service.
maj 01 20:22:01 ubuntu2204 java[1576835]: Hello, World!
maj 01 20:22:01 ubuntu2204 java[1576835]: Bound to port 81
slovdahl@ubuntu2204:~/reproducer$ ls -lh /proc/1576835/root
ls: cannot read symbolic link '/proc/1576835/root': Permission denied
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 1 20:22 /proc/1576835/root
slovdahl@ubuntu2204:~/reproducer$ sudo ls -lh /proc/1576835/root
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 1 20:22 /proc/1576835/root -> /
Fails with vanilla OpenJDK 17:
slovdahl@ubuntu2204:~/reproducer$ /usr/lib/jvm/java-17-openjdk-amd64/bin/jcmd 1576835 VM.version
1576835:
com.sun.tools.attach.AttachNotSupportedException: Unable to open socket file /proc/1576835/root/tmp/.java_pid1576835: target process 1576835 doesn't respond within 10500ms or HotSpot VM not loaded
at jdk.attach/sun.tools.attach.VirtualMachineImpl.<init>(VirtualMachineImpl.java:104)
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)
Works when attaching as root with vanilla OpenJDK 17:
slovdahl@ubuntu2204:~/reproducer$ sudo /usr/lib/jvm/java-17-openjdk-amd64/bin/jcmd 1576835 VM.version
1576835:
OpenJDK 64-Bit Server VM version 17.0.10+7-Ubuntu-122.04.1
JDK 17.0.10
Still works without root with a JDK built from this PR (fixed in https://bugs.openjdk.org/browse/JDK-8226919):
slovdahl@ubuntu2204:~/reproducer$ /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/bin/jcmd 1576835 VM.version
1576835:
OpenJDK 64-Bit Server VM version 17.0.10+7-Ubuntu-122.04.1
JDK 17.0.10
Attaching to a JVM inside a Docker container from the host works as before with vanilla OpenJDK 17 and the JDK built from this PR (always requires root):
slovdahl@ubuntu2204:~/reproducer$ docker run --rm -v .:/app -w /app eclipse-temurin:17 java Reproducer.java
Hello, World!
Bound to port 81
slovdahl@ubuntu2204:~/reproducer$ sudo jcmd -l | grep Reproducer
1587278 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
slovdahl@ubuntu2204:~/reproducer$ sudo /usr/lib/jvm/java-17-openjdk-amd64/bin/jcmd 1587278 VM.version
1587278:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
slovdahl@ubuntu2204:~/reproducer$ sudo /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/bin/jcmd 1587278 VM.version
1587278:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
Attaching to the same Docker container JVM from a sidecar container mounted into the same process namespace:
- Attaching works with Temurin 17, but
jcmd -ldoes not list the process - Attaching and
jcmd -lworks with Temurin 21 - Attaching and listing fails with current mainline JDK
- Attaching and listing works with the fix in this PR
slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm --pid=container:great_curie eclipse-temurin:17.0.11_9-jdk-jammy /bin/bash
root@e8fa333f7f71:/# jcmd
326 jdk.jcmd/sun.tools.jcmd.JCmd
root@e8fa333f7f71:/# jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm --pid=container:great_curie eclipse-temurin:21.0.3_9-jdk-jammy /bin/bash
root@d7c84d951f3d:/# jcmd
384 jdk.jcmd/sun.tools.jcmd.JCmd
1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
root@d7c84d951f3d:/# jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
# Mainline JDK
slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm --pid=container:great_curie --volume /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/:/jdk ubuntu:22.04 /bin/bash
root@10595dab1b4d:/# /jdk/bin/jcmd
1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
443 jdk.jcmd/sun.tools.jcmd.JCmd
root@10595dab1b4d:/# /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.<init>(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)
# JDK built from this PR
slovdahl@ubuntu2204:~/reproducer$ docker run --interactive --tty --rm --pid=container:great_curie --volume /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/:/jdk ubuntu:22.04 /bin/bash
root@7b7ef4ba0b35:/# /jdk/bin/jcmd
1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java
50 jdk.jcmd/sun.tools.jcmd.JCmd
root@7b7ef4ba0b35:/# /jdk/bin/jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
Mailing list message from Laurence Cable on serviceability-dev:
using pid to namespace comparison is IMO inappropriate/misleading what is being tested is the sharing of a common mount namespace, therefore the test should be comparing the "mnt" namespace ids.
Rgds
- Larry
On 5/2/24 3:22 AM, Sebastian L?vdahl wrote:
Mailing list message from Laurence Cable on serviceability-dev:
diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/V irtualMachineImpl.java index 81d4fd259ed..74bd60c791d 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -34,6 +34,7 @@ ?import java.nio.file.Path; ?import java.nio.file.Paths; ?import java.nio.file.Files; +import java.util.Optional;
?import static java.nio.charset.StandardCharsets.UTF_8;
@@ -47,7 +48,21 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { ???? // will not be able to find all Hotspot processes. ???? // Any changes to this needs to be synchronized with HotSpot. ???? private static final String tmpdir = "/tmp"; + +??? private static final Optional<Path> MOUNT_NS; + +??? static { +??????? Path mountns = null; +??????? try { +??????????? mountns = Files.readSymbolicLink(Path.of("/proc/self/ns/mnt")); +??????? } catch (IOException ioe) { +??????? } finally { +??????????? MOUNT_NS = Optional.ofNullable(mountns); +??????? } +??? } + ???? String socket_path; + ???? /** ????? * Attaches to the target VM ????? */ @@ -236,7 +251,18 @@ private File createAttachFile(int pid, int ns_pid) throws IOException {
???? private String findTargetProcessTmpDirectory(int pid, int ns_pid) throws IOException { ???????? String root; -??????? if (pid != ns_pid) { + +??????? Optional<Path> tgtMountNS = Optional.empty(); + +??????? try { +??????????? tgtMountNS = Optional.ofNullable(Files.readSymbolicLink(Path.of("/proc", Integer.toString(pid), "ns", "mnt"))); +??????? } catch (IOException _) { +????????? // do nothing... +??????? } + +??? final boolean sameMountNS = MOUNT_NS.isPresent() && tgtMountNS.isPresent() && MOUNT_NS.equals(tgtMountNS); + +??????? if (!sameMountNS || pid != ns_pid) { ???????????? // A process may not exist in the same mount namespace as the caller, e.g. ???????????? // if we are trying to attach to a JVM process inside a container. ???????????? // Instead, attach relative to the target root filesystem as exposed by @@ -248,11 +274,11 @@ private String findTargetProcessTmpDirectory(int pid, int ns_pid) throws IOExcep ?????????????????????????? "of target process %d", procRootDirectory, pid)); ???????????? }
-??????????? root = procRootDirectory + "/" + tmpdir; -??????? } else { -??????????? root = tmpdir; -??????? } -??????? return root; +??????????? return procRootDirectory + "/" + tmpdir; +??????? } else if (sameMountNS) { +??????????? return tmpdir; +??????? } else +??? ??? throw new IOException(String.format("target process:%d and this do not share common mount namespace for: %s attach faile d", pid, tmpdir)); ???? }
???? /*
Thanks for the patch @larry-cable, much appreciated! I really like this idea.
I tried it out a bit locally. These cases seem to work:
- attaching to a process running on the same host (PID and mount namespace the same)
- attaching as root from the host to a JVM inside a container
- attaching from a sidecar container to a JVM in another container
Unfortunately, attaching to a JVM process running as the same user in the same PID and mount namespace but one that has elevated privileges no longer works (the case that JDK-8226919 fixed). That case ends up failing like this with the patch:
slovdahl@ubuntu2204:~/reproducer$ /home/slovdahl/dev/external/jdk/build/linux-x86_64-server-release/images/jdk/bin/jcmd 1751545 VM.version
1751545:
java.io.IOException: Unable to access root directory /proc/1751545/root of target process 1751545
at jdk.attach/sun.tools.attach.VirtualMachineImpl.findTargetProcessTmpDirectory(VirtualMachineImpl.java:284)
at jdk.attach/sun.tools.attach.VirtualMachineImpl.findSocketFile(VirtualMachineImpl.java:229)
at jdk.attach/sun.tools.attach.VirtualMachineImpl.<init>(VirtualMachineImpl.java:86)
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)
I think it boils down to the same reason as why the fix for JDK-8226919 was needed in the first place - a non-root user cannot read the symlinks in /proc/<pid>/ns for a process running with more privileges even though it's run by the same non-root user.
slovdahl@ubuntu2204:/proc/1751545/ns$ ls -lh
ls: cannot read symbolic link 'net': Permission denied
ls: cannot read symbolic link 'uts': Permission denied
ls: cannot read symbolic link 'ipc': Permission denied
ls: cannot read symbolic link 'pid': Permission denied
ls: cannot read symbolic link 'pid_for_children': Permission denied
ls: cannot read symbolic link 'user': Permission denied
ls: cannot read symbolic link 'mnt': Permission denied
ls: cannot read symbolic link 'cgroup': Permission denied
ls: cannot read symbolic link 'time': Permission denied
ls: cannot read symbolic link 'time_for_children': Permission denied
total 0
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 cgroup
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 ipc
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:27 mnt
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 net
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 pid
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 pid_for_children
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 time
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 time_for_children
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 user
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 uts
slovdahl@ubuntu2204:/proc/1751545/ns$ sudo ls -lh
total 0
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 cgroup -> 'cgroup:[4026531835]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 ipc -> 'ipc:[4026531839]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:27 mnt -> 'mnt:[4026533862]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 net -> 'net:[4026531840]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 pid -> 'pid:[4026531836]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 pid_for_children -> 'pid:[4026531836]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 time -> 'time:[4026531834]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 time_for_children -> 'time:[4026531834]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 user -> 'user:[4026531837]'
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 3 19:28 uts -> 'uts:[4026531838]'
FWIW, my IntelliJ also highlighted the fact that the suggested patch contains unreachable code. The else throw new IOException(String.format("target process:%d and this do not share common mount namespace for: %s attach failed", pid, tmpdir)); path can never be taken, because either the if statement evaluates to true, or the else if.
I'm not sure if we can do any better than always falling back to /tmp? But if anyone has suggestions, I'm certainly happy to try it out.
I think it boils down to the same reason as why the fix for JDK-8226919 was needed in the first place - a non-root user cannot read the symlinks in
/proc/<pid>/nsfor a process running with more privileges even though it's run by the same non-root user.
@slovdahl - In that test case (target JVM process has more privileges), where is the attach file created? Does jcmd end up writing it to /tmp? Or does /proc/<pid>/cwd work? Just curious whether the elevated-privileges scenario affects the attach file and socket file locations equally.
Mailing list message from Laurence Cable on serviceability-dev:
I'll ponder ... have a good weekend!
regardless I think the added check for mnt ns comparison "adds value" by expressing the constraints explicitly vs comparing pid & ns pid
Rgds
- Larry
On 5/3/24 9:45 AM, Sebastian L?vdahl wrote:
I think it boils down to the same reason as why the fix for JDK-8226919 was needed in the first place - a non-root user cannot read the symlinks in
/proc/<pid>/nsfor a process running with more privileges even though it's run by the same non-root user.@slovdahl - In that test case (target JVM process has more privileges), where is the attach file created? Does jcmd end up writing it to
/tmp? Or does/proc/<pid>/cwdwork? Just curious whether the elevated-privileges scenario affects the attach file and socket file locations equally.
Yes, the attach file ends up in /tmp. Elevated privileges means that at least /proc/<pid>/root, /proc/<pid>/cwd and /proc/<pid>/exe are unreadable.
slovdahl@ubuntu2204:/proc/942992$ ls -lh cwd root
ls: cannot read symbolic link 'cwd': Permission denied
ls: cannot read symbolic link 'root': Permission denied
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 6 19:06 cwd
lrwxrwxrwx 1 slovdahl slovdahl 0 maj 6 19:06 root
regardless I think the added check for mnt ns comparison "adds value" by expressing the constraints explicitly vs comparing pid & ns pid
Yep, agreed.
I pushed an updated attempt at this now with d3e26a0c444e06ba9757fd528d72d83f56cd098b. Local testing and make test TEST="jtreg:test/hotspot/jtreg/containers" + make test TEST="jtreg:test/hotspot/jtreg/serviceability" indicate that all the known use-cases work.
Still eager to see what you come up with @larry-cable. createAttachFile could still be improved for example. And I would also be interested in looking into writing some test for the elevated privileges case.
On 5/6/24 10:35 AM, Sebastian Lövdahl wrote:
I pushed an updated attempt at this now with d3e26a0 https://urldefense.com/v3/__https://github.com/openjdk/jdk/commit/d3e26a0c444e06ba9757fd528d72d83f56cd098b__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svPHZrq9ZQ$. Local testing and |make test TEST="jtreg:test/hotspot/jtreg/containers"| + |make test TEST="jtreg:test/hotspot/jtreg/serviceability"| indicate that all the known use-cases work.
Still eager to see what you come up with @larry-cable https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svNMPdGFLg$. |createAttachFile| could still be improved for example. And I would also be interested in looking into writing some test for the elevated privileges case.
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2096564990__;Iw!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svNUwHWtZA$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67SGOTAXCKY2TO2OBDDZA65N5AVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJWGU3DIOJZGA__;!!ACWV5N9M2RV99hQ!M_KzQgiC8WkHTfJnvTw6zsw7l0RACDgJU3ShDs0R1dAWE_IxEinuX1_Uqom0UPt96Bl6iEqHc-yUUmz5svO2ymONGw$. You are receiving this because you were mentioned.Message ID: @.***>
/* diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java index 81d4fd259ed..c148dbd61b7 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -34,6 +34,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.Files; +import java.util.Optional;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -46,8 +47,28 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // location is the same for all processes, otherwise the tools // will not be able to find all Hotspot processes. // Any changes to this needs to be synchronized with HotSpot. - private static final String tmpdir = "/tmp"; + private static final Path TMPDIR = Path.of("/tmp"); + + private static final Path PROC = Path.of("/proc"); + private static final Path NS_MNT = Path.of("ns/mnt"); + private static final Path SELF = PROC.resolve("self"); + private static final Path TMP = Path.of("tmp"); + + private static final Optional<Path> SELF_MNT_NS; + + static { + Path mountns = null; + try { + mountns = Files.readSymbolicLink(SELF.resolve(NS_MNT)); + } catch (IOException _) { + // do nothing + } finally { + SELF_MNT_NS = Optional.ofNullable(mountns); + } + } + String socket_path; + /** * Attaches to the target VM */ @@ -235,24 +256,36 @@ private File createAttachFile(int pid, int ns_pid) throws IOException { }
private String findTargetProcessTmpDirectory(int pid, int ns_pid) throws IOException { - String root; - if (pid != ns_pid) { + final var procPid = PROC.resolve(Integer.toString(pid)); + + Optional<Path> tgtMountNS = Optional.empty(); + + try { + tgtMountNS = Optional.ofNullable(Files.readSymbolicLink(procPid.resolve(NS_MNT))); // attempt to read the tgt's mnt ns id... + } catch (IOException _) { // if we fail to read the tgt's mnt ns id then we either dont have access or it no longer exists! + if (!Files.exists(procPid)) throw new IOException(String.format("unable to attach, %s non-existent! process: %d terminated", procPid, pid)); + + // ok so if we get here we have failed to read the tgt's mnt ns, but the tgt process still exists ... we do not have privs to read its procfs + } + + final boolean sameMountNS = SELF_MNT_NS.isPresent() && SELF_MNT_NS.equals(tgtMountNS); // will be false if we did not read the tgt's mnt ns + + if (!sameMountNS || pid != ns_pid) { // A process may not exist in the same mount namespace as the caller, e.g. // if we are trying to attach to a JVM process inside a container. // Instead, attach relative to the target root filesystem as exposed by // procfs regardless of namespaces. - String procRootDirectory = "/proc/" + pid + "/root"; - if (!Files.isReadable(Path.of(procRootDirectory))) { - throw new IOException( - String.format("Unable to access root directory %s " + - "of target process %d", procRootDirectory, pid)); + final var procRootDirectory = procPid.resolve("root"); + + if (Files.isReadable(procRootDirectory)) + return procRootDirectory.resolve(TMP).toString(); + else if (Files.exists(procPid)) { // process is still alive... so its a permissions issue... fallback but this may also fail + return TMPDIR.toString(); + } else { + throw new IOException(String.format("Unable to access root directory %s " + "of target process %d", procRootDirectory.toString(), pid)); }
- root = procRootDirectory + "/" + tmpdir; - } else { - root = tmpdir; - } - return root; + } else + return TMPDIR.toString(); // we are in the same mnt ns }
/*
--------------ZiY5EhYyDoaC0Fy7Oxy0DC0T Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
I pushed an updated attempt at this now with d3e26a0. Local testing and
make test TEST="jtreg:test/hotspot/jtreg/containers"+make test TEST="jtreg:test/hotspot/jtreg/serviceability"indicate that all the known use-cases work.Still eager to see what you come up with .
createAttachFilecould still be improved for example. And I would also be interested in looking into writing some test for the elevated privileges case.—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: <openjdk/jdk/pull/19055/c2096564990@github.com>
/*
diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
index 81d4fd259ed..c148dbd61b7 100644
--- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
+++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
@@ -34,6 +34,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.Files;
+import java.util.Optional;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -46,8 +47,28 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine {
// location is the same for all processes, otherwise the tools
// will not be able to find all Hotspot processes.
// Any changes to this needs to be synchronized with HotSpot.
- private static final String tmpdir = "/tmp";
+ private static final Path TMPDIR = Path.of("/tmp");
+
+ private static final Path PROC = Path.of("/proc");
+ private static final Path NS_MNT = Path.of("ns/mnt");
+ private static final Path SELF = PROC.resolve("self");
+ private static final Path TMP = Path.of("tmp");
+
+ private static final Optional<Path> SELF_MNT_NS;
+
+ static {
+ Path mountns = null;
+ try {
+ mountns = Files.readSymbolicLink(SELF.resolve(NS_MNT));
+ } catch (IOException _) {
+ // do nothing
+ } finally {
+ SELF_MNT_NS = Optional.ofNullable(mountns);
+ }
+ }
+
String socket_path;
+
/**
* Attaches to the target VM
*/
@@ -235,24 +256,36 @@ private File createAttachFile(int pid, int ns_pid) throws IOException {
}
private String findTargetProcessTmpDirectory(int pid, int ns_pid) throws IOException {
- String root;
- if (pid != ns_pid) {
+ final var procPid = PROC.resolve(Integer.toString(pid));
+
+ Optional<Path> tgtMountNS = Optional.empty();
+
+ try {
+ tgtMountNS = Optional.ofNullable(Files.readSymbolicLink(procPid.resolve(NS_MNT))); // attempt to read the tgt's mnt ns id...
+ } catch (IOException _) { // if we fail to read the tgt's mnt ns id then we either dont have access or it no longer exists!
+ if (!Files.exists(procPid)) throw new IOException(String.format("unable to attach, %s non-existent! process: %d terminated", procPid, pid));
+
+ // ok so if we get here we have failed to read the tgt's mnt ns, but the tgt process still exists ... we do not have privs to read its procfs
+ }
+
+ final boolean sameMountNS = SELF_MNT_NS.isPresent() && SELF_MNT_NS.equals(tgtMountNS); // will be false if we did not read the tgt's mnt ns
+
+ if (!sameMountNS || pid != ns_pid) {
// A process may not exist in the same mount namespace as the caller, e.g.
// if we are trying to attach to a JVM process inside a container.
// Instead, attach relative to the target root filesystem as exposed by
// procfs regardless of namespaces.
- String procRootDirectory = "/proc/" + pid + "/root";
- if (!Files.isReadable(Path.of(procRootDirectory))) {
- throw new IOException(
- String.format("Unable to access root directory %s " +
- "of target process %d", procRootDirectory, pid));
+ final var procRootDirectory = procPid.resolve("root");
+
+ if (Files.isReadable(procRootDirectory))
+ return procRootDirectory.resolve(TMP).toString();
+ else if (Files.exists(procPid)) { // process is still alive... so its a permissions issue... fallback but this may also fail
+ return TMPDIR.toString();
+ } else {
+ throw new IOException(String.format("Unable to access root directory %s " + "of target process %d", procRootDirectory.toString(), pid));
}
-
- root = procRootDirectory + "/" + tmpdir;
- } else {
- root = tmpdir;
- }
- return root;
+ } else
+ return TMPDIR.toString(); // we are in the same mnt ns
}
/*
--------------ZiY5EhYyDoaC0Fy7Oxy0DC0T--
Mailing list message from Laurence Cable on serviceability-dev:
just a thought ... what if the code were to recurse "up" the process tree if the target JVM had elevated privs (and its /proc/... was not accessible), in order to find an ancestor whose /proc was accessible, since the JVM itself does not modify its pid or mnt namespaces (or capabilities) only an ancestor process could elevate the JVM.
once an ancestor with a readable /proc/.../root/tmp was located, that "proc" could be interrogated to determine if the target JVM was present in its namespace by validating that the target nspid was present in therein, then the attach could use the /proc/<ancestor>/root/tmp as the "attach"directory path instead of falling back on "/tmp" which potentially is not in the same mnt ns as the attacher?
of course the recursion would stop at "1" ...
thoughts?
- Larry
On 5/6/24 10:37 AM, Sebastian L?vdahl wrote:
Mailing list message from Laurence Cable on serviceability-dev:
On 5/3/24 10:43 AM, jdoylei wrote:
On Thu, 2 May 2024 10:13:51 GMT, Sebastian L?vdahl <duke at openjdk.org> wrote:
8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container) I think it boils down to the same reason as why the fix for JDK-8226919 was needed in the first place - a non-root user cannot read the symlinks in `/proc/<pid>/ns` for a process running with more privileges even though it's run by the same non-root user. @slovdahl - In that test case (target JVM process has more privileges), where is the attach file created? Does jcmd end up writing it to `/tmp`? Or does `/proc/<pid>/cwd` work? Just curious whether the elevated-privileges scenario affects the attach file and socket file locations equally.
note that the use of 'cwd' is a historical artifact, while the .attach_<pid> file can be written to 'cwd' the actual attach socket file .java_<pid> is always written to /tmp
Mailing list message from Laurence Cable on serviceability-dev:
I did some thinking on this issue over the weekend and came up with an idea that *may* improve the probability of an attach succeeding in the case that the target has elevated privileges and the jcmd is not in the same mnt namespace as the target JVM.
basically, the idea is to recurse "up"the process hierarchy from the target JVM process looking for either occupancy of the same mnt namespace (meaning that using /tmp to rendezvous on the attach socket will succeed) or in the case where the target JVM process has elevated privileges and thus the jcmd cannot determine if it shares a mnt ns, or cannot read/write the /proc/<target_pid>/root/tmp directory.
In this case, the attach code walks "up" the process hierarchy looking for the closest ancestor of the target JVM that either occupies the same mnt ns, or with a r/w /proc/<ancestor_pid>/root/tmp
since the JVM does not manipulate its pid nor mnt ns'es or modify it's (linux) capabilities, if such has occurred then it was caused by an ancestor process of the target (in the case of a container this is most likely the container manager or a delegate thereof.
should the jcmd find either an ancestor in the same mnt ns (/tmp) or a r/w /proc/<ancestor_pid>/root/tmp it will return that path as the directory in which to rendezvous with the target JVM.
this approach "increases the odds" that the jcmd will successfully attach to a containerized and/or elevated privilege JVM.
needless to say this is "experimental" and needs proper stress testing for the appropriate use cases.
Rgds
- Larry
-------------- next part -------------- diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java index 81d4fd259ed..050d8bbb2a9 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -34,6 +34,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.Files; +import java.util.Optional;
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -46,13 +47,45 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // location is the same for all processes, otherwise the tools // will not be able to find all Hotspot processes. // Any changes to this needs to be synchronized with HotSpot. - private static final String tmpdir = "/tmp"; + private static final Path TMPDIR = Path.of("/tmp"); + + private static final Path PROC = Path.of("/proc"); + private static final Path NS_MNT = Path.of("ns/mnt"); + private static final Path NS_PID = Path.of("ns/pid"); + private static final Path SELF = PROC.resolve("self"); + private static final Path TMP = Path.of("tmp"); + private static final Path STATUS = Path.of("status"); + private static final Path ROOT_TMP = Path.of("root/tmp"); + + private static final Optional<Path> SELF_MNT_NS; + private static final Optional<Path> SELF_PID_NS; + + static { + Path nsPath = null; + + try { + nsPath = Files.readSymbolicLink(SELF.resolve(NS_MNT)); + } catch (IOException _) { + // do nothing + } finally { + SELF_MNT_NS = Optional.ofNullable(nsPath); + } + + try { + nsPath = Files.readSymbolicLink(SELF.resolve(NS_PID)); + } catch (IOException _) { + // do nothing + } finally { + SELF_PID_NS = Optional.ofNullable(nsPath); + } + } + String socket_path; + /** * Attaches to the target VM */ - VirtualMachineImpl(AttachProvider provider, String vmid) - throws AttachNotSupportedException, IOException + VirtualMachineImpl(AttachProvider provider, String vmid) throws AttachNotSupportedException, IOException { super(provider, vmid);
@@ -63,12 +96,12 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { }
// Try to resolve to the "inner most" pid namespace - int ns_pid = getNamespacePid(pid); + final long ns_pid = getNamespacePid(pid);
// Find the socket file. If not found then we attempt to start the // attach mechanism in the target VM by sending it a QUIT signal. // Then we attempt to find the socket file again. - File socket_file = findSocketFile(pid, ns_pid); + final File socket_file = findSocketFile(pid, ns_pid); socket_path = socket_file.getPath(); if (!socket_file.exists()) { // Keep canonical version of File, to delete, in case target process ends and /proc link has gone: @@ -210,49 +243,95 @@ protected void close(long fd) throws IOException { }
// Return the socket file for the given process. - private File findSocketFile(int pid, int ns_pid) throws IOException { - String root = findTargetProcessTmpDirectory(pid, ns_pid); - return new File(root, ".java_pid" + ns_pid); + private File findSocketFile(long pid, long ns_pid) throws AttachNotSupportedException, IOException { + return new File(findTargetProcessTmpDirectory(pid, ns_pid), ".java_pid" + ns_pid); }
// On Linux a simple handshake is used to start the attach mechanism // if not already started. The client creates a .attach_pid<pid> file in the // target VM's working directory (or temp directory), and the SIGQUIT handler // checks for the file. - private File createAttachFile(int pid, int ns_pid) throws IOException { - String fn = ".attach_pid" + ns_pid; - String path = "/proc/" + pid + "/cwd/" + fn; - File f = new File(path); + private File createAttachFile(long pid, long ns_pid) throws AttachNotSupportedException, IOException { + Path fn = Path.of(".attach_pid" + ns_pid); + Path path = PROC.resolve(Path.of(Long.toString(pid), "cwd")).resolve(fn); + File f = new File(path.toString()); try { // Do not canonicalize the file path, or we will fail to attach to a VM in a container. f.createNewFile(); - } catch (IOException x) { - String root = findTargetProcessTmpDirectory(pid, ns_pid); - f = new File(root, fn); + } catch (IOException _) { + f = new File(findTargetProcessTmpDirectory(pid, ns_pid), fn.toString()); f.createNewFile(); } return f; }
- private String findTargetProcessTmpDirectory(int pid, int ns_pid) throws IOException { - String root; - if (pid != ns_pid) { - // A process may not exist in the same mount namespace as the caller, e.g. - // if we are trying to attach to a JVM process inside a container. - // Instead, attach relative to the target root filesystem as exposed by - // procfs regardless of namespaces. - String procRootDirectory = "/proc/" + pid + "/root"; - if (!Files.isReadable(Path.of(procRootDirectory))) { - throw new IOException( - String.format("Unable to access root directory %s " + - "of target process %d", procRootDirectory, pid)); + private String findTargetProcessTmpDirectory(long pid, long ns_pid) throws AttachNotSupportedException, IOException { + Optional<ProcessHandle> tgt = ProcessHandle.of(pid); + Optional<ProcessHandle> ph = tgt; + long nsPid = ns_pid; + Optional<Path> prevPidNS = Optional.empty(); + + while (ph.isPresent()) { + final var curPid = ph.get().pid(); + + final var procPidPath = PROC.resolve(Long.toString(curPid)); + + Optional<Path> tgtMountNS = Optional.empty(); + + try { + tgtMountNS = Optional.ofNullable(Files.readSymbolicLink(procPidPath.resolve(NS_MNT))); // attempt to read the tgt's mnt ns id... + } catch ( + IOException _) { // if we fail to read the tgt's mnt ns id then we either dont have access or it no longer exists! + if (!Files.exists(procPidPath)) + throw new IOException(String.format("unable to attach, %s non-existent! process: %d terminated", procPidPath, pid)); + + // ok so if we get here we have failed to read the tgt's mnt ns, but the tgt process still exists ... we do not have privs to read its procfs + } + + final var sameMountNS = SELF_MNT_NS.isPresent() && SELF_MNT_NS.equals(tgtMountNS); // will be false if we did not read the tgt's mnt ns + + if (sameMountNS) { + return TMPDIR.toString(); // we share TMPDIR in common! + } else { + final var procPidRootTmp = procPidPath.resolve(ROOT_TMP); + + if (Files.isReadable(procPidRootTmp)) return procPidRootTmp.toString(); // not in the same mnt ns but tmp is accessible via /proc... + } + + // lets attempt to obtain the pid NS... best efforts to avoid crossing pid ns boundaries (as with a container) + + Optional<Path> curPidNS = Optional.empty(); + + try { + curPidNS = Optional.ofNullable(Files.readSymbolicLink(procPidPath.resolve(NS_PID))); // attempt to read the tgt's mnt ns id... + } catch (IOException _) { // if we fail to read the tgt's pid ns id then we either dont have access or it no longer exists! + if (!Files.exists(procPidPath)) throw new IOException(String.format("unable to attach, %s non-existent! process: %d terminated", procPidPath, pid)); + + // ok so if we get here we have failed to read the tgt's pid ns, but the tgt process still exists ... we do not have privs to read its procfs + } + + // recurse "up" the process heirarchy... if appropriate + + final var havePidNSes = prevPidNS.isPresent() && curPidNS.isPresent(); + + final var ppid = ph.get().parent(); + + if (ppid.isPresent() && (havePidNSes && curPidNS.equals(prevPidNS)) || (!havePidNSes && nsPid > 1)) { + ph = ppid; + + nsPid = getNamespacePid(ph.get().pid()); // get the ns pid of the parent... + + prevPidNS = curPidNS; + } else { + ph = Optional.empty(); } + }
- root = procRootDirectory + "/" + tmpdir; + if (tgt.orElseThrow(AttachNotSupportedException::new).isAlive()) { + return TMPDIR.toString(); // fallback... } else { - root = tmpdir; + throw new IOException(String.format("unable to attach, process: %d terminated", pid)); } - return root; }
/* @@ -272,10 +351,10 @@ private void writeString(int fd, String s) throws IOException {
// Return the inner most namespaced PID if there is one, // otherwise return the original PID. - private int getNamespacePid(int pid) throws AttachNotSupportedException, IOException { + private long getNamespacePid(long pid) throws AttachNotSupportedException, IOException { // Assuming a real procfs sits beneath, reading this doesn't block // nor will it consume a lot of memory. - String statusFile = "/proc/" + pid + "/status"; + final var statusFile = PROC.resolve(Long.toString(pid)).resolve(STATUS).toString(); File f = new File(statusFile); if (!f.exists()) { return pid; // Likely a bad pid, but this is properly handled later. @@ -291,8 +370,7 @@ private int getNamespacePid(int pid) throws AttachNotSupportedException, IOExcep // The last entry represents the PID the JVM "thinks" it is. // Even in non-namespaced pids these entries should be // valid. You could refer to it as the inner most pid. - int ns_pid = Integer.parseInt(parts[parts.length - 1]); - return ns_pid; + return Long.parseLong(parts[parts.length - 1]); } } // Old kernels may not have NSpid field (i.e. 3.10).
Thanks for all the deep thinking you're doing here @larry-cable, appreciated. And sorry for the delay in my response, I'll try to get more time devoted to this during the coming week.
I did some thinking on this issue over the weekend and came up with an idea that may improve the probability of an attach succeeding in the case that the target has elevated privileges and the jcmd is not in the same mnt namespace as the target JVM.
I haven't fully digested the patches you have provided yet, but one question this far. 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. Or is there some way to attach from host to container with a non-root user that I'm missing?
One thing I would like to eventually achieve is to have JDK-8226919 and this fix backported to the current LTS releases. Would it make more sense to fix JDK-8327114 with as few changes as possible, and use that for backports, and do more extensive improvements in a separate follow-up that don't need backporting?
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. 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?
I tested your patch @larry-cable, and, as far as I can tell, everything works. I ran through all the cases in https://github.com/openjdk/jdk/pull/19055#issuecomment-2090136676 once more to verify this. Thanks! Feel free to take a look.
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/
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&group... because it has r/w access to the /proc/
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/
Rgds
- Larry
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2123042958__;Iw!!ACWV5N9M2RV99hQ!NbYaR1TB-qDG-x11SE_XRwyQgFwVKEPiL9gmrrFqxio1p2TAsJmsyBKhvICqMROIcMGpC8U7jfjenhr5WbKzJlBhjQ$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67XPE34N46VDFUEMSHDZDN3NHAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRTGA2DEOJVHA__;!!ACWV5N9M2RV99hQ!NbYaR1TB-qDG-x11SE_XRwyQgFwVKEPiL9gmrrFqxio1p2TAsJmsyBKhvICqMROIcMGpC8U7jfjenhr5WbLIEsaUnQ$. You are receiving this because you were mentioned.Message ID: @.***>
--------------XQdcZo9hO6wbcp2fGsjP1B9A Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
Hi Sebastian!<blockquote> <p dir="auto">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. </p> </blockquote> </blockquote> <br> the constraint (as I understand it) is based upon the filesystem access to /proc/<attachee>/root/tmp, where the createAttachFile fails... if the "attacher" (jcmd) has access, if it has the<br> appropriate +og r/w access then it will be successful.<br> <br> the "root" requirement comes from the default behavior of the container mgmt (docker) running containers as 'root'.<br> <br> 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&group... because it has r/w access to the /proc/<attachee>/root/tmp<br> <br> note: if the container is in a distinct uid ns (from the attacher) I think the current checks performed by <font face="Courier New, Courier, monospace"><b>os::Posix::matches_effective_uid_and_gid_or_root</b></font> 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!)<br> <br> <blockquote type="cite" ***@***.***"> <blockquote> <p dir="auto">Or is there some way to attach from host to container with a non-root user that I'm missing?</p> </blockquote> <p dir="auto">Or could it work in case the container is also run as a non-<code class="notranslate">root</code> user?</p> </blockquote> <br> 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<br> to /tmp will not work and since the attacher and the attachee do not share a fs...<br> <br> if the target JVM has elevated privs a (sidecar) attacher cannot use the target's /proc/<attachee>/root/... hence my experiment to recurse "up" the attachee's pid ns to look for a an un-privileged ancestor, which does<br> share the same mnt ns as the attachee, so the attacher can use the /proc/<ancestor>/root/tmp to attach to the target... all things being equal...<br> <br> Rgds<br> <br> - Larry<br> <br> <blockquote type="cite" ***@***.***"> <p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br> Reply to this email directly, <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2123042958__;Iw!!ACWV5N9M2RV99hQ!NbYaR1TB-qDG-x11SE_XRwyQgFwVKEPiL9gmrrFqxio1p2TAsJmsyBKhvICqMROIcMGpC8U7jfjenhr5WbKzJlBhjQ$" moz-do-not-send="true">view it on GitHub</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67XPE34N46VDFUEMSHDZDN3NHAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRTGA2DEOJVHA__;!!ACWV5N9M2RV99hQ!NbYaR1TB-qDG-x11SE_XRwyQgFwVKEPiL9gmrrFqxio1p2TAsJmsyBKhvICqMROIcMGpC8U7jfjenhr5WbLIEsaUnQ$" moz-do-not-send="true">unsubscribe</a>.<br> You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67Q4FA2GDVB3NAFU5I3ZDN3NHA5CNFSM6AAAAABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6RMII4.gif" alt="" moz-do-not-send="true" width="1" height="1"><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openjdk/jdk/pull/19055/c2123042958</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p> <script type="application/ld+json">[{ @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2123042958", "url": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2123042958", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]
--------------XQdcZo9hO6wbcp2fGsjP1B9A--
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.
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/[email protected]/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.<init>(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
Starting the target container with elevated privileges:
slovdahl@slovdahl-virtual-machine:~$ docker run --name reproducer --cap-add=CAP_NET_RAW --rm -v .:/app -w /app eclipse-temurin:17 java Reproducer.java
Hello, World!
Bound to port 81
slovdahl@slovdahl-virtual-machine:~$ sudo getpcaps 7332
7332: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
Attaching from a sidecar container with a 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@07d305e00ade:/# /jdk/bin/jcmd 1 VM.version
1:
OpenJDK 64-Bit Server VM version 17.0.11+9
JDK 17.0.11
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 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, view it on GitHub https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2124324590__;Iw!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufIAfBLoA$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67WR5PKR6GOLQ5YR4YTZDRRNVAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRUGMZDINJZGA__;!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufsOv7sDg$. 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
<p dir="auto">Thanks for the explanation <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/larry-cable/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufP1OzXcw$" ***@***.***</a>, that makes sense. By chance, did you already test the Docker <code class="notranslate">--user</code> 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.</p> <p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br> Reply to this email directly, <a href="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</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67WR5PKR6GOLQ5YR4YTZDRRNVAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRUGMZDINJZGA__;!!ACWV5N9M2RV99hQ!Itj8vWkvTqaQTLecUEfu_JZ6ABpWcUnNdgvMjd3DIa_lT3gj_sPLvswAzzOOr-hQRtP8oH0WAHwlplxItufsOv7sDg$" moz-do-not-send="true">unsubscribe</a>.<br> You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67SYZIQHARL7TOTRLHLZDRRNVA5CNFSM6AAAAABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6T2PO4.gif" alt="" moz-do-not-send="true" width="1" height="1"><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openjdk/jdk/pull/19055/c2124324590</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p> <script type="application/ld+json">[{ @.": "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 https://urldefense.com/v3/__https://docs.docker.com/engine/security/rootless__;!!ACWV5N9M2RV99hQ!ILXmNJIu4Gt289mnC35eZVQJfnEjBLKSgmLkd-qoanr6YXVS7KHjYw24PXzZbOdX1IT4HTFNICDXNnKMIoWGsleSZA$.
@.:~$ 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/ https://urldefense.com/v3/__https://docs.docker.com/go/rootless/__;!!ACWV5N9M2RV99hQ!ILXmNJIu4Gt289mnC35eZVQJfnEjBLKSgmLkd-qoanr6YXVS7KHjYw24PXzZbOdX1IT4HTFNICDXNnKMIoV88DoH1A$ Main PID: 3314 (rootlesskit) Tasks: 58 Memory: 596.4M CPU: 16.821s CGroup: @./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:
@.***:~$ 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:
@.:~$ 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) @.:~$ jcmd 4139 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 5965 jdk.jcmd/sun.tools.jcmd.JCmd @.***:~$ 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:
@.***:~$ /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:
@.***:~$ /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:
@.:~$ docker run --interactive --tty --rm --pid=container:reproducer eclipse-temurin:17.0.11_9-jdk-jammy /bin/bash @.:/# jcmd 44 jdk.jcmd/sun.tools.jcmd.JCmd @.***:/# 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):
@.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 234 jdk.jcmd/sun.tools.jcmd.JCmd @.***:/# /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:
@.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 154 jdk.jcmd/sun.tools.jcmd.JCmd @.***:/# /jdk/bin/jcmd 1 VM.version 1: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11 |
Starting the target container with elevated privileges:
@.:~$ docker run --name reproducer --cap-add=CAP_NET_RAW --rm -v .:/app -w /app eclipse-temurin:17 java Reproducer.java Hello, World! Bound to port 81 @.:~$ sudo getpcaps 7332 7332: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep |
Attaching from a sidecar container with a JDK built from this PR:
@.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 VM.version 1: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11 |
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2124549900__;Iw!!ACWV5N9M2RV99hQ!ILXmNJIu4Gt289mnC35eZVQJfnEjBLKSgmLkd-qoanr6YXVS7KHjYw24PXzZbOdX1IT4HTFNICDXNnKMIoUiCnhWUw$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67VDJPJFFMPA7SF2YJDZDR5SLAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRUGU2DSOJQGA__;!!ACWV5N9M2RV99hQ!ILXmNJIu4Gt289mnC35eZVQJfnEjBLKSgmLkd-qoanr6YXVS7KHjYw24PXzZbOdX1IT4HTFNICDXNnKMIoWSMOTYwg$. You are receiving this because you were mentioned.Message ID: @.***>
--------------0k0wZisxTpAKNaSxRp0B1RTM Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
it lives ...it lives!!!I love it when a patch comes together!
:)
thx for testing this before my 1dt cup of coffee!
Rgds
- Larry
<p dir="auto">I set up rootless Docker in a VM by following <a href="https://urldefense.com/v3/__https://docs.docker.com/engine/security/rootless__;!!ACWV5N9M2RV99hQ!ILXmNJIu4Gt289mnC35eZVQJfnEjBLKSgmLkd-qoanr6YXVS7KHjYw24PXzZbOdX1IT4HTFNICDXNnKMIoWGsleSZA$" rel="nofollow" moz-do-not-send="true">https://docs.docker.com/engine/security/rootless</a>.</p> <pre class="notranslate"><code ***@***.***:~$ 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: @./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:
<code @.:~$ docker run --name reproducer --rm -v .:/app -w /app eclipse-temurin:17 java Reproducer.java Hello, World! Bound to port 81Using the Ubuntu OpenJDK 17 package:
<code @.***:~$ 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)@.***:~$ jcmd 4139 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 5965 jdk.jcmd/sun.tools.jcmd.JCmd
@.:~$ 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:
<code @.:~$ /jdk/bin/jcmd 4139 VM.version 4139: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11Using JDK built from this PR:
<code @.:~$ /jdk/bin/jcmd 4139 VM.version 4139: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11Using a sidecar container mounted into the same PID namespace with Eclipse Temurin 17:
<code @.:~$ docker run --interactive --tty --rm --pid=container:reproducer eclipse-temurin:17.0.11_9-jdk-jammy /bin/bash @.:/# jcmd 44 jdk.jcmd/sun.tools.jcmd.JCmd @.:/# jcmd 1 VM.version 1: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11Using a sidecar container mounted into the same PID namespace with mainline JDK (expected to fail):
<code @.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 234 jdk.jcmd/sun.tools.jcmd.JCmd @.:/# /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.<init>(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:
<code @.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 jdk.compiler/com.sun.tools.javac.launcher.Main Reproducer.java 154 jdk.jcmd/sun.tools.jcmd.JCmd @.:/# /jdk/bin/jcmd 1 VM.version 1: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11Starting the target container with elevated privileges:
<code @.***:~$ docker run --name reproducer --cap-add=CAP_NET_RAW --rm -v .:/app -w /app eclipse-temurin:17 java Reproducer.java Hello, World! Bound to port 81@.:~$ sudo getpcaps 7332 7332: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep
Attaching from a sidecar container with a JDK built from this PR:
<code @.:~$ docker run --interactive --tty --rm --pid=container:reproducer --volume /jdk/:/jdk ubuntu:22.04 /bin/bash @.:/# /jdk/bin/jcmd 1 VM.version 1: OpenJDK 64-Bit Server VM version 17.0.11+9 JDK 17.0.11—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.
--------------0k0wZisxTpAKNaSxRp0B1RTM--
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://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 :)
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 - LarryI 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 https://urldefense.com/v3/__https://man7.org/linux/man-pages/man7/capabilities.7.html__;!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0JV3zd5SA$ 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 https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2125541556__;Iw!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0JG0EA7Zg$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67VJZL3MIT2HANZ3BLDZDTTG7AVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRVGU2DCNJVGY__;!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0IYrO2-pA$. You are receiving this because you were mentioned.Message ID: @.***>
--------------Rdb42IWaMAGxS5O004yPY6ws Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
<blockquote> <p dir="auto">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</p> </blockquote> <p dir="auto">I know for sure that <code class="notranslate">CAP_NET_BIND_SERVICE</code> prevents access to <code class="notranslate">/proc/<pid>/root</code> 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 <em>any</em> privilege restricts access to <code class="notranslate">/proc/<pid>/root</code> (among others). But right now I cannot recall what gave me that impression. There's a long list of capabilities though: <a href="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</a></p> <blockquote> <p dir="auto">it lives ...it lives!!!</p> <p dir="auto">I love it when a patch comes together!</p> <p dir="auto">:)</p> <p dir="auto">thx for testing this before my 1dt cup of coffee!</p> </blockquote> <p dir="auto">Great feeling indeed! Ah, the best cup of the day, have a good one :)</p> </blockquote> <br> likewise Slainte Mhath!<br> <br> - Larry<br> <br> <blockquote type="cite" ***@***.***"> <p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br> Reply to this email directly, <a href="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</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67VJZL3MIT2HANZ3BLDZDTTG7AVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRVGU2DCNJVGY__;!!ACWV5N9M2RV99hQ!OuFFfoYFVnGvARkAQ11WdUPoVHR3GXEc-XbeZfOWFHFrQAJxR6-suOx9_j-qekgTrr5V66CAb7K0i0zi_0IYrO2-pA$" moz-do-not-send="true">unsubscribe</a>.<br> You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67VXC2SXHYIOCXNVH3DZDTTG7A5CNFSM6AAAAABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6WEYLI.gif" alt="" moz-do-not-send="true" width="1" height="1"><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openjdk/jdk/pull/19055/c2125541556</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p> <script type="application/ld+json">[{ @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2125541556", "url": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2125541556", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]
--------------Rdb42IWaMAGxS5O004yPY6ws--
@larry-cable gentle ping, did you get a chance to test it any further?
Maybe @jerboaa and/or @kevinjwalls that reviewed #17628 / JDK-8226919 would like to take a look at this fix as well?
Maybe it's getting a bit late now, but it would be really awesome if we could get this to land before RDP 1 (on Thursday the 6th), so we avoid regressing any use-cases in the upcoming JDK 23.
Hi Sebastian, sadly no I haven't. :(
it would be good to get it in, it would be good if @kevinjwalls could take a look.
as with regressions, I think as long as it passes the current set of tests then there are unlikely to be any regressions.
we really need a test to validate:
-
attach to elevated JVM
-
attach across container boundary a) to elevated JVM
- Larry
On 6/2/24 9:02 AM, Sebastian Lövdahl wrote:
@larry-cable https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUMt6ya6Ww$ gentle ping, did you get a chance to test it any further?
Maybe @jerboaa https://urldefense.com/v3/__https://github.com/jerboaa__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUNoOyPEJg$ and/or @kevinjwalls https://urldefense.com/v3/__https://github.com/kevinjwalls__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUNeZyFX-w$ that reviewed #17628 https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/17628__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUP26kWhGg$ / JDK-8226919 https://bugs.openjdk.org/browse/JDK-8226919 would like to take a look at this fix as well?
Maybe it's getting a bit late now, but it would be really awesome if we could get this to land before RDP 1 (on Thursday the 6th), so we avoid regressing any use-cases in the upcoming JDK 23.
— Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2143912533__;Iw!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUPnr406TQ$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67WAGBRLIDGAFST4RNDZFM6XRAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTHEYTENJTGM__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUM4ibFsiw$. You are receiving this because you were mentioned.Message ID: @.***>
--------------JJW8ycnoTBYUYFaUv43Mvg08 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit
Hi Sebastian, sadly no I haven't. :(it would be good to get it in, it would be good if @kevinjwalls could take a look.
as with regressions, I think as long as it passes the current set of tests then there are unlikely to be any regressions.
we really need a test to validate:
1) attach to elevated JVM
2) attach across container boundary
a) to elevated JVM
- Larry
<p dir="auto"><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/larry-cable/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urldefense.com/v3/__https://github.com/larry-cable__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUMt6ya6Ww$" ***@***.***</a> gentle ping, did you get a chance to test it any further?</p> <p dir="auto">Maybe <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/jerboaa/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urldefense.com/v3/__https://github.com/jerboaa__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUNoOyPEJg$" ***@***.***</a> and/or <a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/kevinjwalls/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urldefense.com/v3/__https://github.com/kevinjwalls__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUNeZyFX-w$" ***@***.***</a> that reviewed <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="2107520726" data-permission-text="Title is private" data-url="https://github.com/openjdk/jdk/issues/17628" data-hovercard-type="pull_request" data-hovercard-url="/openjdk/jdk/pull/17628/hovercard" href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/17628__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUP26kWhGg$" moz-do-not-send="true">#17628</a> / <a href="https://bugs.openjdk.org/browse/JDK-8226919" rel="nofollow" moz-do-not-send="true">JDK-8226919</a> would like to take a look at this fix as well?</p> <p dir="auto">Maybe it's getting a bit late now, but it would be really awesome if we could get this to land before RDP 1 (on Thursday the 6th), so we avoid regressing any use-cases in the upcoming JDK 23.</p> <p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br> Reply to this email directly, <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19055*issuecomment-2143912533__;Iw!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUPnr406TQ$" moz-do-not-send="true">view it on GitHub</a>, or <a href="https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ANTA67WAGBRLIDGAFST4RNDZFM6XRAVCNFSM6AAAAABHDNNTT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBTHEYTENJTGM__;!!ACWV5N9M2RV99hQ!LoZom5Qy8VCk9HqSqZZs1Puzt4Xaxwg1m1jhO_nw42rjeedWQiRNnG8KtRl1zulrnLYqYuV0TsTTXexnzUM4ibFsiw$" moz-do-not-send="true">unsubscribe</a>.<br> You are receiving this because you were mentioned.<img src="https://github.com/notifications/beacon/ANTA67R7T4ZH5NC7VU3HFN3ZFM6XRA5CNFSM6AAAAABHDNNTT6WGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT7ZGBFK.gif" alt="" moz-do-not-send="true" width="1" height="1"><span style="color: transparent; font-size: 0; display: none; visibility: hidden; overflow: hidden; opacity: 0; width: 0; height: 0; max-width: 0; max-height: 0; mso-hide: all">Message ID: <span><openjdk/jdk/pull/19055/c2143912533</span><span>@</span><span>github</span><span>.</span><span>com></span></span></p> <script type="application/ld+json">[{ @.": "http://schema.org", @.": "EmailMessage", "potentialAction": { @.": "ViewAction", "target": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2143912533", "url": "https://github.com/openjdk/jdk/pull/19055#issuecomment-2143912533", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { @.": "Organization", "name": "GitHub", "url": "https://github.com" } } ]
--------------JJW8ycnoTBYUYFaUv43Mvg08--
Hi Larry, no worries. :)
I can try to look into writing some tests for the elevated use-cases. but it will be quite much treading of new ground for me, so it could take some time to get it done.
What's your take, do we need the new tests in this PR, or could it be done in a follow-up?