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

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

Reply via email to