pkg/rootless: do not use shortcut with --tmpdir

When using --tmpdir for the podman cli we use this as location for the pause.pid file. However the c shortcut code has no idea about this option and always assumes XDG_RUNTIME_DIR/libpod/tmp. This can cause us to join the wrong user namespace which leads to very weird issues as mounts are missing there. Fixes containers#17903 Signed-off-by: Paul Holzinger <[email protected]>
Luap99 · Apr 5, 2023 · 0d6866b · 0d6866b
1 parent cf15829
commit 0d6866b
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 1 deletion.
diff --git a/pkg/rootless/rootless_linux.c b/pkg/rootless/rootless_linux.c
@@ -373,7 +373,15 @@ can_use_shortcut (char **argv)
   for (argc = 0; argv[argc]; argc++)
     {
       if (argc == 0 || argv[argc][0] == '-')
-        continue;
+        {
+          // --tmpdir changes the location of the pause.pid file, so we need to prevent
+          // us from joining the wrong process and let the podman go code handle it
+          // https://github.com/containers/podman/issues/17903#issuecomment-1497232184
+          if (strcmp(argv[argc], "--tmpdir") == 0)
+            return false;
+          continue;
+        }
+
 
       if (strcmp (argv[argc], "mount") == 0
           || strcmp (argv[argc], "machine") == 0

diff --git a/test/system/550-pause-process.bats b/test/system/550-pause-process.bats
@@ -0,0 +1,28 @@
+#!/usr/bin/env bats   -*- bats -*-
+#
+# test to make sure we use the correct podman pause process
+#
+
+load helpers
+
+# Test for https://github.com/containers/podman/issues/17903
+@test "podman uses different pause process with --tmpdir" {
+    skip_if_not_rootless "pause process is only used as rootless"
+    skip_if_remote "--tmpdir not supported via remote"
+
+    # There are nasty bugs when we are not in the correct userns,
+    # we have good reproducer to see how things can go wrong here:
+    # https://github.com/containers/podman/issues/17903#issuecomment-1497232184
+    # However in CI test I rather not kill the pause process, this likely just
+    # causes more tests bugs, instead we will compare the actual namespace values
+
+    run_podman unshare readlink /proc/self/ns/user
+    default_ns="$output"
+
+    run_podman --root $PODMAN_TMPDIR/root --runroot $PODMAN_TMPDIR/runroot --tmpdir $PODMAN_TMPDIR/tmp \
+        unshare readlink /proc/self/ns/user
+    assert "$output" != "$default_ns" "different --tmpdir must use different ns"
+
+    # kill the pause process from our custom tmpdir so we do not leak it forever
+    kill -9 $(cat $PODMAN_TMPDIR/tmp/pause.pid)
+}