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

Laurence Cable larry.cable at oracle.com
Tue Jun 4 22:45:48 UTC 2024



On 6/4/24 5:57 AM, Sebastian Lövdahl wrote:
> On Tue, 21 May 2024 17:10:15 GMT, Sebastian Lövdahl <duke at openjdk.org> wrote:
>
>>> 8327114: Attach in Linux may have wrong behaviour when pid == ns_pid (Kubernetes debug container)
>> Sebastian Lövdahl has updated the pull request incrementally with two additional commits since the last revision:
>>
>>   - Remove unused `SELF_PID_NS`
>>   - Rewrite in line with suggestion from Larry Cable
> This is awesome, Larry. You're my hero :) Thanks a lot in advance!

I modified the container 
test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java to also 
perform a "non root user" test of an elevated JVM from a sidecar.

- Larry

>
> -------------
>
> PR Comment: https://git.openjdk.org/jdk/pull/19055#issuecomment-2147462603
-------------- next part --------------
diff --git a/test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java b/test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java
index f38adcf8b47..35749a1da55 100644
--- a/test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java
+++ b/test/hotspot/jtreg/containers/docker/TestJcmdWithSideCar.java
@@ -38,13 +38,19 @@
  * @build EventGeneratorLoop
  * @run driver TestJcmdWithSideCar
  */
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import jdk.test.lib.Container;
 import jdk.test.lib.Utils;
@@ -61,6 +67,31 @@ public class TestJcmdWithSideCar {
     private static final long TIME_TO_WAIT_FOR_MAIN_METHOD_START = 50 * 1000; // milliseconds
     private static final String MAIN_CONTAINER_NAME = "test-container-main";
 
+    private static final String UID = "uid";
+    private static final String GID = "gid";
+
+    private static final Pattern ID_PATTERN = Pattern.compile("uid=(?<" + UID + ">\\d+)\\([^\\)]+\\)\\s+gid=(?<" + GID + ">\\d+).*");
+
+    private static final Optional<String> USER = ProcessHandle.current().info().user().map(
+            user -> {
+                try (var br = new BufferedReader(new InputStreamReader(new ProcessBuilder("id", user).start().getInputStream()))) {
+                    for (final var line : br.lines().collect(Collectors.toUnmodifiableList())){
+                        final var m = ID_PATTERN.matcher(line);
+
+                        if (m.matches()) {
+                            return "--user=" +m.group(UID) + ":" + m.group(GID);
+                        }
+                    }
+                } catch (IOException e) {
+		    // do nothing...
+                }
+
+                return null;
+            }
+    );
+
+    private static final String NET_BIND_SERVICE = "--cap-add=NET_BIND_SERVICE"; 
+
     public static void main(String[] args) throws Exception {
         if (!DockerTestUtils.canTestDocker()) {
             return;
@@ -69,26 +100,28 @@ public static void main(String[] args) throws Exception {
         DockerTestUtils.buildJdkContainerImage(IMAGE_NAME);
 
         try {
-            // Start the loop process in the "main" container, then run test cases
-            // using a sidecar container.
-            MainContainer mainContainer = new MainContainer();
-            mainContainer.start();
-            mainContainer.waitForMainMethodStart(TIME_TO_WAIT_FOR_MAIN_METHOD_START);
-
-            for (AttachStrategy attachStrategy : EnumSet.allOf(AttachStrategy.class)) {
-                long mainProcPid = testCase01(attachStrategy);
-
-                // Excluding the test case below until JDK-8228850 is fixed
-                // JDK-8228850: jhsdb jinfo fails with ClassCastException:
-                // s.j.h.oops.TypeArray cannot be cast to s.j.h.oops.Instance
-                // mainContainer.assertIsAlive();
-                // testCase02(mainProcPid, attachStrategy);
-
-                mainContainer.assertIsAlive();
-                testCase03(mainProcPid, attachStrategy);
-            }
-
-            mainContainer.waitForAndCheck(TIME_TO_RUN_MAIN_PROCESS * 1000);
+            for (final boolean elevated : USER.isPresent() ? new Boolean[] { false, true } : new Boolean[]{ false }) {
+                // Start the loop process in the "main" container, then run test cases
+                // using a sidecar container.
+                MainContainer mainContainer = new MainContainer();
+                mainContainer.start(elevated);
+                mainContainer.waitForMainMethodStart(TIME_TO_WAIT_FOR_MAIN_METHOD_START);
+    
+                for (AttachStrategy attachStrategy : EnumSet.allOf(AttachStrategy.class)) {
+                    long mainProcPid = testCase01(attachStrategy, elevated);
+    
+                    // Excluding the test case below until JDK-8228850 is fixed
+                    // JDK-8228850: jhsdb jinfo fails with ClassCastException:
+                    // s.j.h.oops.TypeArray cannot be cast to s.j.h.oops.Instance
+                    // mainContainer.assertIsAlive();
+                    // testCase02(mainProcPid, attachStrategy, elevated);
+    
+                    mainContainer.assertIsAlive();
+                    testCase03(mainProcPid, attachStrategy, elevated);
+                }
+    
+                mainContainer.waitForAndCheck(TIME_TO_RUN_MAIN_PROCESS * 1000);
+	    }
         } finally {
             DockerTestUtils.removeDockerImage(IMAGE_NAME);
         }
@@ -96,8 +129,8 @@ public static void main(String[] args) throws Exception {
 
 
     // Run "jcmd -l" in a sidecar container, find a target process.
-    private static long testCase01(AttachStrategy attachStrategy) throws Exception {
-        OutputAnalyzer out = runSideCar(MAIN_CONTAINER_NAME, attachStrategy, "/jdk/bin/jcmd", "-l")
+    private static long testCase01(AttachStrategy attachStrategy, boolean elevated) throws Exception {
+        OutputAnalyzer out = runSideCar(MAIN_CONTAINER_NAME, attachStrategy, elevated, "/jdk/bin/jcmd", "-l")
             .shouldHaveExitValue(0)
             .shouldContain("sun.tools.jcmd.JCmd");
         long pid = findProcess(out, "EventGeneratorLoop");
@@ -109,8 +142,8 @@ private static long testCase01(AttachStrategy attachStrategy) throws Exception {
     }
 
     // run jhsdb jinfo <PID> (jhsdb uses PTRACE)
-    private static void testCase02(long pid, AttachStrategy attachStrategy) throws Exception {
-        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, "/jdk/bin/jhsdb", "jinfo", "--pid", "" + pid)
+    private static void testCase02(long pid, AttachStrategy attachStrategy, boolean elevated) throws Exception {
+        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, elevated, "/jdk/bin/jhsdb", "jinfo", "--pid", "" + pid)
             .shouldHaveExitValue(0)
             .shouldContain("Java System Properties")
             .shouldContain("VM Flags");
@@ -118,11 +151,11 @@ private static void testCase02(long pid, AttachStrategy attachStrategy) throws E
 
     // test jcmd with some commands (help, start JFR recording)
     // JCMD will use signal mechanism and Unix Socket
-    private static void testCase03(long pid, AttachStrategy attachStrategy) throws Exception {
-        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, "/jdk/bin/jcmd", "" + pid, "help")
+    private static void testCase03(long pid, AttachStrategy attachStrategy, boolean elevated) throws Exception {
+        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, elevated, "/jdk/bin/jcmd", "" + pid, "help")
             .shouldHaveExitValue(0)
             .shouldContain("VM.version");
-        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, "/jdk/bin/jcmd", "" + pid, "JFR.start")
+        runSideCar(MAIN_CONTAINER_NAME, attachStrategy, elevated, "/jdk/bin/jcmd", "" + pid, "JFR.start")
             .shouldHaveExitValue(0)
             .shouldContain("Started recording");
     }
@@ -134,9 +167,8 @@ private static void testCase03(long pid, AttachStrategy attachStrategy) throws E
     // we have two options:
     // 1. mount /tmp from the main container using --volumes-from.
     // 2. access /tmp from the main container via /proc/<pid>/root/tmp.
-    private static OutputAnalyzer runSideCar(String mainContainerName, AttachStrategy attachStrategy, String whatToRun,
-                                             String... args) throws Exception {
-        System.out.println("Attach strategy " + attachStrategy);
+    private static OutputAnalyzer runSideCar(String mainContainerName, AttachStrategy attachStrategy, boolean elevated,  String whatToRun, String... args) throws Exception {
+        System.out.println("Attach strategy " + attachStrategy); 
 
         List<String> initialCommands = List.of(
             Container.ENGINE_COMMAND, "run",
@@ -150,12 +182,15 @@ private static OutputAnalyzer runSideCar(String mainContainerName, AttachStrateg
             case ACCESS_TMP_VIA_PROC_ROOT -> List.of();
         };
 
+	List<String> elevatedOpts = elevated && USER.isPresent() ? List.of(NET_BIND_SERVICE, USER.get()) : Collections.emptyList();
+
         List<String> imageAndCommand = List.of(
             IMAGE_NAME, whatToRun
         );
 
         List<String> cmd = new ArrayList<>();
         cmd.addAll(initialCommands);
+	cmd.addAll(elevatedOpts);
         cmd.addAll(attachStrategyCommands);
         cmd.addAll(imageAndCommand);
         cmd.addAll(Arrays.asList(args));
@@ -204,9 +239,15 @@ static class MainContainer {
             }
         };
 
-        public Process start() throws Exception {
+        public Process start(final boolean elevated) throws Exception {
             // start "main" container (the observee)
             DockerRunOptions opts = commonDockerOpts("EventGeneratorLoop");
+
+	    if (elevated && USER.isPresent()) {
+		opts.addDockerOpts(USER.get());
+                opts.addDockerOpts(NET_BIND_SERVICE);
+	    }
+
             opts.addDockerOpts("--cap-add=SYS_PTRACE")
                 .addDockerOpts("--name", MAIN_CONTAINER_NAME)
                 .addDockerOpts("--volume", "/tmp")


More information about the serviceability-dev mailing list