Repository: opencontainers/runc
Pull request #2390 (2020-05-08 15:47:53)

cgroupv2: don't enable threaded mode by default
After this commit: https://github.com/opencontainers/runc/commit/60c647e3b8e0d04cc8fe85b957ba2addfcc4fc4f 
Runc enable `threaded` mode in cgroup v2 by default.
If the cgroupPath is set to a absolute path like `/docker/********`, the memory subsystem can't be used by this mode.
So, I think we should use `domain` mode by default.

Signed-off-by: lifubang <lifubang@acmcoder.com>

lifubang: @AkihiroSuda PTAL
cyphar: I'm not sure I understand why we're putting cgroupv2 into threaded mode -- @AkihiroSuda is this to work around some odd permission issue (and why does it fix an `EPERM`)? The description in 60c647e3b8e0d04cc8fe85b957ba2addfcc4fc4f and #2340 is pretty sparse.
AkihiroSuda: @cyphar 
Writing `threaded` to `cgroup.type` is required when `cgroup.type` becomes `domain invalid`, mostly in nested environment
```console
$ sudo podman run -it --rm --privileged --runtime=crun alpine
/ # cd /sys/fs/cgroup/
/sys/fs/cgroup # cat cgroup.controllers 
cpuset cpu io memory pids
/sys/fs/cgroup # cat cgroup.subtree_control 
/sys/fs/cgroup # echo +cpu > cgroup.subtree_control
/sys/fs/cgroup # mkdir foo
/sys/fs/cgroup # cat foo/cgroup.type
domain invalid
```
cyphar: Okay, but "domain invalid" means that the cgroup is in an invalid state (meaning that one of the cgroup rules has been violated). Putting a cgroup into threaded mode doesn't fix that -- it switches it into an alternative mode which only allows controllers which are thread-aware to be enabled (such as cpu). This is why we can't enable the memory controller.

IMHO, a more complete solution would be to figure out how to deal with the parent cgroup having child processes (which is a bit dodgy if we're going to move other programs on the system between cgroups) or to simply give an error if we hit `domain invalid` and explain what the issue is.
lifubang: > or to simply give an error if we hit `domain invalid` and explain what the issue is.

Personally, I prefer this one. @AkihiroSuda WDYT? Because in [man7 cgroups](http://man7.org/linux/man-pages/man7/cgroups.7.html):
```
2. "Internal" processes are not permitted.  With the exception of the
          root cgroup, processes may reside only in leaf nodes (cgroups that
          do not themselves contain child cgroups).  The details are
          somewhat more subtle than this, and are described below.
```
```
Cgroups v2 "no internal processes" rule
       Cgroups v2 enforces a so-called "no internal processes" rule.
       Roughly speaking, this rule means that, with the exception of the
       root cgroup, processes may reside only in leaf nodes (cgroups that do
       not themselves contain child cgroups).  This avoids the need to
       decide how to partition resources between processes which are members
       of cgroup A and processes in child cgroups of A.

       For instance, if cgroup /cg1/cg2 exists, then a process may reside in
       /cg1/cg2, but not in /cg1.  This is to avoid an ambiguity in cgroups
       v1 with respect to the delegation of resources between processes in
       /cg1 and its child cgroups.  The recommended approach in cgroups v2
       is to create a subdirectory called leaf for any nonleaf cgroup which
       should contain processes, but no child cgroups.  Thus, processes
       which previously would have gone into /cg1 would now go into
       /cg1/leaf.  This has the advantage of making explicit the relation‐
       ship between processes in /cg1/leaf and /cg1's other children.

       The "no internal processes" rule is in fact more subtle than stated
       above.  More precisely, the rule is that a (nonroot) cgroup can't
       both (1) have member processes, and (2) distribute resources into
       child cgroups—that is, have a nonempty cgroup.subtree_control file.
       Thus, it is possible for a cgroup to have both member processes and
       child cgroups, but before controllers can be enabled for that cgroup,
       the member processes must be moved out of the cgroup (e.g., perhaps
       into the child cgroups).

       With the Linux 4.14 addition of "thread mode" (described below), the
       "no internal processes" rule has been relaxed in some cases.
```
AkihiroSuda: SGTM, but when no domain controller is enabled, we can write "threaded" without retuning error
lifubang: https://github.com/kolyshkin/runc/blob/1d143562d2d491d054e28815565fe371c461b8a6/tests/rootless.sh#L103
I think `rootless` doesn't need `threaded` mode in default?
I don't know whether my opinion is right or not. @AkihiroSuda 
AkihiroSuda: Let's check invalid cgroup.type and set threaded conditionally
AkihiroSuda: LGTM

AkihiroSuda: LGTM

AkihiroSuda: @kolyshkin LGTY?

Patch:
diff --git a/Dockerfile b/Dockerfile
index fd891ee4..8ca87b67 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,5 +1,5 @@
 ARG GO_VERSION=1.13
-ARG BATS_VERSION=v1.2.0
+ARG BATS_VERSION=v1.1.0
 ARG CRIU_VERSION=v3.14
 
 FROM golang:${GO_VERSION}-buster
diff --git a/Vagrantfile b/Vagrantfile
index cb4e0286..13c69813 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -13,14 +13,17 @@ Vagrant.configure("2") do |config|
     v.cpus = 2
   end
   config.vm.provision "shell", inline: <<-SHELL
+    curl -OSs https://kojipkgs.fedoraproject.org/packages/criu/3.14/1.fc32/x86_64/criu-3.14-1.fc32.x86_64.rpm
     cat << EOF | dnf -y shell
 config exclude kernel,kernel-core
 config install_weak_deps false
+localinstall criu-3.14-1.fc32.x86_64.rpm
 update
-install iptables gcc make golang-go libseccomp-devel bats jq git-core criu
+install iptables gcc make golang-go libseccomp-devel bats jq git-core
 ts run
 EOF
     dnf clean all
+    rm -f criu-3.14-1.fc32.x86_64.rpm
 
     # Add a user for rootless tests
     useradd -u2000 -m -d/home/rootless -s/bin/bash rootless
diff --git a/libcontainer/cgroups/cgroups.go b/libcontainer/cgroups/cgroups.go
index cbc430bb..c0a96592 100644
--- a/libcontainer/cgroups/cgroups.go
+++ b/libcontainer/cgroups/cgroups.go
@@ -27,22 +27,27 @@ type Manager interface {
 	// Destroys the cgroup set
 	Destroy() error
 
-	// Path returns a cgroup path to the specified controller/subsystem.
-	// For cgroupv2, the argument is unused and can be empty.
-	Path(string) string
+	// The option func SystemdCgroups() and Cgroupfs() require following attributes:
+	// 	Paths   map[string]string
+	// 	Cgroups *configs.Cgroup
+	// Paths maps cgroup subsystem to path at which it is mounted.
+	// Cgroups specifies specific cgroup settings for the various subsystems
+
+	// Returns cgroup paths to save in a state file and to be able to
+	// restore the object later.
+	GetPaths() map[string]string
+
+	// GetUnifiedPath returns the unified path when running in unified mode.
+	// The value corresponds to the all values of GetPaths() map.
+	//
+	// GetUnifiedPath returns error when running in hybrid mode as well as
+	// in legacy mode.
+	GetUnifiedPath() (string, error)
 
 	// Sets the cgroup as configured.
 	Set(container *configs.Config) error
 
-	// GetPaths returns cgroup path(s) to save in a state file in order to restore later.
-	//
-	// For cgroup v1, a key is cgroup subsystem name, and the value is the path
-	// to the cgroup for this subsystem.
-	//
-	// For cgroup v2 unified hierarchy, a key is "", and the value is the unified path.
-	GetPaths() map[string]string
-
-	// GetCgroups returns the cgroup data as configured.
+	// Gets the cgroup as configured.
 	GetCgroups() (*configs.Cgroup, error)
 }
 
diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go
index c3060145..59ce5ea6 100644
--- a/libcontainer/cgroups/fs/apply_raw.go
+++ b/libcontainer/cgroups/fs/apply_raw.go
@@ -61,19 +61,11 @@ type subsystem interface {
 	Set(path string, cgroup *configs.Cgroup) error
 }
 
-type manager struct {
+type Manager struct {
 	mu       sync.Mutex
-	cgroups  *configs.Cgroup
-	rootless bool // ignore permission-related errors
-	paths    map[string]string
-}
-
-func NewManager(cg *configs.Cgroup, paths map[string]string, rootless bool) cgroups.Manager {
-	return &manager{
-		cgroups:  cg,
-		paths:    paths,
-		rootless: rootless,
-	}
+	Cgroups  *configs.Cgroup
+	Rootless bool // ignore permission-related errors
+	Paths    map[string]string
 }
 
 // The absolute path to the root of the cgroup hierarchies.
@@ -132,25 +124,25 @@ func isIgnorableError(rootless bool, err error) bool {
 	return false
 }
 
-func (m *manager) getSubsystems() subsystemSet {
+func (m *Manager) getSubsystems() subsystemSet {
 	return subsystemsLegacy
 }
 
-func (m *manager) Apply(pid int) (err error) {
-	if m.cgroups == nil {
+func (m *Manager) Apply(pid int) (err error) {
+	if m.Cgroups == nil {
 		return nil
 	}
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
-	var c = m.cgroups
+	var c = m.Cgroups
 
-	d, err := getCgroupData(m.cgroups, pid)
+	d, err := getCgroupData(m.Cgroups, pid)
 	if err != nil {
 		return err
 	}
 
-	m.paths = make(map[string]string)
+	m.Paths = make(map[string]string)
 	if c.Paths != nil {
 		for name, path := range c.Paths {
 			_, err := d.path(name)
@@ -160,9 +152,9 @@ func (m *manager) Apply(pid int) (err error) {
 				}
 				return err
 			}
-			m.paths[name] = path
+			m.Paths[name] = path
 		}
-		return cgroups.EnterPid(m.paths, pid)
+		return cgroups.EnterPid(m.Paths, pid)
 	}
 
 	for _, sys := range m.getSubsystems() {
@@ -178,15 +170,15 @@ func (m *manager) Apply(pid int) (err error) {
 			}
 			return err
 		}
-		m.paths[sys.Name()] = p
+		m.Paths[sys.Name()] = p
 
 		if err := sys.Apply(d); err != nil {
 			// In the case of rootless (including euid=0 in userns), where an explicit cgroup path hasn't
 			// been set, we don't bail on error in case of permission problems.
 			// Cases where limits have been set (and we couldn't create our own
 			// cgroup) are handled by Set.
-			if isIgnorableError(m.rootless, err) && m.cgroups.Path == "" {
-				delete(m.paths, sys.Name())
+			if isIgnorableError(m.Rootless, err) && m.Cgroups.Path == "" {
+				delete(m.Paths, sys.Name())
 				continue
 			}
 			return err
@@ -196,30 +188,35 @@ func (m *manager) Apply(pid int) (err error) {
 	return nil
 }
 
-func (m *manager) Destroy() error {
-	if m.cgroups == nil || m.cgroups.Paths != nil {
+func (m *Manager) Destroy() error {
+	if m.Cgroups == nil || m.Cgroups.Paths != nil {
 		return nil
 	}
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	if err := cgroups.RemovePaths(m.paths); err != nil {
+	if err := cgroups.RemovePaths(m.Paths); err != nil {
 		return err
 	}
-	m.paths = make(map[string]string)
+	m.Paths = make(map[string]string)
 	return nil
 }
 
-func (m *manager) Path(subsys string) string {
+func (m *Manager) GetPaths() map[string]string {
 	m.mu.Lock()
-	defer m.mu.Unlock()
-	return m.paths[subsys]
+	paths := m.Paths
+	m.mu.Unlock()
+	return paths
 }
 
-func (m *manager) GetStats() (*cgroups.Stats, error) {
+func (m *Manager) GetUnifiedPath() (string, error) {
+	return "", errors.New("unified path is only supported when running in unified mode")
+}
+
+func (m *Manager) GetStats() (*cgroups.Stats, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()
-	for name, path := range m.paths {
+	for name, path := range m.Paths {
 		sys, err := m.getSubsystems().Get(name)
 		if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) {
 			continue
@@ -231,23 +228,22 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
 	return stats, nil
 }
 
-func (m *manager) Set(container *configs.Config) error {
+func (m *Manager) Set(container *configs.Config) error {
 	if container.Cgroups == nil {
 		return nil
 	}
 
 	// If Paths are set, then we are just joining cgroups paths
 	// and there is no need to set any values.
-	if m.cgroups != nil && m.cgroups.Paths != nil {
+	if m.Cgroups != nil && m.Cgroups.Paths != nil {
 		return nil
 	}
 
-	m.mu.Lock()
-	defer m.mu.Unlock()
+	paths := m.GetPaths()
 	for _, sys := range m.getSubsystems() {
-		path := m.paths[sys.Name()]
+		path := paths[sys.Name()]
 		if err := sys.Set(path, container.Cgroups); err != nil {
-			if m.rootless && sys.Name() == "devices" {
+			if m.Rootless && sys.Name() == "devices" {
 				continue
 			}
 			// When m.Rootless is true, errors from the device subsystem are ignored because it is really not expected to work.
@@ -262,8 +258,8 @@ func (m *manager) Set(container *configs.Config) error {
 		}
 	}
 
-	if m.paths["cpu"] != "" {
-		if err := CheckCpushares(m.paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
+	if m.Paths["cpu"] != "" {
+		if err := CheckCpushares(m.Paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
 			return err
 		}
 	}
@@ -272,33 +268,35 @@ func (m *manager) Set(container *configs.Config) error {
 
 // Freeze toggles the container's freezer cgroup depending on the state
 // provided
-func (m *manager) Freeze(state configs.FreezerState) error {
-	if m.cgroups == nil {
+func (m *Manager) Freeze(state configs.FreezerState) error {
+	if m.Cgroups == nil {
 		return errors.New("cannot toggle freezer: cgroups not configured for container")
 	}
 
 	paths := m.GetPaths()
 	dir := paths["freezer"]
-	prevState := m.cgroups.Resources.Freezer
-	m.cgroups.Resources.Freezer = state
+	prevState := m.Cgroups.Resources.Freezer
+	m.Cgroups.Resources.Freezer = state
 	freezer, err := m.getSubsystems().Get("freezer")
 	if err != nil {
 		return err
 	}
-	err = freezer.Set(dir, m.cgroups)
+	err = freezer.Set(dir, m.Cgroups)
 	if err != nil {
-		m.cgroups.Resources.Freezer = prevState
+		m.Cgroups.Resources.Freezer = prevState
 		return err
 	}
 	return nil
 }
 
-func (m *manager) GetPids() ([]int, error) {
-	return cgroups.GetPids(m.Path("devices"))
+func (m *Manager) GetPids() ([]int, error) {
+	paths := m.GetPaths()
+	return cgroups.GetPids(paths["devices"])
 }
 
-func (m *manager) GetAllPids() ([]int, error) {
-	return cgroups.GetAllPids(m.Path("devices"))
+func (m *Manager) GetAllPids() ([]int, error) {
+	paths := m.GetPaths()
+	return cgroups.GetAllPids(paths["devices"])
 }
 
 func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) {
@@ -404,12 +402,6 @@ func CheckCpushares(path string, c uint64) error {
 	return nil
 }
 
-func (m *manager) GetPaths() map[string]string {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	return m.paths
-}
-
-func (m *manager) GetCgroups() (*configs.Cgroup, error) {
-	return m.cgroups, nil
+func (m *Manager) GetCgroups() (*configs.Cgroup, error) {
+	return m.Cgroups, nil
 }
diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go
index 66afdec7..ba50c8d4 100644
--- a/libcontainer/cgroups/fs2/create.go
+++ b/libcontainer/cgroups/fs2/create.go
@@ -63,6 +63,13 @@ func neededControllers(cgroup *configs.Cgroup) ([]string, error) {
 	return list, nil
 }
 
+// containsDomainController returns whether the current config contains domain controller or not.
+// Refer to: http://man7.org/linux/man-pages/man7/cgroups.7.html
+// As at Linux 4.19, the following controllers are threaded: cpu, perf_event, and pids.
+func containsDomainController(cg *configs.Cgroup) bool {
+	return isMemorySet(cg) || isIoSet(cg) || isCpuSet(cg) || isHugeTlbSet(cg)
+}
+
 // CreateCgroupPath creates cgroupv2 path, enabling all the
 // needed controllers in the process.
 func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
@@ -95,10 +102,34 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
 					}
 				}()
 			}
-			// Write cgroup.type explicitly.
-			// Otherwise ENOTSUP may happen.
-			cgType := filepath.Join(current, "cgroup.type")
-			_ = ioutil.WriteFile(cgType, []byte("threaded"), 0644)
+			cgTypeFile := filepath.Join(current, "cgroup.type")
+			cgType, _ := ioutil.ReadFile(cgTypeFile)
+			switch strings.TrimSpace(string(cgType)) {
+			// If the cgroup is in an invalid mode (usually this means there's an internal
+			// process in the cgroup tree, because we created a cgroup under an
+			// already-populated-by-other-processes cgroup), then we have to error out if
+			// the user requested controllers which are not thread-aware. However, if all
+			// the controllers requested are thread-aware we can simply put the cgroup into
+			// threaded mode.
+			case "domain invalid":
+				if containsDomainController(c) {
+					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in an invalid state", current)
+				} else {
+					// Not entirely correct (in theory we'd always want to be a domain --
+					// since that means we're a properly delegated cgroup subtree) but in
+					// this case there's not much we can do and it's better than giving an
+					// error.
+					_ = ioutil.WriteFile(cgTypeFile, []byte("threaded"), 0644)
+				}
+			// If the cgroup is in (threaded) or (domain threaded) mode, we can only use thread-aware controllers
+			// (and you cannot usually take a cgroup out of threaded mode).
+			case "domain threaded":
+				fallthrough
+			case "threaded":
+				if containsDomainController(c) {
+					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in %s mode", current, strings.TrimSpace(string(cgType)))
+				}
+			}
 		}
 		// enable needed controllers
 		if i < len(elements)-1 {
diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go
index a0881dac..fc469e10 100644
--- a/libcontainer/cgroups/fs2/fs2.go
+++ b/libcontainer/cgroups/fs2/fs2.go
@@ -164,8 +164,22 @@ func (m *manager) Destroy() error {
 	return nil
 }
 
-func (m *manager) Path(_ string) string {
-	return m.dirPath
+// GetPaths is for compatibility purpose and should be removed in future
+func (m *manager) GetPaths() map[string]string {
+	_ = m.getControllers()
+	paths := map[string]string{
+		// pseudo-controller for compatibility
+		"devices": m.dirPath,
+		"freezer": m.dirPath,
+	}
+	for c := range m.controllers {
+		paths[c] = m.dirPath
+	}
+	return paths
+}
+
+func (m *manager) GetUnifiedPath() (string, error) {
+	return m.dirPath, nil
 }
 
 func (m *manager) Set(container *configs.Config) error {
@@ -231,12 +245,6 @@ func (m *manager) Set(container *configs.Config) error {
 	return nil
 }
 
-func (m *manager) GetPaths() map[string]string {
-	paths := make(map[string]string, 1)
-	paths[""] = m.dirPath
-	return paths
-}
-
 func (m *manager) GetCgroups() (*configs.Cgroup, error) {
 	return m.config, nil
 }
diff --git a/libcontainer/cgroups/systemd/unsupported.go b/libcontainer/cgroups/systemd/unsupported.go
index 37fa5035..5318a1e9 100644
--- a/libcontainer/cgroups/systemd/unsupported.go
+++ b/libcontainer/cgroups/systemd/unsupported.go
@@ -42,8 +42,8 @@ func (m *Manager) GetPaths() map[string]string {
 	return nil
 }
 
-func (m *Manager) Path(_ string) string {
-	return ""
+func (m *Manager) GetUnifiedPath() (string, error) {
+	return "", fmt.Errorf("Systemd not supported")
 }
 
 func (m *Manager) GetStats() (*cgroups.Stats, error) {
diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go
index 70440ea6..e66db22a 100644
--- a/libcontainer/cgroups/systemd/v1.go
+++ b/libcontainer/cgroups/systemd/v1.go
@@ -17,17 +17,10 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
-type legacyManager struct {
+type LegacyManager struct {
 	mu      sync.Mutex
-	cgroups *configs.Cgroup
-	paths   map[string]string
-}
-
-func NewLegacyManager(cg *configs.Cgroup, paths map[string]string) cgroups.Manager {
-	return &legacyManager{
-		cgroups: cg,
-		paths:   paths,
-	}
+	Cgroups *configs.Cgroup
+	Paths   map[string]string
 }
 
 type subsystem interface {
@@ -121,9 +114,9 @@ func genV1ResourcesProperties(c *configs.Cgroup) ([]systemdDbus.Property, error)
 	return properties, nil
 }
 
-func (m *legacyManager) Apply(pid int) error {
+func (m *LegacyManager) Apply(pid int) error {
 	var (
-		c          = m.cgroups
+		c          = m.Cgroups
 		unitName   = getUnitName(c)
 		slice      = "system.slice"
 		properties []systemdDbus.Property
@@ -132,7 +125,7 @@ func (m *legacyManager) Apply(pid int) error {
 	if c.Paths != nil {
 		paths := make(map[string]string)
 		for name, path := range c.Paths {
-			_, err := getSubsystemPath(m.cgroups, name)
+			_, err := getSubsystemPath(m.Cgroups, name)
 			if err != nil {
 				// Don't fail if a cgroup hierarchy was not found, just skip this subsystem
 				if cgroups.IsNotFound(err) {
@@ -142,8 +135,8 @@ func (m *legacyManager) Apply(pid int) error {
 			}
 			paths[name] = path
 		}
-		m.paths = paths
-		return cgroups.EnterPid(m.paths, pid)
+		m.Paths = paths
+		return cgroups.EnterPid(m.Paths, pid)
 	}
 
 	if c.Parent != "" {
@@ -203,7 +196,7 @@ func (m *legacyManager) Apply(pid int) error {
 
 	paths := make(map[string]string)
 	for _, s := range legacySubsystems {
-		subsystemPath, err := getSubsystemPath(m.cgroups, s.Name())
+		subsystemPath, err := getSubsystemPath(m.Cgroups, s.Name())
 		if err != nil {
 			// Don't fail if a cgroup hierarchy was not found, just skip this subsystem
 			if cgroups.IsNotFound(err) {
@@ -213,12 +206,12 @@ func (m *legacyManager) Apply(pid int) error {
 		}
 		paths[s.Name()] = subsystemPath
 	}
-	m.paths = paths
+	m.Paths = paths
 	return nil
 }
 
-func (m *legacyManager) Destroy() error {
-	if m.cgroups.Paths != nil {
+func (m *LegacyManager) Destroy() error {
+	if m.Cgroups.Paths != nil {
 		return nil
 	}
 	m.mu.Lock()
@@ -228,18 +221,23 @@ func (m *legacyManager) Destroy() error {
 	if err != nil {
 		return err
 	}
-	unitName := getUnitName(m.cgroups)
+	unitName := getUnitName(m.Cgroups)
 	if err := stopUnit(dbusConnection, unitName); err != nil {
 		return err
 	}
-	m.paths = make(map[string]string)
+	m.Paths = make(map[string]string)
 	return nil
 }
 
-func (m *legacyManager) Path(subsys string) string {
+func (m *LegacyManager) GetPaths() map[string]string {
 	m.mu.Lock()
-	defer m.mu.Unlock()
-	return m.paths[subsys]
+	paths := m.Paths
+	m.mu.Unlock()
+	return paths
+}
+
+func (m *LegacyManager) GetUnifiedPath() (string, error) {
+	return "", errors.New("unified path is only supported when running in unified mode")
 }
 
 func join(c *configs.Cgroup, subsystem string, pid int) (string, error) {
@@ -319,46 +317,46 @@ func getSubsystemPath(c *configs.Cgroup, subsystem string) (string, error) {
 	return filepath.Join(mountpoint, initPath, slice, getUnitName(c)), nil
 }
 
-func (m *legacyManager) Freeze(state configs.FreezerState) error {
-	path, err := getSubsystemPath(m.cgroups, "freezer")
+func (m *LegacyManager) Freeze(state configs.FreezerState) error {
+	path, err := getSubsystemPath(m.Cgroups, "freezer")
 	if err != nil {
 		return err
 	}
-	prevState := m.cgroups.Resources.Freezer
-	m.cgroups.Resources.Freezer = state
+	prevState := m.Cgroups.Resources.Freezer
+	m.Cgroups.Resources.Freezer = state
 	freezer, err := legacySubsystems.Get("freezer")
 	if err != nil {
 		return err
 	}
-	err = freezer.Set(path, m.cgroups)
+	err = freezer.Set(path, m.Cgroups)
 	if err != nil {
-		m.cgroups.Resources.Freezer = prevState
+		m.Cgroups.Resources.Freezer = prevState
 		return err
 	}
 	return nil
 }
 
-func (m *legacyManager) GetPids() ([]int, error) {
-	path, err := getSubsystemPath(m.cgroups, "devices")
+func (m *LegacyManager) GetPids() ([]int, error) {
+	path, err := getSubsystemPath(m.Cgroups, "devices")
 	if err != nil {
 		return nil, err
 	}
 	return cgroups.GetPids(path)
 }
 
-func (m *legacyManager) GetAllPids() ([]int, error) {
-	path, err := getSubsystemPath(m.cgroups, "devices")
+func (m *LegacyManager) GetAllPids() ([]int, error) {
+	path, err := getSubsystemPath(m.Cgroups, "devices")
 	if err != nil {
 		return nil, err
 	}
 	return cgroups.GetAllPids(path)
 }
 
-func (m *legacyManager) GetStats() (*cgroups.Stats, error) {
+func (m *LegacyManager) GetStats() (*cgroups.Stats, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 	stats := cgroups.NewStats()
-	for name, path := range m.paths {
+	for name, path := range m.Paths {
 		sys, err := legacySubsystems.Get(name)
 		if err == errSubsystemDoesNotExist || !cgroups.PathExists(path) {
 			continue
@@ -371,10 +369,10 @@ func (m *legacyManager) GetStats() (*cgroups.Stats, error) {
 	return stats, nil
 }
 
-func (m *legacyManager) Set(container *configs.Config) error {
+func (m *LegacyManager) Set(container *configs.Config) error {
 	// If Paths are set, then we are just joining cgroups paths
 	// and there is no need to set any values.
-	if m.cgroups.Paths != nil {
+	if m.Cgroups.Paths != nil {
 		return nil
 	}
 	properties, err := genV1ResourcesProperties(container.Cgroups)
@@ -401,8 +399,8 @@ func (m *legacyManager) Set(container *configs.Config) error {
 		}
 	}
 
-	if m.paths["cpu"] != "" {
-		if err := fs.CheckCpushares(m.paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
+	if m.Paths["cpu"] != "" {
+		if err := fs.CheckCpushares(m.Paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
 			return err
 		}
 	}
@@ -429,13 +427,6 @@ func setKernelMemory(c *configs.Cgroup) error {
 	}
 	return fs.EnableKernelMemoryAccounting(path)
 }
-
-func (m *legacyManager) GetPaths() map[string]string {
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	return m.paths
-}
-
-func (m *legacyManager) GetCgroups() (*configs.Cgroup, error) {
-	return m.cgroups, nil
+func (m *LegacyManager) GetCgroups() (*configs.Cgroup, error) {
+	return m.Cgroups, nil
 }
diff --git a/libcontainer/cgroups/systemd/v2.go b/libcontainer/cgroups/systemd/v2.go
index d43c855b..46bbd567 100644
--- a/libcontainer/cgroups/systemd/v2.go
+++ b/libcontainer/cgroups/systemd/v2.go
@@ -26,7 +26,7 @@ type unifiedManager struct {
 	rootless bool
 }
 
-func NewUnifiedManager(config *configs.Cgroup, path string, rootless bool) cgroups.Manager {
+func NewUnifiedManager(config *configs.Cgroup, path string, rootless bool) *unifiedManager {
 	return &unifiedManager{
 		cgroups:  config,
 		path:     path,
@@ -153,7 +153,8 @@ func (m *unifiedManager) Apply(pid int) error {
 		return errors.Wrapf(err, "error while starting unit %q with properties %+v", unitName, properties)
 	}
 
-	if err = m.initPath(); err != nil {
+	_, err = m.GetUnifiedPath()
+	if err != nil {
 		return err
 	}
 	if err := fs2.CreateCgroupPath(m.path, m.cgroups); err != nil {
@@ -187,11 +188,22 @@ func (m *unifiedManager) Destroy() error {
 	return nil
 }
 
-func (m *unifiedManager) Path(_ string) string {
-	return m.path
+// this method is for v1 backward compatibility and will be removed
+func (m *unifiedManager) GetPaths() map[string]string {
+	_, _ = m.GetUnifiedPath()
+	paths := map[string]string{
+		"pids":    m.path,
+		"memory":  m.path,
+		"io":      m.path,
+		"cpu":     m.path,
+		"devices": m.path,
+		"cpuset":  m.path,
+		"freezer": m.path,
+	}
+	return paths
 }
 
-// getSliceFull value is used in initPath.
+// getSliceFull value is used in GetUnifiedPath.
 // The value is incompatible with systemdDbus.PropSlice.
 func (m *unifiedManager) getSliceFull() (string, error) {
 	c := m.cgroups
@@ -229,35 +241,37 @@ func (m *unifiedManager) getSliceFull() (string, error) {
 	return slice, nil
 }
 
-func (m *unifiedManager) initPath() error {
+func (m *unifiedManager) GetUnifiedPath() (string, error) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
 	if m.path != "" {
-		return nil
+		return m.path, nil
 	}
 
 	sliceFull, err := m.getSliceFull()
 	if err != nil {
-		return err
+		return "", err
 	}
 
 	c := m.cgroups
 	path := filepath.Join(sliceFull, getUnitName(c))
 	path, err = securejoin.SecureJoin(fs2.UnifiedMountpoint, path)
 	if err != nil {
-		return err
+		return "", err
 	}
+	m.path = path
 
 	// an example of the final path in rootless:
 	// "/sys/fs/cgroup/user.slice/user-1001.slice/user@1001.service/user.slice/libpod-132ff0d72245e6f13a3bbc6cdc5376886897b60ac59eaa8dea1df7ab959cbf1c.scope"
-	m.path = path
-
-	return nil
+	return m.path, nil
 }
 
 func (m *unifiedManager) fsManager() (cgroups.Manager, error) {
-	if err := m.initPath(); err != nil {
+	path, err := m.GetUnifiedPath()
+	if err != nil {
 		return nil, err
 	}
-	return fs2.NewManager(m.cgroups, m.path, m.rootless)
+	return fs2.NewManager(m.cgroups, path, m.rootless)
 }
 
 func (m *unifiedManager) Freeze(state configs.FreezerState) error {
@@ -269,17 +283,19 @@ func (m *unifiedManager) Freeze(state configs.FreezerState) error {
 }
 
 func (m *unifiedManager) GetPids() ([]int, error) {
-	if err := m.initPath(); err != nil {
+	path, err := m.GetUnifiedPath()
+	if err != nil {
 		return nil, err
 	}
-	return cgroups.GetPids(m.path)
+	return cgroups.GetPids(path)
 }
 
 func (m *unifiedManager) GetAllPids() ([]int, error) {
-	if err := m.initPath(); err != nil {
+	path, err := m.GetUnifiedPath()
+	if err != nil {
 		return nil, err
 	}
-	return cgroups.GetAllPids(m.path)
+	return cgroups.GetAllPids(path)
 }
 
 func (m *unifiedManager) GetStats() (*cgroups.Stats, error) {
@@ -310,12 +326,6 @@ func (m *unifiedManager) Set(container *configs.Config) error {
 	return fsMgr.Set(container)
 }
 
-func (m *unifiedManager) GetPaths() map[string]string {
-	paths := make(map[string]string, 1)
-	paths[""] = m.path
-	return paths
-}
-
 func (m *unifiedManager) GetCgroups() (*configs.Cgroup, error) {
 	return m.cgroups, nil
 }
diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go
index 5cea3d6c..ab2fbe14 100644
--- a/libcontainer/container_linux.go
+++ b/libcontainer/container_linux.go
@@ -64,12 +64,8 @@ type State struct {
 	// Set to true if BaseState.Config.RootlessEUID && BaseState.Config.RootlessCgroups
 	Rootless bool `json:"rootless"`
 
-	// Paths to all the container's cgroups, as returned by (*cgroups.Manager).GetPaths
-	//
-	// For cgroup v1, a key is cgroup subsystem name, and the value is the path
-	// to the cgroup for this subsystem.
-	//
-	// For cgroup v2 unified hierarchy, a key is "", and the value is the unified path.
+	// Path to all the cgroups setup for a container. Key is cgroup subsystem name
+	// with the value as the path.
 	CgroupPaths map[string]string `json:"cgroup_paths"`
 
 	// NamespacePaths are filepaths to the container's namespaces. Key is the namespace type
@@ -557,7 +553,7 @@ func (c *linuxContainer) newSetnsProcess(p *Process, cmd *exec.Cmd, messageSockP
 	}
 	return &setnsProcess{
 		cmd:             cmd,
-		cgroupPaths:     state.CgroupPaths,
+		cgroupPaths:     c.cgroupManager.GetPaths(),
 		rootlessCgroups: c.config.RootlessCgroups,
 		intelRdtPath:    state.IntelRdtPath,
 		messageSockPair: messageSockPair,
@@ -652,11 +648,14 @@ func (c *linuxContainer) NotifyOOM() (<-chan struct{}, error) {
 	if c.config.RootlessCgroups {
 		logrus.Warn("getting OOM notifications may fail if you don't have the full access to cgroups")
 	}
-	path := c.cgroupManager.Path("memory")
 	if cgroups.IsCgroup2UnifiedMode() {
+		path, err := c.cgroupManager.GetUnifiedPath()
+		if err != nil {
+			return nil, err
+		}
 		return notifyOnOOMV2(path)
 	}
-	return notifyOnOOM(path)
+	return notifyOnOOM(c.cgroupManager.GetPaths())
 }
 
 func (c *linuxContainer) NotifyMemoryPressure(level PressureLevel) (<-chan struct{}, error) {
@@ -664,7 +663,7 @@ func (c *linuxContainer) NotifyMemoryPressure(level PressureLevel) (<-chan struc
 	if c.config.RootlessCgroups {
 		logrus.Warn("getting memory pressure notifications may fail if you don't have the full access to cgroups")
 	}
-	return notifyMemoryPressure(c.cgroupManager.Path("memory"), level)
+	return notifyMemoryPressure(c.cgroupManager.GetPaths(), level)
 }
 
 var criuFeatures *criurpc.CriuFeatures
@@ -1024,12 +1023,23 @@ func (c *linuxContainer) Checkpoint(criuOpts *CriuOpts) error {
 
 	// CRIU can use cgroup freezer; when rpcOpts.FreezeCgroup
 	// is not set, CRIU uses ptrace() to pause the processes.
-	// Note cgroup v2 freezer is only supported since CRIU release 3.14.
-	if !cgroups.IsCgroup2UnifiedMode() || c.checkCriuVersion(31400) == nil {
-		if fcg := c.cgroupManager.Path("freezer"); fcg != "" {
-			rpcOpts.FreezeCgroup = proto.String(fcg)
+	var fcg string
+	switch cgroups.IsCgroup2UnifiedMode() {
+	case false:
+		fcg = c.cgroupManager.GetPaths()["freezer"]
+	case true:
+		// cgroup v2 freezer is only supported since CRIU release 3.14
+		if c.checkCriuVersion(31400) == nil {
+			fcg, err = c.cgroupManager.GetUnifiedPath()
+			if err != nil {
+				// should not happen
+				return err
+			}
 		}
 	}
+	if fcg != "" {
+		rpcOpts.FreezeCgroup = proto.String(fcg)
+	}
 
 	// append optional criu opts, e.g., page-server and port
 	if criuOpts.PageServer.Address != "" && criuOpts.PageServer.Port != 0 {
@@ -1847,17 +1857,23 @@ func (c *linuxContainer) runType() Status {
 }
 
 func (c *linuxContainer) isPaused() (bool, error) {
-	var filename, pausedState string
+	var fcg, filename, pausedState string
 
-	fcg := c.cgroupManager.Path("freezer")
 	if !cgroups.IsCgroup2UnifiedMode() {
+		fcg = c.cgroupManager.GetPaths()["freezer"]
 		if fcg == "" {
-			// container doesn't have a freezer cgroup
+			// A container doesn't have a freezer cgroup
 			return false, nil
 		}
 		filename = "freezer.state"
 		pausedState = "FROZEN"
 	} else {
+		var err error
+		fcg, err = c.cgroupManager.GetUnifiedPath()
+		if err != nil {
+			// should not happen
+			return false, err
+		}
 		filename = "cgroup.freeze"
 		pausedState = "1"
 	}
diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go
index 59548ef8..373d987f 100644
--- a/libcontainer/factory_linux.go
+++ b/libcontainer/factory_linux.go
@@ -88,7 +88,10 @@ func SystemdCgroups(l *LinuxFactory) error {
 	}
 
 	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		return systemd.NewLegacyManager(config, paths)
+		return &systemd.LegacyManager{
+			Cgroups: config,
+			Paths:   paths,
+		}
 	}
 
 	return nil
@@ -117,23 +120,22 @@ func cgroupfs2(l *LinuxFactory, rootless bool) error {
 	return nil
 }
 
-func cgroupfs(l *LinuxFactory, rootless bool) error {
+// Cgroupfs is an options func to configure a LinuxFactory to return containers
+// that use the native cgroups filesystem implementation to create and manage
+// cgroups.
+func Cgroupfs(l *LinuxFactory) error {
 	if cgroups.IsCgroup2UnifiedMode() {
-		return cgroupfs2(l, rootless)
+		return cgroupfs2(l, false)
 	}
 	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
-		return fs.NewManager(config, paths, rootless)
+		return &fs.Manager{
+			Cgroups: config,
+			Paths:   paths,
+		}
 	}
 	return nil
 }
 
-// Cgroupfs is an options func to configure a LinuxFactory to return containers
-// that use the native cgroups filesystem implementation to create and manage
-// cgroups.
-func Cgroupfs(l *LinuxFactory) error {
-	return cgroupfs(l, false)
-}
-
 // RootlessCgroupfs is an options func to configure a LinuxFactory to return
 // containers that use the native cgroups filesystem implementation to create
 // and manage cgroups. The difference between RootlessCgroupfs and Cgroupfs is
@@ -141,7 +143,17 @@ func Cgroupfs(l *LinuxFactory) error {
 // during rootless container (including euid=0 in userns) setup (while still allowing cgroup usage if
 // they've been set up properly).
 func RootlessCgroupfs(l *LinuxFactory) error {
-	return cgroupfs(l, true)
+	if cgroups.IsCgroup2UnifiedMode() {
+		return cgroupfs2(l, true)
+	}
+	l.NewCgroupsManager = func(config *configs.Cgroup, paths map[string]string) cgroups.Manager {
+		return &fs.Manager{
+			Cgroups:  config,
+			Rootless: true,
+			Paths:    paths,
+		}
+	}
+	return nil
 }
 
 // IntelRdtfs is an options func to configure a LinuxFactory to return
diff --git a/libcontainer/notify_linux.go b/libcontainer/notify_linux.go
index d7d1de1b..47a06783 100644
--- a/libcontainer/notify_linux.go
+++ b/libcontainer/notify_linux.go
@@ -3,7 +3,6 @@
 package libcontainer
 
 import (
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -12,6 +11,8 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+const oomCgroupName = "memory"
+
 type PressureLevel uint
 
 const (
@@ -65,17 +66,19 @@ func registerMemoryEvent(cgDir string, evName string, arg string) (<-chan struct
 
 // notifyOnOOM returns channel on which you can expect event about OOM,
 // if process died without OOM this channel will be closed.
-func notifyOnOOM(dir string) (<-chan struct{}, error) {
+func notifyOnOOM(paths map[string]string) (<-chan struct{}, error) {
+	dir := paths[oomCgroupName]
 	if dir == "" {
-		return nil, errors.New("memory controller missing")
+		return nil, fmt.Errorf("path %q missing", oomCgroupName)
 	}
 
 	return registerMemoryEvent(dir, "memory.oom_control", "")
 }
 
-func notifyMemoryPressure(dir string, level PressureLevel) (<-chan struct{}, error) {
+func notifyMemoryPressure(paths map[string]string, level PressureLevel) (<-chan struct{}, error) {
+	dir := paths[oomCgroupName]
 	if dir == "" {
-		return nil, errors.New("memory controller missing")
+		return nil, fmt.Errorf("path %q missing", oomCgroupName)
 	}
 
 	if level > CriticalPressure {
diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go
index f6212b83..727fdbd1 100644
--- a/libcontainer/specconv/spec_linux.go
+++ b/libcontainer/specconv/spec_linux.go
@@ -260,7 +260,6 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	}
 	if spec.Process != nil {
 		config.OomScoreAdj = spec.Process.OOMScoreAdj
-		config.NoNewPrivileges = spec.Process.NoNewPrivileges
 		if spec.Process.SelinuxLabel != "" {
 			config.ProcessLabel = spec.Process.SelinuxLabel
 		}


Test patch:
diff --git a/libcontainer/container_linux_test.go b/libcontainer/container_linux_test.go
index 356e8f1a..583c1c0c 100644
--- a/libcontainer/container_linux_test.go
+++ b/libcontainer/container_linux_test.go
@@ -15,10 +15,11 @@ import (
 )
 
 type mockCgroupManager struct {
-	pids    []int
-	allPids []int
-	stats   *cgroups.Stats
-	paths   map[string]string
+	pids        []int
+	allPids     []int
+	stats       *cgroups.Stats
+	paths       map[string]string
+	unifiedPath string
 }
 
 type mockIntelRdtManager struct {
@@ -54,8 +55,8 @@ func (m *mockCgroupManager) GetPaths() map[string]string {
 	return m.paths
 }
 
-func (m *mockCgroupManager) Path(subsys string) string {
-	return m.paths[subsys]
+func (m *mockCgroupManager) GetUnifiedPath() (string, error) {
+	return m.unifiedPath, nil
 }
 
 func (m *mockCgroupManager) Freeze(state configs.FreezerState) error {
diff --git a/libcontainer/notify_linux_test.go b/libcontainer/notify_linux_test.go
index 6f8b6d30..1e15ae2c 100644
--- a/libcontainer/notify_linux_test.go
+++ b/libcontainer/notify_linux_test.go
@@ -14,7 +14,7 @@ import (
 	"golang.org/x/sys/unix"
 )
 
-type notifyFunc func(path string) (<-chan struct{}, error)
+type notifyFunc func(paths map[string]string) (<-chan struct{}, error)
 
 func testMemoryNotification(t *testing.T, evName string, notify notifyFunc, targ string) {
 	memoryPath, err := ioutil.TempDir("", "testmemnotification-"+evName)
@@ -29,7 +29,10 @@ func testMemoryNotification(t *testing.T, evName string, notify notifyFunc, targ
 	if err := ioutil.WriteFile(eventPath, []byte{}, 0700); err != nil {
 		t.Fatal(err)
 	}
-	ch, err := notify(memoryPath)
+	paths := map[string]string{
+		"memory": memoryPath,
+	}
+	ch, err := notify(paths)
 	if err != nil {
 		t.Fatal("expected no error, got:", err)
 	}
@@ -99,8 +102,8 @@ func testMemoryNotification(t *testing.T, evName string, notify notifyFunc, targ
 }
 
 func TestNotifyOnOOM(t *testing.T) {
-	f := func(path string) (<-chan struct{}, error) {
-		return notifyOnOOM(path)
+	f := func(paths map[string]string) (<-chan struct{}, error) {
+		return notifyOnOOM(paths)
 	}
 
 	testMemoryNotification(t, "memory.oom_control", f, "")
@@ -114,8 +117,8 @@ func TestNotifyMemoryPressure(t *testing.T) {
 	}
 
 	for level, arg := range tests {
-		f := func(path string) (<-chan struct{}, error) {
-			return notifyMemoryPressure(path, level)
+		f := func(paths map[string]string) (<-chan struct{}, error) {
+			return notifyMemoryPressure(paths, level)
 		}
 
 		testMemoryNotification(t, "memory.pressure_level", f, arg)
diff --git a/libcontainer/specconv/spec_linux_test.go b/libcontainer/specconv/spec_linux_test.go
index 18bb7c7f..9c268865 100644
--- a/libcontainer/specconv/spec_linux_test.go
+++ b/libcontainer/specconv/spec_linux_test.go
@@ -391,11 +391,6 @@ func TestSpecconvExampleValidate(t *testing.T) {
 		t.Errorf("Couldn't create libcontainer config: %v", err)
 	}
 
-	if config.NoNewPrivileges != spec.Process.NoNewPrivileges {
-		t.Errorf("specconv NoNewPrivileges mismatch. Expected %v got %v",
-			spec.Process.NoNewPrivileges, config.NoNewPrivileges)
-	}
-
 	validator := validate.New()
 	if err := validator.Validate(config); err != nil {
 		t.Errorf("Expected specconv to produce valid container config: %v", err)
diff --git a/tests/rootless.sh b/tests/rootless.sh
index fc8e7ddd..19ec43e1 100755
--- a/tests/rootless.sh
+++ b/tests/rootless.sh
@@ -99,13 +99,6 @@ function enable_cgroup() {
 		chown root:rootless "$CGROUP_MOUNT/$CGROUP_PATH" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.subtree_control" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs"
 		chmod g+rwx "$CGROUP_MOUNT/$CGROUP_PATH"
 		chmod g+rw "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.subtree_control" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs"
-		# Fix up cgroup.type.
-		echo threaded > "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.type"
-		# Make sure cgroup.type doesn't contain "invalid". Otherwise write ops will fail with ENOTSUP.
-		# See http://man7.org/linux/man-pages/man7/cgroups.7.html
-		if grep -qw invalid "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.type"; then
-			exit 1
-		fi
 	fi
 }