From 32f871bd4341d21901a1782f0bd7633bd7584255 Mon Sep 17 00:00:00 2001 From: Larry Cable Date: Thu, 17 Oct 2024 14:29:16 -0700 Subject: [PATCH] JDK-8342449: reimplement: JDK-8327114 Attach in Linux may have wrong behavior when pid == ns_pid --- .../sun/tools/attach/VirtualMachineImpl.java | 169 +++++++++--------- 1 file changed, 85 insertions(+), 84 deletions(-) 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 35ce808c18756..a763013185c37 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -34,7 +34,8 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Optional; + +import java.util.regex.Pattern; import static java.nio.charset.StandardCharsets.UTF_8; @@ -51,26 +52,9 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { 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 STATUS = Path.of("status"); private static final Path ROOT_TMP = Path.of("root/tmp"); - private static final Optional SELF_MNT_NS; - - static { - Path nsPath = null; - - try { - nsPath = Files.readSymbolicLink(SELF.resolve(NS_MNT)); - } catch (IOException _) { - // do nothing - } finally { - SELF_MNT_NS = Optional.ofNullable(nsPath); - } - } - String socket_path; /** @@ -98,7 +82,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // Keep canonical version of File, to delete, in case target process ends and /proc link has gone: File f = createAttachFile(pid, ns_pid).getCanonicalFile(); try { - sendQuitTo(pid); + checkCatchesAndSendQuitTo(pid); // give the target VM time to start the attach mechanism final int delay_step = 100; @@ -265,71 +249,25 @@ private String findTargetProcessTmpDirectory(long pid, long ns_pid) throws Attac // 3. Caller and target processes share PID namespace but NOT root filesystem (container to container). // 4. Caller and target processes share neither PID namespace nor root filesystem (host to container). - Optional target = ProcessHandle.of(pid); - Optional ph = target; - long nsPid = ns_pid; - Optional prevPidNS = Optional.empty(); - - while (ph.isPresent()) { - final var curPid = ph.get().pid(); - final var procPidPath = PROC.resolve(Long.toString(curPid)); - Optional targetMountNS = Optional.empty(); - - try { - // attempt to read the target's mnt ns id - targetMountNS = Optional.ofNullable(Files.readSymbolicLink(procPidPath.resolve(NS_MNT))); - } catch (IOException _) { - // if we fail to read the target's mnt ns id then we either don't 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)); - } - // the process still exists, but we don't have privileges to read its procfs - } - - final var sameMountNS = SELF_MNT_NS.isPresent() && SELF_MNT_NS.equals(targetMountNS); - - if (sameMountNS) { - return TMPDIR.toString(); // we share TMPDIR in common! - } else { - // we could not read the target's mnt ns - 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 - } - } - - // let's attempt to obtain the pid ns, best efforts to avoid crossing pid ns boundaries (as with a container) - Optional curPidNS = Optional.empty(); - - try { - // attempt to read the target's pid ns id - curPidNS = Optional.ofNullable(Files.readSymbolicLink(procPidPath.resolve(NS_PID))); - } catch (IOException _) { - // if we fail to read the target's pid ns id then we either don't 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)); - } - // the process still exists, but we don't have privileges to read its procfs - } - - // recurse "up" the process hierarchy if appropriate. PID 1 cannot have a parent in the same namespace - 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(); - } - } - - if (target.orElseThrow(AttachNotSupportedException::new).isAlive()) { - return TMPDIR.toString(); // fallback... - } else { - throw new IOException(String.format("unable to attach, process: %d terminated", pid)); - } + final var procPidRoot = PROC.resolve(Long.toString(pid)).resolve(ROOT_TMP); + + /* + * if target is elevated, we cant use /proc//... so we have to fallback to /tmp, but that may not be shared + * with the target/attachee process, we can try, except in the case where the ns_pid also exists in this pid ns + * which is ambiguous, if we share /tmp with the intended target, the attach will succeed, if we do not, + * then we will potentially attempt to attach to some arbitrary process with the same pid (in this pid ns) + * as that of the intended target (in its * pid ns). + * + * so in that case we should prehaps throw - or risk sending SIGQUIT to some arbitrary process... which could kill it + * + * however we can also check the target pid's signal masks to see if it catches SIGQUIT and only do so if in + * fact it does ... this reduces the risk of killing an innocent process in the current ns as opposed to + * attaching to the actual target JVM ... c.f: checkCatchesAndSendQuitTo() below. + * + * note that if pid == ns_pid we are in a shared pid ns with the target and may (potentially) share /tmp + */ + + return (Files.isWritable(procPidRoot) ? procPidRoot : TMPDIR).toString(); } /* @@ -378,6 +316,69 @@ private long getNamespacePid(long pid) throws AttachNotSupportedException, IOExc } } + private static final String FIELD = "field"; + private static final String MASK = "mask"; + + private static final String SIGNAL_MASK_PATTERN = "(?<" + FIELD + ">Sig\\p{Alpha}{3}):\\s+(?<" + MASK + ">\\p{XDigit}{16}).*"; + + private static final long SIGQUIT = 1L << 2; + + private static void checkCatchesAndSendQuitTo(int pid) throws AttachNotSupportedException, IOException { + var quitIgn = false; + var quitBlk = false; + var quitCgt = false; + + final var procPid = PROC.resolve(Integer.toString(pid)); + + final var p = Pattern.compile(SIGNAL_MASK_PATTERN); + + var readBlk = false; + var readIgn = false; + var readCgt = false; + + + if (!Files.exists(procPid)) throw new IOException("non existent pid: " + pid); + + for (var line : Files.readAllLines(procPid.resolve("status"))) { + + if (!line.startsWith("Sig")) continue; // to speed things up ... avoids the matcher/RE invocation... + + final var m = p.matcher(line); + + if (!m.matches()) continue; + + var signals = m.group(MASK); + final var slen = signals.length(); + + signals = signals.substring(slen / 2 , slen); // only really interested in the non r/t signals ... + + final var sigquit = (Long.valueOf(signals, 16) & SIGQUIT) != 0L; + + switch (m.group(FIELD)) { + case "SigBlk": { quitBlk = sigquit; readBlk = true; break; } + case "SigIgn": { quitIgn = sigquit; readCgt = true; break; } + case "SigCgt": { quitCgt = sigquit; readIgn = true; break; } + } + + if (readBlk && readIgn && readCgt) break; + } + + + if (!quitBlk && !quitIgn && quitCgt) { + sendQuitTo(pid); + } else { + final var cmdline = Files.lines(procPid.resolve("cmdline")).findFirst(); + + var cmd = "null"; // default + + if (cmdline.isPresent()) { + cmd = cmdline.get(); + cmd = cmd.substring(0, cmd.length() - 1); // remove trailing \0 + } + + throw new AttachNotSupportedException("pid: " + pid + " cmd: '" + cmd + "' state is not ready to participate in attach handshake!"); + } + } //-- native methods