From 0519e7ef87a402a3f22193b49883cfc66812e61a Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 27 Oct 2021 09:31:32 +0200 Subject: utils: do not overwrite the err variable Signed-off-by: Giuseppe Scrivano --- utils/utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'utils') diff --git a/utils/utils.go b/utils/utils.go index b08630d2f..55af41bb3 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -191,8 +191,8 @@ func moveProcessToScope(pidPath, slice, scope string) error { func MovePauseProcessToScope(pausePidPath string) { err := moveProcessToScope(pausePidPath, "user.slice", "podman-pause.scope") if err != nil { - unified, err := cgroups.IsCgroup2UnifiedMode() - if err != nil { + unified, err2 := cgroups.IsCgroup2UnifiedMode() + if err2 != nil { logrus.Warnf("Failed to detect if running with cgroup unified: %v", err) } if RunsOnSystemd() && unified { -- cgit v1.2.3-54-g00ecf From 2d6252b98a94482346cc8dd16f97b4c59d16dc4d Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Thu, 28 Oct 2021 11:18:48 +0200 Subject: 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(-) (limited to 'utils') diff --git a/libpod/runtime.go b/libpod/runtime.go index a99f55fb3..a208b6db4 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -543,9 +543,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 55af41bb3..109ae088b 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 -- cgit v1.2.3-54-g00ecf