diff --git a/pkg/sentry/fsimpl/cgroupfs/base.go b/pkg/sentry/fsimpl/cgroupfs/base.go index c68085fbc..f757dcda0 100644 --- a/pkg/sentry/fsimpl/cgroupfs/base.go +++ b/pkg/sentry/fsimpl/cgroupfs/base.go @@ -45,6 +45,11 @@ func (c *controllerCommon) init(ty kernel.CgroupControllerType, fs *filesystem) c.fs = fs } +func (c *controllerCommon) cloneFrom(other *controllerCommon) { + c.ty = other.ty + c.fs = other.fs +} + // Type implements kernel.CgroupController.Type. func (c *controllerCommon) Type() kernel.CgroupControllerType { return kernel.CgroupControllerType(c.ty) @@ -78,6 +83,11 @@ func (c *controllerCommon) RootCgroup() kernel.Cgroup { type controller interface { kernel.CgroupController + // Clone creates a new controller based on the internal state of the current + // controller. This is used to initialize a sub-cgroup based on the state of + // the parent. + Clone() controller + // AddControlFiles should extend the contents map with inodes representing // control files defined by this controller. AddControlFiles(ctx context.Context, creds *auth.Credentials, c *cgroupInode, contents map[string]kernfs.Inode) @@ -89,6 +99,12 @@ type controller interface { type cgroupInode struct { dir + // controllers is the set of controllers for this cgroup. This is used to + // store controller-specific state per cgroup. The set of controllers should + // match the controllers for this hierarchy as tracked by the filesystem + // object. Immutable. + controllers map[kernel.CgroupControllerType]controller + // ts is the list of tasks in this cgroup. The kernel is responsible for // removing tasks from this list before they're destroyed, so any tasks on // this list are always valid. @@ -99,10 +115,11 @@ type cgroupInode struct { var _ kernel.CgroupImpl = (*cgroupInode)(nil) -func (fs *filesystem) newCgroupInode(ctx context.Context, creds *auth.Credentials) kernfs.Inode { +func (fs *filesystem) newCgroupInode(ctx context.Context, creds *auth.Credentials, parent *cgroupInode) kernfs.Inode { c := &cgroupInode{ - dir: dir{fs: fs}, - ts: make(map[*kernel.Task]struct{}), + dir: dir{fs: fs}, + ts: make(map[*kernel.Task]struct{}), + controllers: make(map[kernel.CgroupControllerType]controller), } c.dir.cgi = c @@ -110,8 +127,19 @@ func (fs *filesystem) newCgroupInode(ctx context.Context, creds *auth.Credential contents["cgroup.procs"] = fs.newControllerFile(ctx, creds, &cgroupProcsData{c}) contents["tasks"] = fs.newControllerFile(ctx, creds, &tasksData{c}) - for _, ctl := range fs.controllers { - ctl.AddControlFiles(ctx, creds, c, contents) + if parent != nil { + for ty, ctl := range parent.controllers { + new := ctl.Clone() + c.controllers[ty] = new + new.AddControlFiles(ctx, creds, c, contents) + } + } else { + for _, ctl := range fs.controllers { + new := ctl.Clone() + // Uniqueness of controllers enforced by the filesystem on creation. + c.controllers[ctl.Type()] = new + new.AddControlFiles(ctx, creds, c, contents) + } } c.dir.InodeAttrs.Init(ctx, creds, linux.UNNAMED_MAJOR, fs.devMinor, fs.NextIno(), linux.ModeDirectory|linux.FileMode(0555)) diff --git a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go index e089b2c28..85b3e8801 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go +++ b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go @@ -294,7 +294,7 @@ func (fsType FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt fs.kcontrollers = append(fs.kcontrollers, c) } - root := fs.newCgroupInode(ctx, creds) + root := fs.newCgroupInode(ctx, creds, nil) var rootD kernfs.Dentry rootD.InitRoot(&fs.Filesystem, root) fs.root = &rootD @@ -451,7 +451,7 @@ func (d *dir) NewDir(ctx context.Context, name string, opts vfs.MkdirOptions) (k } return d.OrderedChildren.Inserter(name, func() kernfs.Inode { d.IncLinks(1) - return d.fs.newCgroupInode(ctx, auth.CredentialsFromContext(ctx)) + return d.fs.newCgroupInode(ctx, auth.CredentialsFromContext(ctx), d.cgi) }) } diff --git a/pkg/sentry/fsimpl/cgroupfs/cpu.go b/pkg/sentry/fsimpl/cgroupfs/cpu.go index 24d86a277..d81bc3e6d 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cpu.go +++ b/pkg/sentry/fsimpl/cgroupfs/cpu.go @@ -62,6 +62,17 @@ func newCPUController(fs *filesystem, defaults map[string]int64) *cpuController return c } +// Clone implements controller.Clone. +func (c *cpuController) Clone() controller { + new := &cpuController{ + cfsPeriod: c.cfsPeriod, + cfsQuota: c.cfsQuota, + shares: c.shares, + } + new.controllerCommon.cloneFrom(&c.controllerCommon) + return new +} + // AddControlFiles implements controller.AddControlFiles. func (c *cpuController) AddControlFiles(ctx context.Context, creds *auth.Credentials, _ *cgroupInode, contents map[string]kernfs.Inode) { contents["cpu.cfs_period_us"] = c.fs.newStaticControllerFile(ctx, creds, linux.FileMode(0644), fmt.Sprintf("%d\n", c.cfsPeriod)) diff --git a/pkg/sentry/fsimpl/cgroupfs/cpuacct.go b/pkg/sentry/fsimpl/cgroupfs/cpuacct.go index d4104a00e..8f9818423 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cpuacct.go +++ b/pkg/sentry/fsimpl/cgroupfs/cpuacct.go @@ -38,6 +38,13 @@ func newCPUAcctController(fs *filesystem) *cpuacctController { return c } +// Clone implements controller.Clone. +func (c *cpuacctController) Clone() controller { + new := &cpuacctController{} + new.controllerCommon.cloneFrom(&new.controllerCommon) + return c +} + // AddControlFiles implements controller.AddControlFiles. func (c *cpuacctController) AddControlFiles(ctx context.Context, creds *auth.Credentials, cg *cgroupInode, contents map[string]kernfs.Inode) { cpuacctCG := &cpuacctCgroup{cg} diff --git a/pkg/sentry/fsimpl/cgroupfs/cpuset.go b/pkg/sentry/fsimpl/cgroupfs/cpuset.go index 0f1990ba4..f6d7cfc39 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cpuset.go +++ b/pkg/sentry/fsimpl/cgroupfs/cpuset.go @@ -61,6 +61,20 @@ func newCPUSetController(k *kernel.Kernel, fs *filesystem) *cpusetController { return c } +// Clone implements controller.Clone. +func (c *cpusetController) Clone() controller { + cpus := c.cpus.Clone() + mems := c.mems.Clone() + new := &cpusetController{ + maxCpus: c.maxCpus, + maxMems: c.maxMems, + cpus: &cpus, + mems: &mems, + } + new.controllerCommon.cloneFrom(&c.controllerCommon) + return new +} + // AddControlFiles implements controller.AddControlFiles. func (c *cpusetController) AddControlFiles(ctx context.Context, creds *auth.Credentials, _ *cgroupInode, contents map[string]kernfs.Inode) { contents["cpuset.cpus"] = c.fs.newControllerWritableFile(ctx, creds, &cpusData{c: c}) diff --git a/pkg/sentry/fsimpl/cgroupfs/job.go b/pkg/sentry/fsimpl/cgroupfs/job.go index 3b2067007..9b3cae2d3 100644 --- a/pkg/sentry/fsimpl/cgroupfs/job.go +++ b/pkg/sentry/fsimpl/cgroupfs/job.go @@ -38,6 +38,15 @@ func newJobController(fs *filesystem) *jobController { return c } +// Clone implements controller.Clone. +func (c *jobController) Clone() controller { + new := &jobController{ + id: c.id, + } + new.controllerCommon.cloneFrom(&c.controllerCommon) + return new +} + func (c *jobController) AddControlFiles(ctx context.Context, creds *auth.Credentials, _ *cgroupInode, contents map[string]kernfs.Inode) { contents["job.id"] = c.fs.newControllerWritableFile(ctx, creds, &jobIDData{c: c}) } diff --git a/pkg/sentry/fsimpl/cgroupfs/memory.go b/pkg/sentry/fsimpl/cgroupfs/memory.go index d880c9bc4..aeefa01c6 100644 --- a/pkg/sentry/fsimpl/cgroupfs/memory.go +++ b/pkg/sentry/fsimpl/cgroupfs/memory.go @@ -63,6 +63,17 @@ func newMemoryController(fs *filesystem, defaults map[string]int64) *memoryContr return c } +// Clone implements controller.Clone. +func (c *memoryController) Clone() controller { + new := &memoryController{ + limitBytes: c.limitBytes, + softLimitBytes: c.softLimitBytes, + moveChargeAtImmigrate: c.moveChargeAtImmigrate, + } + new.controllerCommon.cloneFrom(&c.controllerCommon) + return new +} + // AddControlFiles implements controller.AddControlFiles. func (c *memoryController) AddControlFiles(ctx context.Context, creds *auth.Credentials, _ *cgroupInode, contents map[string]kernfs.Inode) { contents["memory.usage_in_bytes"] = c.fs.newControllerFile(ctx, creds, &memoryUsageInBytesData{}) diff --git a/test/syscalls/linux/cgroup.cc b/test/syscalls/linux/cgroup.cc index 6d07e4c2d..278c3c734 100644 --- a/test/syscalls/linux/cgroup.cc +++ b/test/syscalls/linux/cgroup.cc @@ -298,6 +298,34 @@ TEST(Cgroup, SubcontainerInitiallyEmpty) { EXPECT_TRUE(procs.empty()); } +TEST(Cgroup, SubcontainersHaveIndependentState) { + SKIP_IF(!CgroupsAvailable()); + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + // Use the job cgroup as a simple cgroup with state we can modify. + Cgroup c = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("job")); + + // Initially job.id should be the default value of 0. + EXPECT_THAT(c.ReadIntegerControlFile("job.id"), IsPosixErrorOkAndHolds(0)); + + // Set id so it is no longer the default. + ASSERT_NO_ERRNO(c.WriteIntegerControlFile("job.id", 1234)); + + // Create a child. The child should inherit the value from the parent, and not + // the default value of 0. + Cgroup child = ASSERT_NO_ERRNO_AND_VALUE(c.CreateChild("child1")); + EXPECT_THAT(child.ReadIntegerControlFile("job.id"), + IsPosixErrorOkAndHolds(1234)); + + // Setting the parent doesn't change the child. + ASSERT_NO_ERRNO(c.WriteIntegerControlFile("job.id", 5678)); + EXPECT_THAT(child.ReadIntegerControlFile("job.id"), + IsPosixErrorOkAndHolds(1234)); + + // Likewise, setting the child doesn't change the parent. + ASSERT_NO_ERRNO(child.WriteIntegerControlFile("job.id", 9012)); + EXPECT_THAT(c.ReadIntegerControlFile("job.id"), IsPosixErrorOkAndHolds(5678)); +} + TEST(MemoryCgroup, MemoryUsageInBytes) { SKIP_IF(!CgroupsAvailable());