From 960831f9c8ff83f427c296adcef0ed114c434fca Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 28 Oct 2021 11:18:48 +0200 Subject: [PATCH] runtime: change PID existence check commit 6b3b0a17c625bdf71b0ec8b783b288886d8e48d7 introduced a check for the PID file before attempting to move the PID to a new scope. This is still vulnerable to TOCTOU race condition though, since the PID file or the PID can be removed/killed after the check was successful but before it was used. Closes: https://github.com/containers/podman/issues/12065 [NO NEW TESTS NEEDED] it fixes a CI flake Signed-off-by: Giuseppe Scrivano --- libpod/runtime.go | 4 +--- utils/utils.go | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/libpod/runtime.go b/libpod/runtime.go index 950ac65eb2..b01f8dd133 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -545,9 +545,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) { if became { // Check if the pause process was created. If it was created, then // move it to its own systemd scope. - if _, err = os.Stat(pausePid); err == nil { - utils.MovePauseProcessToScope(pausePid) - } + utils.MovePauseProcessToScope(pausePid) os.Exit(ret) } } diff --git a/utils/utils.go b/utils/utils.go index 55af41bb34..109ae088b9 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -14,6 +14,7 @@ import ( "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/cgroups" "github.com/containers/storage/pkg/archive" + "github.com/godbus/dbus/v5" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -177,13 +178,26 @@ func RunsOnSystemd() bool { func moveProcessToScope(pidPath, slice, scope string) error { data, err := ioutil.ReadFile(pidPath) if err != nil { + // do not raise an error if the file doesn't exist + if os.IsNotExist(err) { + return nil + } return errors.Wrapf(err, "cannot read pid file %s", pidPath) } pid, err := strconv.ParseUint(string(data), 10, 0) if err != nil { return errors.Wrapf(err, "cannot parse pid file %s", pidPath) } - return RunUnderSystemdScope(int(pid), slice, scope) + err = RunUnderSystemdScope(int(pid), slice, scope) + + // If the PID is not valid anymore, do not return an error. + if dbusErr, ok := err.(dbus.Error); ok { + if dbusErr.Name == "org.freedesktop.DBus.Error.UnixProcessIdUnknown" { + return nil + } + } + + return err } // MovePauseProcessToScope moves the pause process used for rootless mode to keep the namespaces alive to