From 0d6866bbd987b19e1a277ad2d95a99041a019147 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 5 Apr 2023 14:21:38 +0200 Subject: [PATCH] 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 #17903 Signed-off-by: Paul Holzinger --- pkg/rootless/rootless_linux.c | 10 +++++++++- test/system/550-pause-process.bats | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/system/550-pause-process.bats diff --git a/pkg/rootless/rootless_linux.c b/pkg/rootless/rootless_linux.c index 016c677ad6..d5fa084d55 100644 --- 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 new file mode 100644 index 0000000000..dd3ea08349 --- /dev/null +++ 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) +}