runc create/run: warn on rootless + shared pidns + no cgroup

Shared pid namespace means `runc kill` (or `runc delete -f`) have to
kill all container process, not just init. To do so, it needs a cgroup
to read the PIDs from.

If there is no cgroup, it's impossible to kill all container processes.

Therefore, such configuration is bad and should not be allowed. To keep
backward compatibility, though, let's merely warn about this for now.

Alas, the only way to know if cgroup access is available is by returning
an error from Manager.Apply. Amend fs cgroup managers to do so (systemd
doesn't need it, since v1 can't work with rootless, and cgroup v2 does
not have a special rootless case).

Signed-off-by: Kir Kolyshkin <[email protected]>

Author: kolyshkin

libcontainer/cgroups/cgroups.go

@@ -11,6 +11,11 @@ var (
 	// is not configured to set device rules.
 	ErrDevicesUnsupported = errors.New("cgroup manager is not configured to set device rules")
 
+	// ErrRootless is returned by [Manager.Apply] when there is an error
+	// creating cgroup directory, and cgroup.Rootless is set. In general,
+	// this error is to be ignored.
+	ErrRootless = errors.New("cgroup manager can not access cgroup (rootless container)")
+
 	// DevicesSetV1 and DevicesSetV2 are functions to set devices for
 	// cgroup v1 and v2, respectively. Unless
 	// [github.com/opencontainers/runc/libcontainer/cgroups/devices]

libcontainer/cgroups/fs/fs.go

@@ -88,11 +88,7 @@ func NewManager(cg *configs.Cgroup, paths map[string]string) (*Manager, error) {
 // sense of the word). This includes EROFS (which for an unprivileged user is
 // basically a permission error) and EACCES (for similar reasons) as well as
 // the normal EPERM.
-func isIgnorableError(rootless bool, err error) bool {
-	// We do not ignore errors if we are root.
-	if !rootless {
-		return false
-	}
+func isIgnorableError(err error) bool {
 	// Is it an ordinary EPERM?
 	if errors.Is(err, os.ErrPermission) {
 		return true
@@ -105,7 +101,7 @@ func isIgnorableError(rootless bool, err error) bool {
 	return false
 }
 
-func (m *Manager) Apply(pid int) (err error) {
+func (m *Manager) Apply(pid int) (retErr error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
@@ -128,15 +124,16 @@ func (m *Manager) Apply(pid int) (err error) {
 			// Cases where limits for the subsystem have been set are handled
 			// later by Set, which fails with a friendly error (see
 			// if path == "" in Set).
-			if isIgnorableError(c.Rootless, err) && c.Path == "" {
+			if c.Rootless && c.Path == "" && isIgnorableError(err) {
+				retErr = cgroups.ErrRootless
 				delete(m.paths, name)
 				continue
 			}
 			return err
 		}
 
 	}
-	return nil
+	return retErr
 }
 
 func (m *Manager) Destroy() error {

libcontainer/cgroups/fs2/fs2.go

@@ -68,13 +68,11 @@ func (m *Manager) Apply(pid int) error {
 		// - "runc create (no limits + no cgrouppath + no permission) succeeds"
 		// - "runc create (rootless + no limits + cgrouppath + no permission) fails with permission error"
 		// - "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error"
-		if m.config.Rootless {
-			if m.config.Path == "" {
-				if blNeed, nErr := needAnyControllers(m.config.Resources); nErr == nil && !blNeed {
-					return nil
-				}
-				return fmt.Errorf("rootless needs no limits + no cgrouppath when no permission is granted for cgroups: %w", err)
+		if m.config.Rootless && m.config.Path == "" {
+			if blNeed, nErr := needAnyControllers(m.config.Resources); nErr == nil && !blNeed {
+				return cgroups.ErrRootless
 			}
+			return fmt.Errorf("rootless needs no limits + no cgrouppath when no permission is granted for cgroups: %w", err)
 		}
 		return err
 	}

libcontainer/process_linux.go

@@ -580,7 +580,18 @@ func (p *initProcess) start() (retErr error) {
 	// cgroup. We don't need to worry about not doing this and not being root
 	// because we'd be using the rootless cgroup manager in that case.
 	if err := p.manager.Apply(p.pid()); err != nil {
-		return fmt.Errorf("unable to apply cgroup configuration: %w", err)
+		if errors.Is(err, cgroups.ErrRootless) {
+			// ErrRootless is to be ignored except when the
+			// container doesn't have private pidns.
+			if !p.config.Config.Namespaces.IsPrivate(configs.NEWPID) {
+				// TODO: make this an error in runc 1.3.
+				logrus.Warn("Creating a rootless container with no cgroup and no private pid namespace. " +
+					"Such configuration is strongly discouraged (as it is impossible to properly kill all container's processes) " +
+					"and will result in an error in a future runc version.")
+			}
+		} else {
+			return fmt.Errorf("unable to apply cgroup configuration: %w", err)
+		}
 	}
 	if p.intelRdtManager != nil {
 		if err := p.intelRdtManager.Apply(p.pid()); err != nil {

tests/integration/create.bats

@@ -81,3 +81,30 @@ function teardown() {
 
 	testcontainer test_busybox running
 }
+
+# https://github.com/opencontainers/runc/issues/4394#issuecomment-2334926257
+@test "runc create [shared pidns + rootless]" {
+	# Remove pidns so it's shared with the host.
+	update_config '	  .linux.namespaces -= [{"type": "pid"}]'
+	if [ $EUID -ne 0 ]; then
+		if rootless_cgroup; then
+			set_cgroups_path
+		fi
+		# Can't mount real /proc when rootless + no pidns,
+		# so change it to a bind-mounted one from the host.
+		update_config '	  .mounts |= map((select(.type == "proc")
+                                | .type = "none"
+                                | .source = "/proc"
+                                | .options = ["rbind", "nosuid", "nodev", "noexec"]
+                          ) // .)'
+	fi
+
+	exp="Such configuration is strongly discouraged"
+	runc create --console-socket "$CONSOLE_SOCKET" test
+	[ "$status" -eq 0 ]
+	if [ $EUID -ne 0 ] && ! rootless_cgroup; then
+		[[ "$output" = *"$exp"* ]]
+	else
+		[[ "$output" != *"$exp"* ]]
+	fi
+}