runtime: change PID existence check

commit 6b3b0a1 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: containers#12065 [NO NEW TESTS NEEDED] it fixes a CI flake Signed-off-by: Giuseppe Scrivano <[email protected]>
mheon · Nov 12, 2021 · 2d6252b · 2d6252b
1 parent a208bc2
commit 2d6252b
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 4 deletions.
diff --git 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
@@ -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