jdk icon indicating copy to clipboard operation
jdk copied to clipboard

8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)

Open slovdahl opened this issue 1 year ago • 47 comments


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

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

Link to Webrev Comment

slovdahl avatar May 02 '24 10:05 slovdahl

: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.

bridgekeeper[bot] avatar May 02 '24 10:05 bridgekeeper[bot]

@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).

openjdk[bot] avatar May 02 '24 10:05 openjdk[bot]

@slovdahl The following labels will be automatically applied to this pull request:

  • hotspot-runtime
  • serviceability

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.

openjdk[bot] avatar May 02 '24 10:05 openjdk[bot]

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.

slovdahl avatar May 02 '24 10:05 slovdahl

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 -l does not list the process
  • Attaching and jcmd -l works 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

slovdahl avatar May 02 '24 10:05 slovdahl

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:

mlbridge[bot] avatar May 02 '24 20:05 mlbridge[bot]

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)); ???? }

???? /*

mlbridge[bot] avatar May 03 '24 05:05 mlbridge[bot]

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.

slovdahl avatar May 03 '24 16:05 slovdahl

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.

jdoylei avatar May 03 '24 17:05 jdoylei

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:

mlbridge[bot] avatar May 03 '24 17:05 mlbridge[bot]

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.

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.

slovdahl avatar May 06 '24 16:05 slovdahl

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.

slovdahl avatar May 06 '24 17:05 slovdahl

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



On 5/6/24 10:35 AM, Sebastian Lövdahl wrote:

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 . 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, 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--

larry-cable avatar May 06 '24 18:05 larry-cable

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:

mlbridge[bot] avatar May 08 '24 05:05 mlbridge[bot]

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

mlbridge[bot] avatar May 09 '24 03:05 mlbridge[bot]

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).

mlbridge[bot] avatar May 09 '24 03:05 mlbridge[bot]

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?

slovdahl avatar May 12 '24 18:05 slovdahl

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?

slovdahl avatar May 21 '24 16:05 slovdahl

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.

slovdahl avatar May 21 '24 17:05 slovdahl

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&group... 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 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!

On 5/21/24 9:50 AM, Sebastian Lövdahl wrote:
  <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/&lt;attachee&gt;/root/tmp, where the
createAttachFile fails... if the &quot;attacher&quot; (jcmd) has access, if it
has the<br>
appropriate +og r/w access then it will be successful.<br>
<br>
the &quot;root&quot; 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&amp;group... because it has r/w access to the
/proc/&lt;attachee&gt;/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
&quot;container&quot; (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/&lt;attachee&gt;/root/... hence my experiment to
recurse &quot;up&quot; 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/&lt;ancestor&gt;/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>&lt;openjdk/jdk/pull/19055/c2123042958</span><span>@</span><span>github</span><span>.</span><span>com&gt;</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--

larry-cable avatar May 21 '24 21:05 larry-cable

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.

slovdahl avatar May 22 '24 09:05 slovdahl

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

slovdahl avatar May 22 '24 11:05 slovdahl

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

On 5/22/24 2:37 AM, Sebastian Lövdahl wrote:
  <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>&lt;openjdk/jdk/pull/19055/c2124324590</span><span>@</span><span>github</span><span>.</span><span>com&gt;</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--

larry-cable avatar May 22 '24 18:05 larry-cable

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

On 5/22/24 4:21 AM, Sebastian Lövdahl wrote:
  <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 81

Using 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.11

Using 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.11

Using 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.11

Using 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.11

Starting 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.Message ID: <openjdk/jdk/pull/19055/c2124549900@github.com>


--------------0k0wZisxTpAKNaSxRp0B1RTM--

larry-cable avatar May 22 '24 18:05 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

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 :)

slovdahl avatar May 22 '24 18:05 slovdahl

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 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



On 5/22/24 11:58 AM, Sebastian Lövdahl wrote:
  <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/&lt;pid&gt;/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/&lt;pid&gt;/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>&lt;openjdk/jdk/pull/19055/c2125541556</span><span>@</span><span>github</span><span>.</span><span>com&gt;</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 avatar May 22 '24 19:05 larry-cable

@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.

slovdahl avatar Jun 02 '24 16:06 slovdahl

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

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

On 6/2/24 9:02 AM, Sebastian Lövdahl wrote:
  <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>&lt;openjdk/jdk/pull/19055/c2143912533</span><span>@</span><span>github</span><span>.</span><span>com&gt;</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--

larry-cable avatar Jun 03 '24 15:06 larry-cable

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?

slovdahl avatar Jun 03 '24 17:06 slovdahl